Leak of ScrollCompletionCallbackData (16 bytes) in com.apple.WebKit.WebContent runnin...
authorddkilzer@apple.com <ddkilzer@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 02:07:04 +0000 (02:07 +0000)
committerddkilzer@apple.com <ddkilzer@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 02:07:04 +0000 (02:07 +0000)
<https://webkit.org/b/193222>
<rdar://problem/46862309>

Reviewed by Joseph Pecoraro.

Source/WebKit:

* WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
(WKBundlePageRegisterScrollOperationCompletionCallback): Change
to return true if callback will be called, else false.
* WebProcess/InjectedBundle/API/c/WKBundlePage.h:
(WKBundlePageRegisterScrollOperationCompletionCallback): Change
to return `bool` value to denote whether callback will be called
(true) or not called (false).

Tools:

* WebKitTestRunner/InjectedBundle/EventSendingController.cpp:
(WTR::executeCallback): Fix camel case of variable name.
(WTR::EventSendingController::callAfterScrollingCompletes): If
WKBundlePageRegisterScrollOperationCompletionCallback() returns
false, make sure to release the ScrollCompletionCallbackData
object.  This fixes the leak.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239719 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp
Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp

index 47fb014..cae5710 100644 (file)
@@ -1,3 +1,19 @@
+2019-01-07  David Kilzer  <ddkilzer@apple.com>
+
+        Leak of ScrollCompletionCallbackData (16 bytes) in com.apple.WebKit.WebContent running WebKit layout tests
+        <https://webkit.org/b/193222>
+        <rdar://problem/46862309>
+
+        Reviewed by Joseph Pecoraro.
+
+        * WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
+        (WKBundlePageRegisterScrollOperationCompletionCallback): Change
+        to return true if callback will be called, else false.
+        * WebProcess/InjectedBundle/API/c/WKBundlePage.h:
+        (WKBundlePageRegisterScrollOperationCompletionCallback): Change
+        to return `bool` value to denote whether callback will be called
+        (true) or not called (false).
+
 2019-01-07  Alex Christensen  <achristensen@webkit.org>
 
         Remove use of NetworkProcess::singleton from CacheStorage::Engine::from
index ffa6288..d300dc1 100644 (file)
@@ -632,19 +632,20 @@ void WKBundlePageStartMonitoringScrollOperations(WKBundlePageRef pageRef)
     page->ensureTestTrigger();
 }
 
-void WKBundlePageRegisterScrollOperationCompletionCallback(WKBundlePageRef pageRef, WKBundlePageTestNotificationCallback callback, void* context)
+bool WKBundlePageRegisterScrollOperationCompletionCallback(WKBundlePageRef pageRef, WKBundlePageTestNotificationCallback callback, void* context)
 {
     if (!callback)
-        return;
+        return false;
     
     WebKit::WebPage* webPage = WebKit::toImpl(pageRef);
     WebCore::Page* page = webPage ? webPage->corePage() : nullptr;
     if (!page || !page->expectsWheelEventTriggers())
-        return;
+        return false;
     
     page->ensureTestTrigger().setTestCallbackAndStartNotificationTimer([=]() {
         callback(context);
     });
+    return true;
 }
 
 void WKBundlePageCallAfterTasksAndTimers(WKBundlePageRef pageRef, WKBundlePageTestNotificationCallback callback, void* context)
index 9cc1e7c..283c114 100644 (file)
@@ -120,7 +120,8 @@ WK_EXPORT void WKBundlePageStartMonitoringScrollOperations(WKBundlePageRef page)
 WK_EXPORT WKStringRef WKBundlePageCopyGroupIdentifier(WKBundlePageRef page);
 
 typedef void (*WKBundlePageTestNotificationCallback)(void* context);
-WK_EXPORT void WKBundlePageRegisterScrollOperationCompletionCallback(WKBundlePageRef, WKBundlePageTestNotificationCallback, void* context);
+// Returns true  if the callback function will be called, else false.
+WK_EXPORT bool WKBundlePageRegisterScrollOperationCompletionCallback(WKBundlePageRef, WKBundlePageTestNotificationCallback, void* context);
 
 // Call the given callback after both a postTask() on the page's document's ScriptExecutionContext, and a zero-delay timer.
 WK_EXPORT void WKBundlePageCallAfterTasksAndTimers(WKBundlePageRef, WKBundlePageTestNotificationCallback, void* context);
index fb760ab..ed3a4b4 100644 (file)
@@ -1,3 +1,18 @@
+2019-01-07  David Kilzer  <ddkilzer@apple.com>
+
+        Leak of ScrollCompletionCallbackData (16 bytes) in com.apple.WebKit.WebContent running WebKit layout tests
+        <https://webkit.org/b/193222>
+        <rdar://problem/46862309>
+
+        Reviewed by Joseph Pecoraro.
+
+        * WebKitTestRunner/InjectedBundle/EventSendingController.cpp:
+        (WTR::executeCallback): Fix camel case of variable name.
+        (WTR::EventSendingController::callAfterScrollingCompletes): If
+        WKBundlePageRegisterScrollOperationCompletionCallback() returns
+        false, make sure to release the ScrollCompletionCallbackData
+        object.  This fixes the leak.
+
 2019-01-07  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [Win] EWS: wincairo-ews cannot apply a patch with *.png
index 25cfcbb..2ed94cb 100644 (file)
@@ -604,10 +604,10 @@ static void executeCallback(void* context)
     if (!context)
         return;
 
-    std::unique_ptr<ScrollCompletionCallbackData> callBackData(reinterpret_cast<ScrollCompletionCallbackData*>(context));
+    std::unique_ptr<ScrollCompletionCallbackData> callbackData(reinterpret_cast<ScrollCompletionCallbackData*>(context));
 
-    JSObjectCallAsFunction(callBackData->m_context, callBackData->m_function, nullptr, 0, nullptr, nullptr);
-    JSValueUnprotect(callBackData->m_context, callBackData->m_function);
+    JSObjectCallAsFunction(callbackData->m_context, callbackData->m_function, nullptr, 0, nullptr, nullptr);
+    JSValueUnprotect(callbackData->m_context, callbackData->m_function);
 }
 
 void EventSendingController::callAfterScrollingCompletes(JSValueRef functionCallback)
@@ -626,8 +626,12 @@ void EventSendingController::callAfterScrollingCompletes(JSValueRef functionCall
     JSValueProtect(context, functionCallbackObject);
 
     auto scrollCompletionCallbackData = std::make_unique<ScrollCompletionCallbackData>(context, functionCallbackObject);
-
-    WKBundlePageRegisterScrollOperationCompletionCallback(page, executeCallback, scrollCompletionCallbackData.release());
+    auto scrollCompletionCallbackDataPtr = scrollCompletionCallbackData.release();
+    bool callbackWillBeCalled = WKBundlePageRegisterScrollOperationCompletionCallback(page, executeCallback, scrollCompletionCallbackDataPtr);
+    if (!callbackWillBeCalled) {
+        // Reassign raw pointer to std::unique_ptr<> so it will not be leaked.
+        scrollCompletionCallbackData.reset(scrollCompletionCallbackDataPtr);
+    }
 }
 
 #if ENABLE(TOUCH_EVENTS)