- Revision
- 188150
- Author
- [email protected]
- Date
- 2015-08-07 13:10:42 -0700 (Fri, 07 Aug 2015)
Log Message
Crash when following a Google search link to Twitter with Limit Adult Content enabled
https://bugs.webkit.org/show_bug.cgi?id=147651
Reviewed by Brady Eidson.
When a loaded CachedRawResource gets a new client, it synthesizes the callbacks that the new client would have
received while the resource was loading. Unlike a real network load, it synthesizes these callbacks in a single
run loop iteration. When DocumentLoader receives a redirect, and finds substitute data in the app cache for the
redirect URL, it schedules a timer that removes DocumentLoader as a client of the CachedRawResource then
synthesizes its own set of CachedRawResourceClient callbacks. But since CachedRawResource has already delivered
client callbacks before the app cache timer fires, DocumentLoader unexpectedly ends up getting two sets of
client callbacks and badness ensues.
The fix is to let CachedRawResource detect if a redirect will trigger the client to load substitute data. If so,
stop delivering client callbacks.
Layout test to follow.
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::syntheticRedirectReceived): If there is valid substitute data, do not continue.
* loader/DocumentLoader.h:
* loader/cache/CachedRawResource.cpp: Returned early if syntheticRedirectReceived() said not to continue.
(WebCore::CachedRawResource::didAddClient):
* loader/cache/CachedRawResourceClient.h:
(WebCore::CachedRawResourceClient::syntheticRedirectReceived):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (188149 => 188150)
--- trunk/Source/WebCore/ChangeLog 2015-08-07 20:06:24 UTC (rev 188149)
+++ trunk/Source/WebCore/ChangeLog 2015-08-07 20:10:42 UTC (rev 188150)
@@ -1,3 +1,31 @@
+2015-08-07 Andy Estes <[email protected]>
+
+ Crash when following a Google search link to Twitter with Limit Adult Content enabled
+ https://bugs.webkit.org/show_bug.cgi?id=147651
+
+ Reviewed by Brady Eidson.
+
+ When a loaded CachedRawResource gets a new client, it synthesizes the callbacks that the new client would have
+ received while the resource was loading. Unlike a real network load, it synthesizes these callbacks in a single
+ run loop iteration. When DocumentLoader receives a redirect, and finds substitute data in the app cache for the
+ redirect URL, it schedules a timer that removes DocumentLoader as a client of the CachedRawResource then
+ synthesizes its own set of CachedRawResourceClient callbacks. But since CachedRawResource has already delivered
+ client callbacks before the app cache timer fires, DocumentLoader unexpectedly ends up getting two sets of
+ client callbacks and badness ensues.
+
+ The fix is to let CachedRawResource detect if a redirect will trigger the client to load substitute data. If so,
+ stop delivering client callbacks.
+
+ Layout test to follow.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::syntheticRedirectReceived): If there is valid substitute data, do not continue.
+ * loader/DocumentLoader.h:
+ * loader/cache/CachedRawResource.cpp: Returned early if syntheticRedirectReceived() said not to continue.
+ (WebCore::CachedRawResource::didAddClient):
+ * loader/cache/CachedRawResourceClient.h:
+ (WebCore::CachedRawResourceClient::syntheticRedirectReceived):
+
2015-08-06 Dean Jackson <[email protected]>
Shadows don't draw on fillText when using a gradient fill
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (188149 => 188150)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2015-08-07 20:06:24 UTC (rev 188149)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2015-08-07 20:10:42 UTC (rev 188150)
@@ -489,6 +489,14 @@
willSendRequest(request, redirectResponse);
}
+void DocumentLoader::syntheticRedirectReceived(CachedResource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse, bool& shouldContinue)
+{
+ redirectReceived(resource, request, redirectResponse);
+
+ // If we will soon remove our reference to the CachedRawResource in favor of a SubstituteData load, we don't want to continue receiving synthetic CachedRawResource callbacks.
+ shouldContinue = !m_substituteData.isValid();
+}
+
void DocumentLoader::willSendRequest(ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
{
// Note that there are no asserts here as there are for the other callbacks. This is due to the
Modified: trunk/Source/WebCore/loader/DocumentLoader.h (188149 => 188150)
--- trunk/Source/WebCore/loader/DocumentLoader.h 2015-08-07 20:06:24 UTC (rev 188149)
+++ trunk/Source/WebCore/loader/DocumentLoader.h 2015-08-07 20:10:42 UTC (rev 188150)
@@ -306,6 +306,7 @@
void finishedLoading(double finishTime);
void mainReceivedError(const ResourceError&);
WEBCORE_EXPORT virtual void redirectReceived(CachedResource*, ResourceRequest&, const ResourceResponse&) override;
+ WEBCORE_EXPORT virtual void syntheticRedirectReceived(CachedResource*, ResourceRequest&, const ResourceResponse&, bool& /* shouldContinue */) override;
WEBCORE_EXPORT virtual void responseReceived(CachedResource*, const ResourceResponse&) override;
WEBCORE_EXPORT virtual void dataReceived(CachedResource*, const char* data, int length) override;
WEBCORE_EXPORT virtual void notifyFinished(CachedResource*) override;
Modified: trunk/Source/WebCore/loader/cache/CachedRawResource.cpp (188149 => 188150)
--- trunk/Source/WebCore/loader/cache/CachedRawResource.cpp 2015-08-07 20:06:24 UTC (rev 188149)
+++ trunk/Source/WebCore/loader/cache/CachedRawResource.cpp 2015-08-07 20:10:42 UTC (rev 188150)
@@ -132,8 +132,9 @@
for (size_t i = 0; i < redirectCount; i++) {
RedirectPair redirect = m_redirectChain[i];
ResourceRequest request(redirect.m_request);
- client->redirectReceived(this, request, redirect.m_redirectResponse);
- if (!hasClient(c))
+ bool shouldContinue = true;
+ client->syntheticRedirectReceived(this, request, redirect.m_redirectResponse, shouldContinue);
+ if (!hasClient(c) || !shouldContinue)
return;
}
ASSERT(redirectCount == m_redirectChain.size());
Modified: trunk/Source/WebCore/loader/cache/CachedRawResourceClient.h (188149 => 188150)
--- trunk/Source/WebCore/loader/cache/CachedRawResourceClient.h 2015-08-07 20:06:24 UTC (rev 188149)
+++ trunk/Source/WebCore/loader/cache/CachedRawResourceClient.h 2015-08-07 20:10:42 UTC (rev 188150)
@@ -41,6 +41,10 @@
virtual void responseReceived(CachedResource*, const ResourceResponse&) { }
virtual void dataReceived(CachedResource*, const char* /* data */, int /* length */) { }
virtual void redirectReceived(CachedResource*, ResourceRequest&, const ResourceResponse&) { }
+
+ // In response to a redirect, some clients wish to receive no futher callbacks, but cannot immediately remove themselves as a client.
+ // Those clients can express their desire to recieve no futher callbacks by setting shouldContinue to false.
+ virtual void syntheticRedirectReceived(CachedResource* resource, ResourceRequest& request, const ResourceResponse& response, bool& /* shouldContinue */) { redirectReceived(resource, request, response); }
#if USE(SOUP)
virtual char* getOrCreateReadBuffer(CachedResource*, size_t /* requestedSize */, size_t& /* actualSize */) { return 0; }
#endif
