Attachment 358551 Details for Bug 193222 – Patch v1

[patch] Patch v1

bug-193222-20190107161152.patch (text/plain), 6.58 KB, created by David Kilzer (:ddkilzer) on 2019-01-07 16:11:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: David Kilzer (:ddkilzer)

Created: 2019-01-07 16:11:53 PST

Size: 6.58 KB

patch

obsolete

>Subversion Revision: 239709
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index a14ba53733abcb8563d928e82dc3d38977818f5c..31049aec873595b3cd514b97c1614c802fddfc55 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        * 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  David Kilzer  <ddkilzer@apple.com>
> 
>         Prefer RetainPtr<NSObject> to RetainPtr<NSObject *>
>diff --git a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp
>index ffa6288af92bc7c7f46e9932e3b3e65652923f74..d300dc161c6e37b41c7c300046aa3a9971d003cb 100644
>--- a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp
>+++ b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp
>@@ -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)
>diff --git a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h
>index 9cc1e7c2977eb6947ce96c87a4060bc1515f3faa..283c114d05b741f2c6852cb92c5c7da6fceeea10 100644
>--- a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h
>+++ b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h
>@@ -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);
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index fa6b3003f698974af18fb44caad41a9d9d670f76..6cf8afdec7686c87ae6d6fc5b6e64068ae99c869 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        * 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  David Kilzer  <ddkilzer@apple.com>
> 
>         Prefer RetainPtr<NSObject> to RetainPtr<NSObject *>
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp b/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp
>index 25cfcbb0542b9eab9d93f1de11a6c6609a4b9594..2ed94cb85df25e40b5dacd2ca2c26a411e49f99c 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp
>+++ b/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp
>@@ -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)

Actions: View | Formatted Diff | Diff

Attachments on bug 193222: 358551