Title: [253065] trunk/Source/WebKit
Revision
253065
Author
cdu...@apple.com
Date
2019-12-03 15:17:44 -0800 (Tue, 03 Dec 2019)

Log Message

REGRESSION (r252778): ASSERT(!m_networkLoad); in ~SpeculativeLoad()
https://bugs.webkit.org/show_bug.cgi?id=204813
<rdar://problem/57581082>

Reviewed by Antti Koivisto.

After r252778, SpeculativeLoadManager::revalidateSubresource() may delay the revalidation until we receive
the response for the main resource. We can hit the assertion in the SpeculativeLoad destructor if a speculative
revalidation for the SAME resource gets scheduled while we're waiting for the main resource response. When we
eventually receive the main resource response, we would call revalidateSubresource() again, which would create
a SpeculativeLoad and try to add it to m_pendingPreloads. Because m_pendingPreloads would already contain a
preload for this same resource, the SpeculativeLoad would not get added to the map and it would get destroyed
right away, before completing (thus hitting the assert). This unnecessary creation of the SpeculativeLoad is
inefficient and it is thus best to avoid it.

To address the issue, when we receive the response, we now make sure that m_pendingPreloads does not already
contain a preload for this resource before calling revalidateSubresource() again, similarly to what was
already done at the beginning of SpeculativeLoadManager::preloadEntry().

No new tests, unknown how to reproduce.

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

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (253064 => 253065)


--- trunk/Source/WebKit/ChangeLog	2019-12-03 23:03:58 UTC (rev 253064)
+++ trunk/Source/WebKit/ChangeLog	2019-12-03 23:17:44 UTC (rev 253065)
@@ -1,3 +1,29 @@
+2019-12-03  Chris Dumez  <cdu...@apple.com>
+
+        REGRESSION (r252778): ASSERT(!m_networkLoad); in ~SpeculativeLoad()
+        https://bugs.webkit.org/show_bug.cgi?id=204813
+        <rdar://problem/57581082>
+
+        Reviewed by Antti Koivisto.
+
+        After r252778, SpeculativeLoadManager::revalidateSubresource() may delay the revalidation until we receive
+        the response for the main resource. We can hit the assertion in the SpeculativeLoad destructor if a speculative
+        revalidation for the SAME resource gets scheduled while we're waiting for the main resource response. When we
+        eventually receive the main resource response, we would call revalidateSubresource() again, which would create
+        a SpeculativeLoad and try to add it to m_pendingPreloads. Because m_pendingPreloads would already contain a
+        preload for this same resource, the SpeculativeLoad would not get added to the map and it would get destroyed
+        right away, before completing (thus hitting the assert). This unnecessary creation of the SpeculativeLoad is
+        inefficient and it is thus best to avoid it.
+
+        To address the issue, when we receive the response, we now make sure that m_pendingPreloads does not already
+        contain a preload for this resource before calling revalidateSubresource() again, similarly to what was
+        already done at the beginning of SpeculativeLoadManager::preloadEntry().
+
+        No new tests, unknown how to reproduce.
+
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
+        (WebKit::NetworkCache::SpeculativeLoadManager::revalidateSubresource):
+
 2019-12-03  Eric Carlson  <eric.carl...@apple.com>
 
         Add a runtime setting for media in the GPU process

Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp (253064 => 253065)


--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp	2019-12-03 23:03:58 UTC (rev 253064)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp	2019-12-03 23:17:44 UTC (rev 253065)
@@ -486,6 +486,9 @@
     if (pendingLoad && !pendingLoad->didReceiveMainResourceResponse() && subresourceInfo.isFirstParty()) {
         preconnectForSubresource(subresourceInfo, entry.get(), frameID);
         pendingLoad->addPostMainResourceResponseTask([this, subresourceInfo, entry = WTFMove(entry), frameID]() mutable {
+            if (m_pendingPreloads.contains(subresourceInfo.key()))
+                return;
+
             revalidateSubresource(subresourceInfo, WTFMove(entry), frameID);
         });
         return;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to