Many modern media control tests leak documents in testing
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Sep 2018 04:14:39 +0000 (04:14 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Sep 2018 04:14:39 +0000 (04:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189437

Reviewed by Darin Adler.
Source/WebCore:

In order to accurately detect leaks in media controls tests which use lots of
SVGImages, we have to:
- Fire a zero-delay timer after the postTask, in order for ImagesLoader's m_derefElementTimer
  to clear references to elements.
- Have releaseCriticalMemory() call CachedResourceLoader's garbageCollectDocumentResources()
  to drop the last handle to the CachedResource for an SVGImage.
- Call WKBundleReleaseMemory() after the GC and timer, since we need garbageCollectDocumentResources()
  to run again after that timer has fired.

This should fix most of the spurious leak reports involving SVGImage documents.

* page/MemoryRelease.cpp:
(WebCore::releaseCriticalMemory):

Source/WebKit:

In order to accurately detect leaks in media controls tests which use lots of
SVGImages, we have to:
- Fire a zero-delay timer after the postTask, in order for ImagesLoader's m_derefElementTimer
  to clear references to elements.
- Have releaseCriticalMemory() call CachedResourceLoader's garbageCollectDocumentResources()
  to drop the last handle to the CachedResource for an SVGImage.
- Call WKBundleReleaseMemory() after the GC and timer, since we need garbageCollectDocumentResources()
  to run again after that timer has fired.

This should fix most of the spurious leak reports involving SVGImage documents.

* WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
(WKBundlePageCallAfterTasksAndTimers):
(WKBundlePagePostTask): Deleted.
* WebProcess/InjectedBundle/API/c/WKBundlePage.h:

Tools:

In order to accurately detect leaks in media controls tests which use lots of
SVGImages, we have to:
- Fire a zero-delay timer after the postTask, in order for ImagesLoader's m_derefElementTimer
  to clear references to elements.
- Have releaseCriticalMemory() call CachedResourceLoader's garbageCollectDocumentResources()
  to drop the last handle to the CachedResource for an SVGImage.
- Call WKBundleReleaseMemory() after the GC and timer, since we need garbageCollectDocumentResources()
  to run again after that timer has fired.

This should fix most of the spurious leak reports involving SVGImage documents.

* WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
(WTR::InjectedBundle::reportLiveDocuments):
(WTR::InjectedBundle::didReceiveMessageToPage):

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

Source/WebCore/ChangeLog
Source/WebCore/page/MemoryRelease.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp
Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp

index 715675b..c336c14 100644 (file)
@@ -1,3 +1,24 @@
+2018-09-17  Simon Fraser  <simon.fraser@apple.com>
+
+        Many modern media control tests leak documents in testing
+        https://bugs.webkit.org/show_bug.cgi?id=189437
+
+        Reviewed by Darin Adler.
+
+        In order to accurately detect leaks in media controls tests which use lots of
+        SVGImages, we have to:
+        - Fire a zero-delay timer after the postTask, in order for ImagesLoader's m_derefElementTimer
+          to clear references to elements.
+        - Have releaseCriticalMemory() call CachedResourceLoader's garbageCollectDocumentResources()
+          to drop the last handle to the CachedResource for an SVGImage.
+        - Call WKBundleReleaseMemory() after the GC and timer, since we need garbageCollectDocumentResources()
+          to run again after that timer has fired.
+        
+        This should fix most of the spurious leak reports involving SVGImage documents.
+
+        * page/MemoryRelease.cpp:
+        (WebCore::releaseCriticalMemory):
+
 2018-09-17  Jer Noble  <jer.noble@apple.com>
 
         Add support for HEVC codec types in Media Capabilities
index 57778bf..4a67fe0 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "CSSFontSelector.h"
 #include "CSSValuePool.h"
+#include "CachedResourceLoader.h"
 #include "Chrome.h"
 #include "ChromeClient.h"
 #include "CommonVM.h"
@@ -88,6 +89,7 @@ static void releaseCriticalMemory(Synchronous synchronous)
     for (auto& document : copyToVectorOf<RefPtr<Document>>(Document::allDocuments())) {
         document->styleScope().releaseMemory();
         document->fontSelector().emptyCaches();
+        document->cachedResourceLoader().garbageCollectDocumentResources();
     }
 
     GCController::singleton().deleteAllCode(JSC::DeleteAllCodeIfNotCollecting);
index d902e5e..2b50c8f 100644 (file)
@@ -1,3 +1,26 @@
+2018-09-17  Simon Fraser  <simon.fraser@apple.com>
+
+        Many modern media control tests leak documents in testing
+        https://bugs.webkit.org/show_bug.cgi?id=189437
+
+        Reviewed by Darin Adler.
+
+        In order to accurately detect leaks in media controls tests which use lots of
+        SVGImages, we have to:
+        - Fire a zero-delay timer after the postTask, in order for ImagesLoader's m_derefElementTimer
+          to clear references to elements.
+        - Have releaseCriticalMemory() call CachedResourceLoader's garbageCollectDocumentResources()
+          to drop the last handle to the CachedResource for an SVGImage.
+        - Call WKBundleReleaseMemory() after the GC and timer, since we need garbageCollectDocumentResources()
+          to run again after that timer has fired.
+        
+        This should fix most of the spurious leak reports involving SVGImage documents.
+
+        * WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
+        (WKBundlePageCallAfterTasksAndTimers):
+        (WKBundlePagePostTask): Deleted.
+        * WebProcess/InjectedBundle/API/c/WKBundlePage.h:
+
 2018-09-17  Dan Bernstein  <mitz@apple.com>
 
         Try to fix Apple internal builds with the iOS 12.0 SDK.
index 24d4f0d..fcd6e1b 100644 (file)
@@ -620,7 +620,7 @@ void WKBundlePageRegisterScrollOperationCompletionCallback(WKBundlePageRef pageR
     });
 }
 
