Bug 147339 - Crash in WebCore::DocumentLoader::willSendRequest() with ContentFilter and AppCache
Summary: Crash in WebCore::DocumentLoader::willSendRequest() with ContentFilter and Ap...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Brady Eidson
Depends on:
Reported: 2015-07-27 16:30 PDT by Brady Eidson
Modified: 2015-07-27 17:08 PDT (History)
3 users (show)

See Also:

Patch v1 (4.50 KB, patch)
2015-07-27 16:34 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-07-27 16:30:45 PDT
Crash in WebCore::DocumentLoader::willSendRequest() with ContentFilter and AppCache

#0	0x00000001050c1040 in WebCore::ResourceLoader::identifier() const at /Volumes/Data/git/OpenSource/Source/WebCore/loader/ResourceLoader.h:92
#1	0x00000001054b9174 in WebCore::DocumentLoader::willSendRequest(WebCore::ResourceRequest&, WebCore::ResourceResponse const&) at /Volumes/Data/git/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:554
#2	0x00000001054b8ca0 in WebCore::DocumentLoader::redirectReceived(WebCore::CachedResource*, WebCore::ResourceRequest&, WebCore::ResourceResponse const&) at /Volumes/Data/git/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:489
#3	0x00000001050c0088 in WebCore::CachedRawResource::didAddClient(WebCore::CachedResourceClient*) at /Volumes/Data/git/OpenSource/Source/WebCore/loader/cache/CachedRawResource.cpp:135
#4	0x00000001050c631c in WebCore::CachedResource::Callback::timerFired() at /Volumes/Data/git/OpenSource/Source/WebCore/loader/cache/CachedResource.cpp:779

The scenario is as follows:
- Content filters are on in Safari
- Visit a page that uses app cache, and has redirects for their main URL. Example is twitter.com, which uses app cache, and on iOS redirects to mobile.twitter.com
- When DocumentLoader adds itself as a client to the CachedRawResource for the main resource, the CachedResource doesn't actually add it synchronously. From CachedResource::addClientToSet:
        // Certain resources (especially XHRs and main resources) do crazy things if an asynchronous load returns
        // synchronously (e.g., scripts may not have set all the state they need to handle the load).
        // Therefore, rather than immediately sending callbacks on a cache hit like other CachedResources,
        // we schedule the callbacks and ensure we never finish synchronously.
        m_clientsAwaitingCallback.add(client, std::make_unique<Callback>(*this, *client));
- Before that timer fires, the main resource finishes loading, which clears the ResourceLoader from the CachedResource.
- Then the timer fires, actually adding the DocumentLoader as a client, and then all of the delegate callbacks are replayed.
- This includes the redirect, which redirects to a URL in the app cache, which sets up a substitute resource load and attempts to grab the load identifier for later use.


Even though the steps that lead to this crash are well understood at this point, creating a layout test for it has proven to be an uphill battle so far.

There's also a further downstream crash where the existence of a ResourceLoader is incorrectly assumed. That will also be reflected in the upcoming patch.

Comment 1 Brady Eidson 2015-07-27 16:34:31 PDT
Created attachment 257612 [details]
Patch v1
Comment 2 Alexey Proskuryakov 2015-07-27 16:36:19 PDT
Comment on attachment 257612 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=257612&action=review

> Source/WebCore/loader/cache/CachedResource.h:263
> +    unsigned long identifierForLoadWithoutResourceLoader() const { return m_identifierForLoadWithoutResourceLoader; }

Load without resource loader?
Comment 3 WebKit Commit Bot 2015-07-27 17:07:58 PDT
Comment on attachment 257612 [details]
Patch v1

Clearing flags on attachment: 257612

Committed r187466: <http://trac.webkit.org/changeset/187466>
Comment 4 WebKit Commit Bot 2015-07-27 17:08:01 PDT
All reviewed patches have been landed.  Closing bug.