REGRESSION(r218896): ASSERT in WebPageProxy::dataCallback
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Jun 2017 19:19:17 +0000 (19:19 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Jun 2017 19:19:17 +0000 (19:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173968

Patch by Carlos Garcia Campos <cgarcia@igalia.com> on 2017-06-29
Reviewed by Michael Catanzaro.

The problem is that WebPageProxy::getLoadDecisionForIcon() sends 0 as callback ID when the decision is to not
load the icon. Since r218896 we always notify the client even when the decision is to not load the icon, in
which case the UI doesn't really expect a callback. When WebPageProxy::dataCallback is called with a 0 callback ID,
CallbackMap::take() crashes in RELEASE_ASSERT(callbackID).

Fixes several GTK+ unit tests that are crashing.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::didGetLoadDecisionForIcon): Return earlier if decision is false or frame is nullptr.
(WebCore::DocumentLoader::finishedLoadingIcon): Move RELEASE_ASSERT to notifyFinishedLoadingIcon().
(WebCore::DocumentLoader::notifyFinishedLoadingIcon): Assert if callbackIdentifier is 0 or m_frame is nullptr,
since it's no longer expected to happen.

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

Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp

index 00a291c..72c1c17 100644 (file)
@@ -1,3 +1,23 @@
+2017-06-29  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        REGRESSION(r218896): ASSERT in WebPageProxy::dataCallback
+        https://bugs.webkit.org/show_bug.cgi?id=173968
+
+        Reviewed by Michael Catanzaro.
+
+        The problem is that WebPageProxy::getLoadDecisionForIcon() sends 0 as callback ID when the decision is to not
+        load the icon. Since r218896 we always notify the client even when the decision is to not load the icon, in
+        which case the UI doesn't really expect a callback. When WebPageProxy::dataCallback is called with a 0 callback ID,
+        CallbackMap::take() crashes in RELEASE_ASSERT(callbackID).
+
+        Fixes several GTK+ unit tests that are crashing.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::didGetLoadDecisionForIcon): Return earlier if decision is false or frame is nullptr.
+        (WebCore::DocumentLoader::finishedLoadingIcon): Move RELEASE_ASSERT to notifyFinishedLoadingIcon().
+        (WebCore::DocumentLoader::notifyFinishedLoadingIcon): Assert if callbackIdentifier is 0 or m_frame is nullptr,
+        since it's no longer expected to happen.
+
 2017-06-29  Chris Dumez  <cdumez@apple.com>
 
         statistics.mostRecentUserInteraction should be of type WallTime
index 989f99d..c8fca60 100644 (file)
@@ -1683,6 +1683,10 @@ void DocumentLoader::didGetLoadDecisionForIcon(bool decision, uint64_t loadIdent
 {
     auto icon = m_iconsPendingLoadDecision.take(loadIdentifier);
 
+    // If the decision was not to load or this DocumentLoader is already detached, there is no load to perform.
+    if (!decision || !m_frame)
+        return;
+
     // If the LinkIcon we just took is empty, then the DocumentLoader had all of its loaders stopped
     // while this icon load decision was pending.
     // In this case we need to notify the client that the icon finished loading with empty data.
@@ -1691,10 +1695,6 @@ void DocumentLoader::didGetLoadDecisionForIcon(bool decision, uint64_t loadIdent
         return;
     }
 
-    // If the decision was not to load or this DocumentLoader is already detached, there is no load to perform.
-    if (!decision || !m_frame)
-        return;
-
     auto iconLoader = std::make_unique<IconLoader>(*this, icon.url);
     auto* rawIconLoader = iconLoader.get();
     m_iconLoaders.set(WTFMove(iconLoader), newCallbackID);
@@ -1708,15 +1708,14 @@ void DocumentLoader::finishedLoadingIcon(IconLoader& loader, SharedBuffer* buffe
     ASSERT(m_frame);
 
     auto callbackIdentifier = m_iconLoaders.take(&loader);
-    RELEASE_ASSERT(callbackIdentifier);
-
     notifyFinishedLoadingIcon(callbackIdentifier, buffer);
 }
 
 void DocumentLoader::notifyFinishedLoadingIcon(uint64_t callbackIdentifier, SharedBuffer* buffer)
 {
-    if (m_frame)
-        m_frame->loader().client().finishedLoadingIcon(callbackIdentifier, buffer);
+    RELEASE_ASSERT(callbackIdentifier);
+    RELEASE_ASSERT(m_frame);
+    m_frame->loader().client().finishedLoadingIcon(callbackIdentifier, buffer);
 }
 
 void DocumentLoader::dispatchOnloadEvents()