Crash under WebCore::DocumentWriter::addData()
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Jul 2018 23:19:31 +0000 (23:19 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Jul 2018 23:19:31 +0000 (23:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187819
<rdar://problem/41328743>

Reviewed by Brady Eidson.

When AppCache is used a DocumentLoader may start a NetworkLoad even though it has substitute data.
In DocumentLoader::continueAfterContentPolicy(), if we have substitute data we commit this data
and call finishLoad(). However, if the case where there was a NetworkLoad started, we'll send the
ContinueDidReceiveResponse IPC back to the network process and it will start sending us data for
the load. This could lead to crashes such as <rdar://problem/41328743> since the DocumentLoader
has already committed data and finished loading when it gets the data from the network process.

To address the issue, we now call clearMainResource() in continueAfterContentPolicy(), after we've
decided to commit the substitute data. This effectively removes the DocumentLoader as a client of
the CachedResource so that its will not be notified of following load progress. We do not cancel
the load as other CachedResourceClients may be interested in the load (ApplicationCacheResourceLoader
in particular, in order to update its cached data).

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::continueAfterContentPolicy):

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

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

index df867a8..aec9f6b 100644 (file)
@@ -1,3 +1,27 @@
+2018-07-19  Chris Dumez  <cdumez@apple.com>
+
+        Crash under WebCore::DocumentWriter::addData()
+        https://bugs.webkit.org/show_bug.cgi?id=187819
+        <rdar://problem/41328743>
+
+        Reviewed by Brady Eidson.
+
+        When AppCache is used a DocumentLoader may start a NetworkLoad even though it has substitute data.
+        In DocumentLoader::continueAfterContentPolicy(), if we have substitute data we commit this data
+        and call finishLoad(). However, if the case where there was a NetworkLoad started, we'll send the
+        ContinueDidReceiveResponse IPC back to the network process and it will start sending us data for
+        the load. This could lead to crashes such as <rdar://problem/41328743> since the DocumentLoader
+        has already committed data and finished loading when it gets the data from the network process.
+
+        To address the issue, we now call clearMainResource() in continueAfterContentPolicy(), after we've
+        decided to commit the substitute data. This effectively removes the DocumentLoader as a client of
+        the CachedResource so that its will not be notified of following load progress. We do not cancel
+        the load as other CachedResourceClients may be interested in the load (ApplicationCacheResourceLoader
+        in particular, in order to update its cached data).
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::continueAfterContentPolicy):
+
 2018-07-19  Dean Jackson  <dino@apple.com>
 
         CrashTracer: com.apple.WebKit.WebContent.Development at com.apple.WebCore: std::optional<WTF::Vector<WebCore::PluginInfo, 0ul, WTF::CrashOnOverflow, 16ul> >::operator* & + 73
index 7d8f92c..7da8c18 100644 (file)
@@ -946,6 +946,11 @@ void DocumentLoader::continueAfterContentPolicy(PolicyAction policy)
             dataReceived(content->data(), content->size());
         if (isLoadingMainResource())
             finishedLoading();
+
+        // Remove ourselves as a client of this CachedResource as we've decided to commit substitute data but the
+        // load may keep going and be useful to other clients of the CachedResource. If we did not do this, we
+        // may receive data later on even though this DocumentLoader has finished loading.
+        clearMainResource();
     }
 }