147339 2015-07-27 16:30:45 -0700 Crash in WebCore::DocumentLoader::willSendRequest() with ContentFilter and AppCache 2015-07-27 17:08:01 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) All Unspecified RESOLVED FIXED P2 Normal --- 1 beidson beidson ap commit-queue japhet oldest_to_newest 1112578 0 beidson 2015-07-27 16:30:45 -0700 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. *phew* 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. <rdar://problem/21960398> 1112580 1 257612 beidson 2015-07-27 16:34:31 -0700 Created attachment 257612 Patch v1 1112582 2 257612 ap 2015-07-27 16:36:19 -0700 Comment on attachment 257612 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? 1112608 3 257612 commit-queue 2015-07-27 17:07:58 -0700 Comment on attachment 257612 Patch v1 Clearing flags on attachment: 257612 Committed r187466: <http://trac.webkit.org/changeset/187466> 1112609 4 commit-queue 2015-07-27 17:08:01 -0700 All reviewed patches have been landed. Closing bug. 257612 2015-07-27 16:34:31 -0700 2015-07-27 17:07:58 -0700 Patch v1 patch text/plain 4612 beidson ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAyYTI5Nzk3Li4yM2E1YzdiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEg QEAKKzIwMTUtMDctMjcgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgorCisgICAg ICAgIENyYXNoIGluIFdlYkNvcmU6OkRvY3VtZW50TG9hZGVyOjp3aWxsU2VuZFJlcXVlc3QoKSB3 aXRoIENvbnRlbnRGaWx0ZXIgYW5kIEFwcENhY2hlLgorICAgICAgICA8cmRhcjovL3Byb2JsZW0v MjE5NjAzOTg+IGFuZCBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ3 MzM5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8g bmV3IHRlc3RzIChOb3QgeWV0IHByb3ZlbiB0byBiZSBwb3NzaWJsZSB0byB0ZXN0IHRoaXMpLgor CisgICAgICAgICogbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkRvY3VtZW50TG9hZGVyOjp3aWxsU2VuZFJlcXVlc3QpOiBHcmFiIHRoZSBpZGVudGlmaWVyIGZy b20gdGhlIENhY2hlZFJlc291cmNlIGRpcmVjdGx5LCBub3QgZnJvbSB0aGUgbnVsbCBSZXNvdXJj ZUxvYWRlci4KKyAgICAgICAgKFdlYkNvcmU6OkRvY3VtZW50TG9hZGVyOjpjb250aW51ZUFmdGVy TmF2aWdhdGlvblBvbGljeSk6IE51bGwgY2hlY2sgdGhlIFJlc291cmNlTG9hZGVyLCBhcyBpdCBj YW4gZGVmaW5pdGVseSBiZSBnb25lIGJ5IHRoaXMgcG9pbnQuCisKKyAgICAgICAgKiBsb2FkZXIv Y2FjaGUvQ2FjaGVkUmVzb3VyY2UuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Q2FjaGVkUmVzb3Vy Y2U6OmNsZWFyTG9hZGVyKTogU2F2ZSBvZmYgdGhlIGlkZW50aWZpZXIgZm9yIGxhdGVyIHVzZS4K KyAgICAgICAgKiBsb2FkZXIvY2FjaGUvQ2FjaGVkUmVzb3VyY2UuaDoKKyAgICAgICAgKFdlYkNv cmU6OkNhY2hlZFJlc291cmNlOjppZGVudGlmaWVyRm9yTG9hZFdpdGhvdXRSZXNvdXJjZUxvYWRl cik6IEV4cG9zZSB0aGUgaWRlbnRpZmllciB0aGF0IHRoZSBSZXNvdXJjZUxvYWRlciBoYWQgd2hl biBpdCB3ZW50IGF3YXkuCisKIDIwMTUtMDctMjcgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNl ckBhcHBsZS5jb20+CiAKICAgICAgICAgRW5oYW5jZSBBbmltYXRpb24gbG9nZ2luZyBzbGlnaHRs eQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcCBi L1NvdXJjZS9XZWJDb3JlL2xvYWRlci9Eb2N1bWVudExvYWRlci5jcHAKaW5kZXggZGJjOGZlZS4u YzAyNmRlOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL0RvY3VtZW50TG9hZGVy LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvRG9jdW1lbnRMb2FkZXIuY3BwCkBAIC01 NTAsOCArNTUwLDEwIEBAIHZvaWQgRG9jdW1lbnRMb2FkZXI6OndpbGxTZW5kUmVxdWVzdChSZXNv dXJjZVJlcXVlc3QmIG5ld1JlcXVlc3QsIGNvbnN0IFJlc291cmNlCiAgICAgICAgIC8vIFdlIGNo ZWNrZWQgYXBwbGljYXRpb24gY2FjaGUgZm9yIGluaXRpYWwgVVJMLCBub3cgd2UgbmVlZCB0byBj aGVjayBpdCBmb3IgcmVkaXJlY3RlZCBvbmUuCiAgICAgICAgIEFTU0VSVCghbV9zdWJzdGl0dXRl RGF0YS5pc1ZhbGlkKCkpOwogICAgICAgICBtX2FwcGxpY2F0aW9uQ2FjaGVIb3N0LT5tYXliZUxv YWRNYWluUmVzb3VyY2VGb3JSZWRpcmVjdChuZXdSZXF1ZXN0LCBtX3N1YnN0aXR1dGVEYXRhKTsK LSAgICAgICAgaWYgKG1fc3Vic3RpdHV0ZURhdGEuaXNWYWxpZCgpKQotICAgICAgICAgICAgbV9p ZGVudGlmaWVyRm9yTG9hZFdpdGhvdXRSZXNvdXJjZUxvYWRlciA9IG1haW5SZXNvdXJjZUxvYWRl cigpLT5pZGVudGlmaWVyKCk7CisgICAgICAgIGlmIChtX3N1YnN0aXR1dGVEYXRhLmlzVmFsaWQo KSkgeworICAgICAgICAgICAgUkVMRUFTRV9BU1NFUlQobV9tYWluUmVzb3VyY2UpOworICAgICAg ICAgICAgbV9pZGVudGlmaWVyRm9yTG9hZFdpdGhvdXRSZXNvdXJjZUxvYWRlciA9IG1fbWFpblJl c291cmNlLT5pZGVudGlmaWVyRm9yTG9hZFdpdGhvdXRSZXNvdXJjZUxvYWRlcigpOworICAgICAg ICB9CiAgICAgfQogCiAgICAgLy8gRklYTUU6IElkZWFsbHkgd2UnZCBzdG9wIHRoZSBJL08gdW50 aWwgd2UgaGVhciBiYWNrIGZyb20gdGhlIG5hdmlnYXRpb24gcG9saWN5IGRlbGVnYXRlCkBAIC01 ODEsMTAgKzU4MywxNSBAQCB2b2lkIERvY3VtZW50TG9hZGVyOjpjb250aW51ZUFmdGVyTmF2aWdh dGlvblBvbGljeShjb25zdCBSZXNvdXJjZVJlcXVlc3QmLCBib29sCiAgICAgICAgIC8vIEhvd2V2 ZXIsIGZyb20gYW4gQVBJIHBlcnNwZWN0aXZlLCB0aGlzIGlzbid0IGEgY2FuY2VsbGF0aW9uLiBU aGVyZWZvcmUsIHNldmVyIG91ciByZWxhdGlvbnNoaXAgd2l0aCB0aGUgbmV0d29yayBsb2FkLAog ICAgICAgICAvLyBidXQgcHJldmVudCB0aGUgUmVzb3VyY2VMb2FkZXIgZnJvbSBzZW5kaW5nIFJl c291cmNlTG9hZE5vdGlmaWVyIGNhbGxiYWNrcy4KICAgICAgICAgUmVmUHRyPFJlc291cmNlTG9h ZGVyPiByZXNvdXJjZUxvYWRlciA9IG1haW5SZXNvdXJjZUxvYWRlcigpOwotICAgICAgICBBU1NF UlQocmVzb3VyY2VMb2FkZXItPnNob3VsZFNlbmRSZXNvdXJjZUxvYWRDYWxsYmFja3MoKSk7Ci0g ICAgICAgIHJlc291cmNlTG9hZGVyLT5zZXRTZW5kQ2FsbGJhY2tQb2xpY3koRG9Ob3RTZW5kQ2Fs bGJhY2tzKTsKKyAgICAgICAgaWYgKHJlc291cmNlTG9hZGVyKSB7CisgICAgICAgICAgICBBU1NF UlQocmVzb3VyY2VMb2FkZXItPnNob3VsZFNlbmRSZXNvdXJjZUxvYWRDYWxsYmFja3MoKSk7Cisg ICAgICAgICAgICByZXNvdXJjZUxvYWRlci0+c2V0U2VuZENhbGxiYWNrUG9saWN5KERvTm90U2Vu ZENhbGxiYWNrcyk7CisgICAgICAgIH0KKwogICAgICAgICBjbGVhck1haW5SZXNvdXJjZSgpOwot ICAgICAgICByZXNvdXJjZUxvYWRlci0+c2V0U2VuZENhbGxiYWNrUG9saWN5KFNlbmRDYWxsYmFj a3MpOworCisgICAgICAgIGlmIChyZXNvdXJjZUxvYWRlcikKKyAgICAgICAgICAgIHJlc291cmNl TG9hZGVyLT5zZXRTZW5kQ2FsbGJhY2tQb2xpY3koU2VuZENhbGxiYWNrcyk7CiAgICAgICAgIGhh bmRsZVN1YnN0aXR1dGVEYXRhTG9hZFNvb24oKTsKICAgICB9CiB9CmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV2ViQ29yZS9sb2FkZXIvY2FjaGUvQ2FjaGVkUmVzb3VyY2UuY3BwIGIvU291cmNlL1dlYkNv cmUvbG9hZGVyL2NhY2hlL0NhY2hlZFJlc291cmNlLmNwcAppbmRleCAzYmVmMjFlLi41YTQwNTli IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvY2FjaGUvQ2FjaGVkUmVzb3VyY2Uu Y3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9jYWNoZS9DYWNoZWRSZXNvdXJjZS5jcHAK QEAgLTQwNSw2ICs0MDUsNyBAQCB2b2lkIENhY2hlZFJlc291cmNlOjpyZXNwb25zZVJlY2VpdmVk KGNvbnN0IFJlc291cmNlUmVzcG9uc2UmIHJlc3BvbnNlKQogdm9pZCBDYWNoZWRSZXNvdXJjZTo6 Y2xlYXJMb2FkZXIoKQogewogICAgIEFTU0VSVChtX2xvYWRlcik7CisgICAgbV9pZGVudGlmaWVy Rm9yTG9hZFdpdGhvdXRSZXNvdXJjZUxvYWRlciA9IG1fbG9hZGVyLT5pZGVudGlmaWVyKCk7CiAg ICAgbV9sb2FkZXIgPSBudWxscHRyOwogICAgIGRlbGV0ZUlmUG9zc2libGUoKTsKIH0KZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9jYWNoZS9DYWNoZWRSZXNvdXJjZS5oIGIvU291 cmNlL1dlYkNvcmUvbG9hZGVyL2NhY2hlL0NhY2hlZFJlc291cmNlLmgKaW5kZXggYzI4MjE5Yy4u ZDZmOTNmNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL2NhY2hlL0NhY2hlZFJl c291cmNlLmgKKysrIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL2NhY2hlL0NhY2hlZFJlc291cmNl LmgKQEAgLTI2MCw2ICsyNjAsOCBAQCBwdWJsaWM6CiAgICAgdmlydHVhbCBjaGFyKiBnZXRPckNy ZWF0ZVJlYWRCdWZmZXIoc2l6ZV90IC8qIHJlcXVlc3RlZFNpemUgKi8sIHNpemVfdCYgLyogYWN0 dWFsU2l6ZSAqLykgeyByZXR1cm4gbnVsbHB0cjsgfQogI2VuZGlmCiAKKyAgICB1bnNpZ25lZCBs b25nIGlkZW50aWZpZXJGb3JMb2FkV2l0aG91dFJlc291cmNlTG9hZGVyKCkgY29uc3QgeyByZXR1 cm4gbV9pZGVudGlmaWVyRm9yTG9hZFdpdGhvdXRSZXNvdXJjZUxvYWRlcjsgfQorCiBwcm90ZWN0 ZWQ6CiAgICAgdm9pZCBzZXRFbmNvZGVkU2l6ZSh1bnNpZ25lZCk7CiAgICAgdm9pZCBzZXREZWNv ZGVkU2l6ZSh1bnNpZ25lZCk7CkBAIC0zNDEsNiArMzQzLDggQEAgcHJpdmF0ZToKICAgICBIYXNo U2V0PENhY2hlZFJlc291cmNlSGFuZGxlQmFzZSo+IG1faGFuZGxlc1RvUmV2YWxpZGF0ZTsKIAog ICAgIFJlZGlyZWN0Q2hhaW5DYWNoZVN0YXR1cyBtX3JlZGlyZWN0Q2hhaW5DYWNoZVN0YXR1czsK KworICAgIHVuc2lnbmVkIGxvbmcgbV9pZGVudGlmaWVyRm9yTG9hZFdpdGhvdXRSZXNvdXJjZUxv YWRlciB7IDAgfTsKIH07CiAKIGNsYXNzIENhY2hlZFJlc291cmNlOjpDYWxsYmFjayB7Cg==