Changed URL from WebResourceLoadDelegate's webView:resource:willSendRequest:... is ignored

• Component: Page Loading
• Status: RESOLVED FIXED

[Dan Wood]

If you have a WebResourceLoadDelegate that defines webView:resource:willSendRequest:redirectResponse:fromDataSource: to return a new NSURLRequest, that new URL request is ignored, and the original URL is used.

I believe that I have traced this to this method.

When this is called, newRequest is <NSMutableURLRequest applewebdata://85A64C16-16E4-4262-909B-109A07DB268C/_Media/placeholder_large.jpeg>

My delegate code, called downstream from ResourceLoader::willSendRequest, sets clientRequest to <NSURLRequest media:/11D16F2645B64AB190E6/placeholder_large.jpeg>

It's not clear to me whether the "if" branch should be taken and there is a bug there, or if there is a bug in the test logic, or if there is a bug in that oldURL is set to nil, messing up the test.

NSURLRequest *SubresourceLoader::willSendRequest(NSURLRequest *newRequest, NSURLResponse *redirectResponse)
{
    NSURL *oldURL = [request() URL];   // NOTE: THIS RETURNS NIL -- I DON'T KNOW WHY
    NSURLRequest *clientRequest = ResourceLoader::willSendRequest(newRequest, redirectResponse);
    if (clientRequest && oldURL != [clientRequest URL] && ![oldURL isEqual:[clientRequest URL]]) {
        ResourceRequest request = newRequest;
        if (m_client)
            m_client->willSendRequest(this, request, redirectResponse);
        
        return request.nsURLRequest();
    }
    
    return clientRequest;
}

[Graham Dennis]

Created attachment 12233 [details]
patch 1

Patch 1.
Passes layout tests, but no layout test added, as to test this, you need to be able to create a WebResourceLoadDelegate.

The problem was that m_request isn't set until the end of ResourceLoader::willSendRequest(), and so oldURL was always empty. But even if we replace oldURL with m_originalURL (which is set at the start of ResourceLoader::load()), the newRequest is only updated if the new and old URLs are the same. This patch fixes both problems so that WebResourceLoadDelegates can now change URLs.

[Darin Adler]

Comment on attachment 12233 [details]
patch 1

Looks good. Maybe we should put a WebResourceLoadDelegate into DumpRenderTree so we can test this.

r=me

[Alexey Proskuryakov]

Committed by Rob in revision 18646.