Bug 101674

Summary: [WebKit2] Need API in UIProcess to enable loading of custom protocols
Product: WebKit Reporter: Andy Estes <aestes>
Component: New BugsAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, philn, webkit-ews, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch beidson: review+

Description Andy Estes 2012-11-08 17:31:55 PST
[WebKit2] Need API in UIProcess to enable loading of custom protocols
Comment 1 Andy Estes 2012-11-08 18:02:17 PST
Created attachment 173163 [details]
Patch
Comment 2 Brady Eidson 2012-11-13 14:20:33 PST
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?
Comment 3 Andy Estes 2012-11-13 16:31:12 PST
(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.
Comment 4 Andy Estes 2012-11-13 17:18:16 PST
Created attachment 174034 [details]
Patch

rebase to tip of tree
Comment 5 Early Warning System Bot 2012-11-13 17:26:10 PST
Comment on attachment 174034 [details]
Patch

Attachment 174034 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14837051
Comment 6 EFL EWS Bot 2012-11-13 17:32:39 PST
Comment on attachment 174034 [details]
Patch

Attachment 174034 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14824497
Comment 7 Build Bot 2012-11-13 17:53:15 PST
Comment on attachment 174034 [details]
Patch

Attachment 174034 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14828221
Comment 8 Andy Estes 2012-11-13 18:10:12 PST
Created attachment 174045 [details]
Patch
Comment 9 Brady Eidson 2012-11-13 19:04:30 PST
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.
Comment 10 Andy Estes 2012-11-14 15:30:09 PST
Committed r134681: <http://trac.webkit.org/changeset/134681>
Comment 11 Alexey Proskuryakov 2012-12-10 15:09:36 PST
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.
Comment 12 Andy Estes 2012-12-10 17:44:32 PST
(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.