Summary: | Pass the resource identifier in willSendRequest callback; and pass pageIsProvisionallyLoading flag in didInitiateLoadForResource. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yongjun Zhang <yongjun_zhang> | ||||||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, mitz, sam, thorton, yongjun_zhang | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Yongjun Zhang
2014-08-27 17:29:02 PDT
Created attachment 237274 [details]
Patch.
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.
Created attachment 237275 [details]
Fix style issue.
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? (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. Created attachment 237389 [details]
Revise patch; falls back to old delegate methods if the new ones are not implemented by a client.
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.
Created attachment 237513 [details]
Fi style issues.
Comment on attachment 237513 [details] Fi style issues. Clearing flags on attachment: 237513 Committed r173191: <http://trac.webkit.org/changeset/173191> All reviewed patches have been landed. Closing bug. |