DocumentThreadableLoader::redirectReceived() should not rely on the resource's loader. The rest of the methods do not.
It is unsafe it rely on the resource's loader because it gets cleared when the load completes. A resource may be reused from the memory cache once its load has completed.
This may cause crashes in CachedRawResource::didAddClient() when replaying the redirects because it would call DocumentThreadableLoader::redirectReceived() and potentially not have a loader anymore.
Created attachment 311192 [details]
Comment on attachment 311192 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=311192&action=review
> +void DocumentLoader::redirectReceived(CachedResource& resource, ResourceRequest& request, const ResourceResponse& redirectResponse, SecurityOrigin*)
I would go with some kind of enum/boolean flag.
> + WEBCORE_EXPORT void redirectReceived(CachedResource&, ResourceRequest&, const ResourceResponse&, SecurityOrigin* newOrigin) override;
DocumentLoader should probably made final if not already and the same for redirectReceived.
> + m_origin = newOrigin;
We would create a new unique origin if passed a boolean/enum.
> + m_options.maxRedirectCount -= m_resource->redirectCount();
Is this correct in case of memory cached resources?
Maybe we should just decrement m_options.maxRedirectCount on each redirectReceived call.
> + void redirectReceived(CachedResource&, ResourceRequest&, const ResourceResponse&, SecurityOrigin*) override;
Ditto for final.
Looks like I have a test case, I’ll clean it up and include it in my next iteration.
Created attachment 311240 [details]
Created attachment 311250 [details]
Fails a couple of tests:
Regressions: Unexpected text-only failures (2)
http/tests/security/isolatedWorld/bypass-worker-csp-for-xhr-redirect-cross-origin.html [ Failure ]
http/tests/xmlhttprequest/redirect-cors-origin-null.html [ Failure ]
I'll investigate why.
Comment on attachment 311250 [details]
Attachment 311250 [details] did not pass mac-wk2-ews (mac-wk2):
New failing tests:
Created attachment 311258 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 311259 [details]
Comment on attachment 311259 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=311259&action=review
> + m_options.maxRedirectCount -= m_redirectCount;
Shouldn't m_redirectCount be reset to zero here?
Can you file a bugzilla about beefing up cached redirection tests?
Created attachment 311276 [details]
Comment on attachment 311276 [details]
Clearing flags on attachment: 311276
Committed r217445: <http://trac.webkit.org/changeset/217445>
All reviewed patches have been landed. Closing bug.