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+

Andy Estes
Reported 2012-11-08 17:31:55 PST
[WebKit2] Need API in UIProcess to enable loading of custom protocols
Attachments
Patch (79.23 KB, patch)
2012-11-08 18:02 PST, Andy Estes 		no flags
DetailsFormatted DiffDiff
Patch (79.02 KB, patch)
2012-11-13 17:18 PST, Andy Estes 		no flags
DetailsFormatted DiffDiff
Patch (78.99 KB, patch)
2012-11-13 18:10 PST, Andy Estes 		beidson: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Andy Estes
Comment 1 2012-11-08 18:02:17 PST
Created attachment 173163 [details] Patch
Brady Eidson
Comment 2 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?
Andy Estes
Comment 3 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.
Andy Estes
Comment 4 2012-11-13 17:18:16 PST
Created attachment 174034 [details] Patch rebase to tip of tree
Early Warning System Bot
Comment 5 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
EFL EWS Bot
Comment 6 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
Build Bot
Comment 7 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
Andy Estes
Comment 8 2012-11-13 18:10:12 PST
Created attachment 174045 [details] Patch
Brady Eidson
Comment 9 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.
Andy Estes
Comment 10 2012-11-14 15:30:09 PST
Committed r134681: <http://trac.webkit.org/changeset/134681>
Alexey Proskuryakov
Comment 11 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.
Andy Estes
Comment 12 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.
Note You need to log in before you can comment on or make changes to this bug.