Summary: | WKWebProcessPlugInLoadDelegate should have API for saying which rendering events it wants to listen for | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||
Component: | WebKit2 | Assignee: | Saam Barati <saam> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, aestes, beidson, cdumez, commit-queue, ggaren, koivisto, rniwa, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Saam Barati
2018-12-06 13:08:32 PST
(In reply to Saam Barati from comment #0) > ... Patch forthcoming Created attachment 356765 [details]
patch
Comment on attachment 356765 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=356765&action=review > Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:56 > +- (_WKRenderingProgressEvents)getLayoutMilestonesWithWebProcessPlugInBrowserContextController:(WKWebProcessPlugInBrowserContextController*)controller; The layout milestones / rendering progress events terminology mismatch is a little strange here. But assuming you keep "layout milestones" term, the right name for this selector would be -webProcessPlugInBrowserContextControllerLayoutMilestones:. (In reply to Andy Estes from comment #3) > Comment on attachment 356765 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=356765&action=review > > > Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:56 > > +- (_WKRenderingProgressEvents)getLayoutMilestonesWithWebProcessPlugInBrowserContextController:(WKWebProcessPlugInBrowserContextController*)controller; > > The layout milestones / rendering progress events terminology mismatch is a > little strange here. Good point. I forgot to change the name to reflect the type when I changed the type. Will update. > > But assuming you keep "layout milestones" term, the right name for this > selector would be -webProcessPlugInBrowserContextControllerLayoutMilestones:. 👍🏼 Created attachment 356771 [details]
patch
Comment on attachment 356771 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=356771&action=review > Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:56 > +- (_WKRenderingProgressEvents)webProcessPlugInBrowserContextControllerRenderingProgressEvents:(WKWebProcessPlugInBrowserContextController*)controller; WKWebProcessPlugInBrowserContextController * Comment on attachment 356771 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=356771&action=review >> Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:56 >> +- (_WKRenderingProgressEvents)webProcessPlugInBrowserContextControllerRenderingProgressEvents:(WKWebProcessPlugInBrowserContextController*)controller; > > WKWebProcessPlugInBrowserContextController * Will fix. Thanks for the review. Created attachment 356831 [details]
patch for landing
Comment on attachment 356831 [details] patch for landing Clearing flags on attachment: 356831 Committed r238969: <https://trac.webkit.org/changeset/238969> All reviewed patches have been landed. Closing bug. Comment on attachment 356831 [details] patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=356831&action=review > Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:56 > +- (_WKRenderingProgressEvents)webProcessPlugInBrowserContextControllerRenderingProgressEvents:(WKWebProcessPlugInBrowserContextController *)controller; I think this should've been completion handler based. There's no reason this needs to be synchronous. (In reply to Alex Christensen from comment #12) > Comment on attachment 356831 [details] > patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=356831&action=review > > > Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:56 > > +- (_WKRenderingProgressEvents)webProcessPlugInBrowserContextControllerRenderingProgressEvents:(WKWebProcessPlugInBrowserContextController *)controller; > > I think this should've been completion handler based. There's no reason > this needs to be synchronous. That’s not right. WebKit calls this expecting a response |