Bug 123658 – [WK2] Update WebGeolocationManager to support multiple WebContent processes

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

[Benjamin Poulain]

[WK2] Update WebGeolocationManager to support multiple WebContent processes

[Benjamin Poulain]

Created attachment 215797 [details]
Patch

[Anders Carlsson]

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]

(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]

Committed r158600: <http://trac.webkit.org/changeset/158600>