Bug 136325 – Pass the resource identifier in willSendRequest callback; and pass pageIsProvisionallyLoading flag in didInitiateLoadForResource.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Yongjun Zhang]

The delegate method webProcessPlugInBrowserContextController:frame:willSendRequest:redirectResponse: in WKWebProcessPlugInLoadDelegate doesn't pass the resource identifier, we should add it back, like we did with didInitialLoadForResource: method.

[Yongjun Zhang]

Created attachment 237274 [details]
Patch.

[WebKit Commit Bot]

Attachment 237274 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:234:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:236:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:237:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 3 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yongjun Zhang]

Created attachment 237275 [details]
Fix style issue.

[Darin Adler]

Comment on attachment 237275 [details]
Fix style issue.

View in context: https://bugs.webkit.org/attachment.cgi?id=237275&action=review

> Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:54
> +- (NSURLRequest *)webProcessPlugInBrowserContextController:(WKWebProcessPlugInBrowserContextController *)controller frame:(WKWebProcessPlugInFrame *)frame willSendRequestForResource:(uint64_t)resource request:(NSURLRequest *)request redirectResponse:(NSURLResponse *)redirectResponse;

How can we just change a protocol like this in public API? Don’t we need to keep existing clients compatible? I think there’s a pattern we need to follow here to deprecate an old method signature and replace it with a new one, calling the old one if the new one isn’t present.

Or maybe this isn’t public API yet?

[Yongjun Zhang]

(In reply to comment #4)
> (From update of attachment 237275 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237275&action=review
> 
> > Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:54
> > +- (NSURLRequest *)webProcessPlugInBrowserContextController:(WKWebProcessPlugInBrowserContextController *)controller frame:(WKWebProcessPlugInFrame *)frame willSendRequestForResource:(uint64_t)resource request:(NSURLRequest *)request redirectResponse:(NSURLResponse *)redirectResponse;
> 
> How can we just change a protocol like this in public API? Don’t we need to keep existing clients compatible? I think there’s a pattern we need to follow here to deprecate an old method signature and replace it with a new one, calling the old one if the new one isn’t present.
> 
> Or maybe this isn’t public API yet?

No, this is not public API yet and it is subject to change.  That said, I think it is better to follow the staging pattern that falls back to old signature if the new method isn't implemented by the delegate.  And we can remove the check when all client are moving to the new method.

[Yongjun Zhang]

Created attachment 237389 [details]
Revise patch; falls back to old delegate methods if the new ones are not implemented by a client.

[WebKit Commit Bot]

Attachment 237389 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:234:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:236:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:237:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:244:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:245:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:246:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:263:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:265:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 8 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yongjun Zhang]

Created attachment 237513 [details]
Fi style issues.

[WebKit Commit Bot]

Comment on attachment 237513 [details]
Fi style issues.

Clearing flags on attachment: 237513

Committed r173191: <http://trac.webkit.org/changeset/173191>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.