Bug 192473 – WKWebProcessPlugInLoadDelegate should have API for saying which rendering events it wants to listen for

• Component: WebKit2
• Status: RESOLVED FIXED

[Saam Barati]

...

[Saam Barati]

(In reply to Saam Barati from comment #0)
> ...

Patch forthcoming

[Saam Barati]

Created attachment 356765 [details]
patch

[Andy Estes]

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:.

[Saam Barati]

(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:.

👍🏼

[Saam Barati]

Created attachment 356771 [details]
patch

[Andy Estes]

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 *

[Saam Barati]

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.

[Saam Barati]

Created attachment 356831 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 356831 [details]
patch for landing

Clearing flags on attachment: 356831

Committed r238969: <https://trac.webkit.org/changeset/238969>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46565296>

[Alex Christensen]

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.

[Saam Barati]

(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