Bug 101674 – [WebKit2] Need API in UIProcess to enable loading of custom protocols

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andy Estes]

[WebKit2] Need API in UIProcess to enable loading of custom protocols

[Andy Estes]

Created attachment 173163 [details]
Patch

[Brady Eidson]

Comment on attachment 173163 [details]
Patch

This is a large patch.  I'm not sure if I've meaningfully reviewed it.  The description of the approach seems great and I'm glad I don't have to ask how you tested..  Could WKCustomProtocol and WKCustomProtocolLoader be in separate files?

[Andy Estes]

(In reply to comment #2)
> (From update of attachment 173163 [details])
Could WKCustomProtocol and WKCustomProtocolLoader be in separate files?

They could be. I chose not to put them in separate files since they felt like implementation details of the CustomProtocolManager and CustomProtocolManagerProxy and had no use outside of those contexts.

[Andy Estes]

Created attachment 174034 [details]
Patch

rebase to tip of tree

[Early Warning System Bot]

Comment on attachment 174034 [details]
Patch

Attachment 174034 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14837051

[EFL EWS Bot]

Comment on attachment 174034 [details]
Patch

Attachment 174034 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14824497

[Build Bot]

Comment on attachment 174034 [details]
Patch

Attachment 174034 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14828221

[Andy Estes]

Created attachment 174045 [details]
Patch

[Brady Eidson]

Comment on attachment 174045 [details]
Patch

Andy assured me on IRC he'd split those classes into their own files before landing.

I've taken a slightly more detailed look.  As long as EWS signs off, so do I.

[Andy Estes]

Committed r134681: <http://trac.webkit.org/changeset/134681>

[Alexey Proskuryakov]

Comment on attachment 174045 [details]
Patch

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

> Source/WebKit2/UIProcess/mac/WebContextMac.mm:135
> +    m_customSchemeRegisteredObserver = [[NSNotificationCenter defaultCenter] addObserverForName:WebKit::SchemeForCustomProtocolRegisteredNotificationName object:nil queue:[NSOperationQueue currentQueue] usingBlock:^(NSNotification *notification) {

This code is run for every new WebProcess, so we end up registering a ton of listeners.

[Andy Estes]

(In reply to comment #11)
> (From update of attachment 174045 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174045&action=review
> 
> > Source/WebKit2/UIProcess/mac/WebContextMac.mm:135
> > +    m_customSchemeRegisteredObserver = [[NSNotificationCenter defaultCenter] addObserverForName:WebKit::SchemeForCustomProtocolRegisteredNotificationName object:nil queue:[NSOperationQueue currentQueue] usingBlock:^(NSNotification *notification) {
> 
> This code is run for every new WebProcess, so we end up registering a ton of listeners.

Oops, good catch. I'll fix to only register it once per WebContext.