12106 2007-01-03 15:41:29 -0800 Changed URL from WebResourceLoadDelegate's webView:resource:willSendRequest:... is ignored 2007-01-07 03:55:45 -0800 1 1 1 Unclassified WebKit Page Loading 420+ Mac OS X 10.4 RESOLVED FIXED Regression P2 Major --- 1 dwood Graham.Dennis rwlbuis oldest_to_newest 37250 0 dwood 2007-01-03 15:41:29 -0800 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; } 36991 1 12233 Graham.Dennis 2007-01-04 21:01:00 -0800 Created attachment 12233 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. 36627 2 12233 darin 2007-01-06 18:11:03 -0800 Comment on attachment 12233 patch 1 Looks good. Maybe we should put a WebResourceLoadDelegate into DumpRenderTree so we can test this. r=me 36579 3 ap 2007-01-07 03:55:45 -0800 Committed by Rob in revision 18646. 12233 2007-01-04 21:01:00 -0800 2007-01-06 18:11:03 -0800 patch 1 patch-12106-v1.diff text/plain 2359 Graham.Dennis SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAxODYwNikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDctMDEtMDUgIEdyYWhhbSBEZW5uaXMgIDxncmFoYW0uZGVubmlz QGdtYWlsLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBQYXRjaCBmb3IgaHR0cDovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTIx MDYKKyAgICAgICAgQ2hhbmdlZCBVUkwgZnJvbSBXZWJSZXNvdXJjZUxvYWREZWxlZ2F0ZSdzIHdl YlZpZXc6cmVzb3VyY2U6d2lsbFNlbmRSZXF1ZXN0Oi4uLiBpcyBpZ25vcmVkCisKKyAgICAgICAg KiBsb2FkZXIvbWFjL1N1YnJlc291cmNlTG9hZGVyTWFjLm1tOgorICAgICAgICAoV2ViQ29yZTo6 U3VicmVzb3VyY2VMb2FkZXI6OndpbGxTZW5kUmVxdWVzdCk6IFVzZSBtX29yaWdpbmFsVVJMIGlu c3RlYWQgb2YgcmVxdWVzdCgpLnVybCgpCisgICAgICAgIGFzIG1fcmVxdWVzdCBpcyBub3Qgc2V0 IHVudGlsIHRoZSBlbmQgb2YgUmVzb3VyY2VMb2FkZXI6OndpbGxTZW5kUmVxdWVzdC4gQWxzbywg ZW5zdXJlIHRoYXQKKyAgICAgICAgdGhlIHJlcXVlc3QgaXMgdXBkYXRlZCBpZiB0aGUgZnJhbWUn cyBjbGllbnQgY2hhbmdlcyBpdCwgYW5kIHRoYXQgdGhlIFN1YnJlc291cmNlTG9hZGVyJ3MgY2xp ZW50CisgICAgICAgIGdldHMgdGhlIG5ldyByZXF1ZXN0IGFuZCBub3QgdGhlIG9sZCBvbmUuIAor CiAyMDA3LTAxLTA0ICBBbmRlcnMgQ2FybHNzb24gIDxhY2FybHNzb25AYXBwbGUuY29tPgogCiAg ICAgICAgIFJldmlld2VkIGJ5IEFkYW0uCkluZGV4OiBXZWJDb3JlL2xvYWRlci9tYWMvU3VicmVz b3VyY2VMb2FkZXJNYWMubW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9sb2FkZXIvbWFjL1N1YnJl c291cmNlTG9hZGVyTWFjLm1tCShyZXZpc2lvbiAxODYwNikKKysrIFdlYkNvcmUvbG9hZGVyL21h Yy9TdWJyZXNvdXJjZUxvYWRlck1hYy5tbQkod29ya2luZyBjb3B5KQpAQCAtMSw1ICsxLDYgQEAK IC8qCiAgKiBDb3B5cmlnaHQgKEMpIDIwMDUsIDIwMDYgQXBwbGUgQ29tcHV0ZXIsIEluYy4gIEFs bCByaWdodHMgcmVzZXJ2ZWQuCisgKiAgICAgICAgICAgKEMpIDIwMDcgR3JhaGFtIERlbm5pcyAo Z3JhaGFtLmRlbm5pc0BnbWFpbC5jb20pCiAgKgogICogUmVkaXN0cmlidXRpb24gYW5kIHVzZSBp biBzb3VyY2UgYW5kIGJpbmFyeSBmb3Jtcywgd2l0aCBvciB3aXRob3V0CiAgKiBtb2RpZmljYXRp b24sIGFyZSBwZXJtaXR0ZWQgcHJvdmlkZWQgdGhhdCB0aGUgZm9sbG93aW5nIGNvbmRpdGlvbnMK QEAgLTEwNiwxOCArMTA3LDkgQEAgUGFzc1JlZlB0cjxTdWJyZXNvdXJjZUxvYWRlcj4gU3VicmVz b3VyYwogCiB2b2lkIFN1YnJlc291cmNlTG9hZGVyOjp3aWxsU2VuZFJlcXVlc3QoUmVzb3VyY2VS ZXF1ZXN0JiBuZXdSZXF1ZXN0LCBjb25zdCBSZXNvdXJjZVJlc3BvbnNlJiByZWRpcmVjdFJlc3Bv bnNlKQogewotICAgIEtVUkwgb2xkVVJMID0gcmVxdWVzdCgpLnVybCgpOwotICAgIAotICAgIFJl c291cmNlUmVxdWVzdCBjbGllbnRSZXF1ZXN0KG5ld1JlcXVlc3QpOwotICAgIFJlc291cmNlTG9h ZGVyOjp3aWxsU2VuZFJlcXVlc3QoY2xpZW50UmVxdWVzdCwgcmVkaXJlY3RSZXNwb25zZSk7Ci0g ICAgaWYgKCFjbGllbnRSZXF1ZXN0LmlzTnVsbCgpICYmIG9sZFVSTCAhPSBjbGllbnRSZXF1ZXN0 LnVybCgpKSB7Ci0gICAgICAgIGlmIChtX2NsaWVudCkKLSAgICAgICAgICAgIG1fY2xpZW50LT53 aWxsU2VuZFJlcXVlc3QodGhpcywgbmV3UmVxdWVzdCwgcmVkaXJlY3RSZXNwb25zZSk7Ci0gICAg ICAgIAotICAgICAgICByZXR1cm47Ci0gICAgfQotICAgIAotICAgIG5ld1JlcXVlc3QgPSBjbGll bnRSZXF1ZXN0OworICAgIFJlc291cmNlTG9hZGVyOjp3aWxsU2VuZFJlcXVlc3QobmV3UmVxdWVz dCwgcmVkaXJlY3RSZXNwb25zZSk7CisgICAgaWYgKCFuZXdSZXF1ZXN0LmlzTnVsbCgpICYmIG1f b3JpZ2luYWxVUkwgIT0gbmV3UmVxdWVzdC51cmwoKSAmJiBtX2NsaWVudCkKKyAgICAgICAgbV9j bGllbnQtPndpbGxTZW5kUmVxdWVzdCh0aGlzLCBuZXdSZXF1ZXN0LCByZWRpcmVjdFJlc3BvbnNl KTsKIH0KIAogdm9pZCBTdWJyZXNvdXJjZUxvYWRlcjo6ZGlkUmVjZWl2ZVJlc3BvbnNlKGNvbnN0 IFJlc291cmNlUmVzcG9uc2UmIHIpCg==