-void WKBundlePagePostTask(WKBundlePageRef pageRef, WKBundlePageTestNotificationCallback callback, void* context)
+void WKBundlePageCallAfterTasksAndTimers(WKBundlePageRef pageRef, WKBundlePageTestNotificationCallback callback, void* context)
 {
     if (!callback)
         return;
@@ -634,8 +634,29 @@ void WKBundlePagePostTask(WKBundlePageRef pageRef, WKBundlePageTestNotificationC
     if (!document)
         return;
 
+    class TimerOwner {
+    public:
+        TimerOwner(WTF::Function<void (void*)>&& callback, void* context)
+            : m_timer(*this, &TimerOwner::timerFired)
+            , m_callback(WTFMove(callback))
+            , m_context(context)
+        {
+            m_timer.startOneShot(0_s);
+        }
+        
+        void timerFired()
+        {
+            m_callback(m_context);
+            delete this;
+        }
+        
+        WebCore::Timer m_timer;
+        WTF::Function<void (void*)> m_callback;
+        void* m_context;
+    };
+    
     document->postTask([=] (WebCore::ScriptExecutionContext&) {
-        callback(context);
+        new TimerOwner(callback, context); // deletes itself when done.
     });
 }
 
index 2765891..cd013df 100644 (file)
@@ -117,8 +117,8 @@ WK_EXPORT WKStringRef WKBundlePageCopyGroupIdentifier(WKBundlePageRef page);
 typedef void (*WKBundlePageTestNotificationCallback)(void* context);
 WK_EXPORT void WKBundlePageRegisterScrollOperationCompletionCallback(WKBundlePageRef, WKBundlePageTestNotificationCallback, void* context);
 
-// Posts a task in the ScriptExecutionContext of the main frame. Used to do work after other tasks have completed.
-WK_EXPORT void WKBundlePagePostTask(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);
 
 WK_EXPORT void WKBundlePagePostMessage(WKBundlePageRef page, WKStringRef messageName, WKTypeRef messageBody);
 
index 30321e7..5bbbc26 100644 (file)
@@ -1,3 +1,25 @@
+2018-09-17  Simon Fraser  <simon.fraser@apple.com>
+
+        Many modern media control tests leak documents in testing
+        https://bugs.webkit.org/show_bug.cgi?id=189437
+
+        Reviewed by Darin Adler.
+        
+        In order to accurately detect leaks in media controls tests which use lots of
+        SVGImages, we have to:
+        - Fire a zero-delay timer after the postTask, in order for ImagesLoader's m_derefElementTimer
+          to clear references to elements.
+        - Have releaseCriticalMemory() call CachedResourceLoader's garbageCollectDocumentResources()
+          to drop the last handle to the CachedResource for an SVGImage.
+        - Call WKBundleReleaseMemory() after the GC and timer, since we need garbageCollectDocumentResources()
+          to run again after that timer has fired.
+        
+        This should fix most of the spurious leak reports involving SVGImage documents.
+
+        * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
+        (WTR::InjectedBundle::reportLiveDocuments):
+        (WTR::InjectedBundle::didReceiveMessageToPage):
+
 2018-09-17  Chris Dumez  <cdumez@apple.com>
 
         PSON: window.open() with 'noopener' should only process-swap cross-site, not cross-origin
index 72a057f..5f1c220 100644 (file)
@@ -189,6 +189,9 @@ static void postGCTask(void* context)
 
 void InjectedBundle::reportLiveDocuments(WKBundlePageRef page)
 {
+    // Release memory again, after the GC and timer fire. This is necessary to clear entries from CachedResourceLoader's m_documentResources in some scenarios.
+    WKBundleReleaseMemory(m_bundle);
+
     const bool excludeDocumentsInPageGroup = true;
     auto documentURLs = adoptWK(WKBundleGetLiveDocumentURLs(m_bundle, m_pageGroup, excludeDocumentsInPageGroup));
     auto ackMessageName = adoptWK(WKStringCreateWithUTF8CString("LiveDocuments"));
@@ -274,7 +277,7 @@ void InjectedBundle::didReceiveMessageToPage(WKBundlePageRef page, WKStringRef m
         WKBundleReleaseMemory(m_bundle);
 
         WKRetain(page); // Balanced by the release in postGCTask.
-        WKBundlePagePostTask(page, postGCTask, (void*)page);
+        WKBundlePageCallAfterTasksAndTimers(page, postGCTask, (void*)page);
         return;
     }