Changeset 234013 in webkit


Ignore:
Timestamp:
Jul 19, 2018 4:19:31 PM (6 years ago)
Author:
Chris Dumez
Message:

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):

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r234011 r234013  
     12018-07-19  Chris Dumez  <cdumez@apple.com>
     2
     3        Crash under WebCore::DocumentWriter::addData()
     4        https://bugs.webkit.org/show_bug.cgi?id=187819
     5        <rdar://problem/41328743>
     6
     7        Reviewed by Brady Eidson.
     8
     9        When AppCache is used a DocumentLoader may start a NetworkLoad even though it has substitute data.
     10        In DocumentLoader::continueAfterContentPolicy(), if we have substitute data we commit this data
     11        and call finishLoad(). However, if the case where there was a NetworkLoad started, we'll send the
     12        ContinueDidReceiveResponse IPC back to the network process and it will start sending us data for
     13        the load. This could lead to crashes such as <rdar://problem/41328743> since the DocumentLoader
     14        has already committed data and finished loading when it gets the data from the network process.
     15
     16        To address the issue, we now call clearMainResource() in continueAfterContentPolicy(), after we've
     17        decided to commit the substitute data. This effectively removes the DocumentLoader as a client of
     18        the CachedResource so that its will not be notified of following load progress. We do not cancel
     19        the load as other CachedResourceClients may be interested in the load (ApplicationCacheResourceLoader
     20        in particular, in order to update its cached data).
     21
     22        * loader/DocumentLoader.cpp:
     23        (WebCore::DocumentLoader::continueAfterContentPolicy):
     24
    1252018-07-19  Dean Jackson  <dino@apple.com>
    226
  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r233668 r234013  
    947947        if (isLoadingMainResource())
    948948            finishedLoading();
     949
     950        // Remove ourselves as a client of this CachedResource as we've decided to commit substitute data but the
     951        // load may keep going and be useful to other clients of the CachedResource. If we did not do this, we
     952        // may receive data later on even though this DocumentLoader has finished loading.
     953        clearMainResource();
    949954    }
    950955}
Note: See TracChangeset for help on using the changeset viewer.