Summary: | [WebKit2] Need API in UIProcess to enable loading of custom protocols | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andy Estes <aestes> | ||||||||
Component: | New Bugs | Assignee: | 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
Andy Estes
2012-11-08 17:31:55 PST
Created attachment 173163 [details]
Patch
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?
(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. Created attachment 174034 [details]
Patch
rebase to tip of tree
Comment on attachment 174034 [details] Patch Attachment 174034 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14837051 Comment on attachment 174034 [details] Patch Attachment 174034 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14824497 Comment on attachment 174034 [details] Patch Attachment 174034 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14828221 Created attachment 174045 [details]
Patch
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.
Committed r134681: <http://trac.webkit.org/changeset/134681> 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. (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. |