Title: [260390] trunk/Source/WebKit
Revision
260390
Author
[email protected]
Date
2020-04-20 13:04:49 -0700 (Mon, 20 Apr 2020)

Log Message

When SpeculativeLoadManager is destroyed, properly clean up its PendingFrameLoads
https://bugs.webkit.org/show_bug.cgi?id=210759
<rdar://problem/62056856>

Patch by Alex Christensen <[email protected]> on 2020-04-20
Reviewed by Darin Adler.

Recent work on the resourceLoadStatistics layout tests increased the amount we swap out the WebsiteDataStore.
When this happens, the NetworkSession is eventually destroyed in the NetworkProcess, sometimes when running the next test.
An assertion was firing in the PendingFrameLoad destructor because it hadn't been marked as complete when it was destroyed.
Rather than remove the assertion, when we destroy the SpeculativeLoadManager (which only happens when a WebsiteDataStore
is destroyed) during a speculative pending frame load, just mark the pending frame load as complete because it is being cancelled.
Marking the pending frame load as complete can tell the SpeculativeLoadManager to mutate m_pendingFrameLoads, which we don't want
to do while iterating, so copy the RefPtrs into a Vector first then iterate that to get them all.

This fixes an assertion that was sometimes hit in http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-prevalent-subresource-redirects-database.html
but only after running other tests that had initiated speculative pending frame loads.  This was ostensibly started by r260322 but is quite unrelated.

* NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
(WebKit::NetworkCache::SpeculativeLoadManager::~SpeculativeLoadManager):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (260389 => 260390)


--- trunk/Source/WebKit/ChangeLog	2020-04-20 19:56:08 UTC (rev 260389)
+++ trunk/Source/WebKit/ChangeLog	2020-04-20 20:04:49 UTC (rev 260390)
@@ -1,3 +1,25 @@
+2020-04-20  Alex Christensen  <[email protected]>
+
+        When SpeculativeLoadManager is destroyed, properly clean up its PendingFrameLoads
+        https://bugs.webkit.org/show_bug.cgi?id=210759
+        <rdar://problem/62056856>
+
+        Reviewed by Darin Adler.
+
+        Recent work on the resourceLoadStatistics layout tests increased the amount we swap out the WebsiteDataStore.
+        When this happens, the NetworkSession is eventually destroyed in the NetworkProcess, sometimes when running the next test.
+        An assertion was firing in the PendingFrameLoad destructor because it hadn't been marked as complete when it was destroyed.
+        Rather than remove the assertion, when we destroy the SpeculativeLoadManager (which only happens when a WebsiteDataStore
+        is destroyed) during a speculative pending frame load, just mark the pending frame load as complete because it is being cancelled.
+        Marking the pending frame load as complete can tell the SpeculativeLoadManager to mutate m_pendingFrameLoads, which we don't want
+        to do while iterating, so copy the RefPtrs into a Vector first then iterate that to get them all.
+
+        This fixes an assertion that was sometimes hit in http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-prevalent-subresource-redirects-database.html
+        but only after running other tests that had initiated speculative pending frame loads.  This was ostensibly started by r260322 but is quite unrelated.
+
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
+        (WebKit::NetworkCache::SpeculativeLoadManager::~SpeculativeLoadManager):
+
 2020-04-20  Stephan Szabo  <[email protected]>
 
         [PlayStation] Fix build after r260277

Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp (260389 => 260390)


--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp	2020-04-20 19:56:08 UTC (rev 260389)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp	2020-04-20 20:04:49 UTC (rev 260390)
@@ -267,6 +267,10 @@
 
 SpeculativeLoadManager::~SpeculativeLoadManager()
 {
+    for (auto& pendingFrameLoad : copyToVector(m_pendingFrameLoads.values())) {
+        if (pendingFrameLoad)
+            pendingFrameLoad->markLoadAsCompleted();
+    }
 }
 
 bool SpeculativeLoadManager::canUsePreloadedEntry(const PreloadedEntry& entry, const ResourceRequest& actualRequest)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to