Title: [218954] trunk/Source/WebCore
- Revision
- 218954
- Author
- [email protected]
- Date
- 2017-06-29 12:19:17 -0700 (Thu, 29 Jun 2017)
Log Message
REGRESSION(r218896): ASSERT in WebPageProxy::dataCallback
https://bugs.webkit.org/show_bug.cgi?id=173968
Patch by Carlos Garcia Campos <[email protected]> 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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (218953 => 218954)
--- trunk/Source/WebCore/ChangeLog 2017-06-29 19:13:10 UTC (rev 218953)
+++ trunk/Source/WebCore/ChangeLog 2017-06-29 19:19:17 UTC (rev 218954)
@@ -1,3 +1,23 @@
+2017-06-29 Carlos Garcia Campos <[email protected]>
+
+ 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 <[email protected]>
statistics.mostRecentUserInteraction should be of type WallTime
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (218953 => 218954)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2017-06-29 19:13:10 UTC (rev 218953)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2017-06-29 19:19:17 UTC (rev 218954)
@@ -1683,6 +1683,10 @@
{
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 @@
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 @@
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()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
