Bug 12106 - Changed URL from WebResourceLoadDelegate's webView:resource:willSendRequest:... is ignored
Summary: Changed URL from WebResourceLoadDelegate's webView:resource:willSendRequest:....
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Major
Assignee: Graham Dennis
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2007-01-03 15:41 PST by Dan Wood
Modified: 2007-01-07 03:55 PST (History)
1 user (show)

See Also:


Attachments
patch 1 (2.30 KB, patch)
2007-01-04 21:01 PST, Graham Dennis
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Wood 2007-01-03 15:41:29 PST
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;
}
Comment 1 Graham Dennis 2007-01-04 21:01:00 PST
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.
Comment 2 Darin Adler 2007-01-06 18:11:03 PST
Comment on attachment 12233 [details]
patch 1

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

r=me
Comment 3 Alexey Proskuryakov 2007-01-07 03:55:45 PST
Committed by Rob in revision 18646.