Bug 123658

Summary: [WK2] Update WebGeolocationManager to support multiple WebContent processes
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch andersca: review+

Benjamin Poulain
Reported 2013-11-01 23:06:01 PDT
[WK2] Update WebGeolocationManager to support multiple WebContent processes
Attachments
Patch (7.41 KB, patch)
2013-11-01 23:10 PDT, Benjamin Poulain
andersca: review+
Benjamin Poulain
Comment 1 2013-11-01 23:10:12 PDT
Anders Carlsson
Comment 2 2013-11-04 14:26:10 PST
Comment on attachment 215797 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=215797&action=review > Source/WebKit2/UIProcess/WebContext.cpp:685 > + static_cast<WebContextSupplement*>(supplement<WebGeolocationManagerProxy>())->processDidClose(process); Don’t you want to call this on all the WebContextSupplements? > Source/WebKit2/UIProcess/WebGeolocationManagerProxy.h:72 > + HashSet<const CoreIPC::Connection::Client*> m_updateRequesters; Instead of using a HashSet, could you simply have a counter that keeps track of the number of requests?
Benjamin Poulain
Comment 3 2013-11-04 14:40:46 PST
(In reply to comment #2) > (From update of attachment 215797 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=215797&action=review > > > Source/WebKit2/UIProcess/WebContext.cpp:685 > > + static_cast<WebContextSupplement*>(supplement<WebGeolocationManagerProxy>())->processDidClose(process); > > Don’t you want to call this on all the WebContextSupplements? The other WebContextSupplement do not support multiple WebProcess. They track the state as if there was a single process communicating with them. As a result, they do very bad things on processDidClose(). (I filed <rdar://problem/15376383>) > > Source/WebKit2/UIProcess/WebGeolocationManagerProxy.h:72 > > + HashSet<const CoreIPC::Connection::Client*> m_updateRequesters; > > Instead of using a HashSet, could you simply have a counter that keeps track of the number of requests? The HashSet is a defensive mechanism against mistake in the WebProcess. If for any reason the GeolocationController starts or stops querying twice, the HashSet ensure the state on the UIProcess remains valid.
Benjamin Poulain
Comment 4 2013-11-04 15:01:23 PST
Note You need to log in before you can comment on or make changes to this bug.