Bug 27853 – Make Chrome::requestGeolocationPermissionForFrame() into a synchronous call

• Component: WebCore Misc.
• Status: RESOLVED WONTFIX

[Greg Bolsinga]

It is currently in WebKit as an asynchronous call, but that was bogus. To make iPhone's implementation, it should be synchronous.

[Greg Bolsinga]

Created attachment 33841 [details]
patch to implement bug fix

[Greg Bolsinga]

Make that "to match iPhone's implementation".

[George Staikos]

Please also see 26993, waiting for quite some time for review.

[Adam Barth]

What is the technical advantage of having synchronous call?

[Greg Bolsinga]

Well, it simply doesn't make much sense to have it be asynchronous. WebCore is all on one thread, and the UI is the same thread.

[Greg Bolsinga]

(In reply to comment #3)
> Please also see 26993, waiting for quite some time for review.

Bug 26993 will be obsolete if this change is taken.

[Adam Barth]

(In reply to comment #5)
> Well, it simply doesn't make much sense to have it be asynchronous. WebCore is
> all on one thread, and the UI is the same thread.

WebCore doesn't have any UI.  The UI is handled by the WebKit port.  Not all ports are single threaded.  For example, the Chromium port is multiprocess.

[Yong Li]

(In reply to comment #6)
> (In reply to comment #3)
> > Please also see 26993, waiting for quite some time for review.
> 
> Bug 26993 will be obsolete if this change is taken.

The patch for Bug 26993 simply makes it work for both async and sync cases. It's up to the implementation of m_client->requestGeolocationPermissionForFrame()

[Greg Bolsinga]

(In reply to comment #7)
> ports are single threaded.  For example, the Chromium port is multiprocess.

Nothing else in WebCore is asynchronous on this type of stuff. The ChomeClient object is UI.

[Adam Barth]

> The patch for Bug 26993 simply makes it work for both async and sync cases.
> It's up to the implementation of
> m_client->requestGeolocationPermissionForFrame()

That sounds like a better fix.

[Adam Barth]

*** This bug has been marked as a duplicate of bug 26993 ***

[Greg Bolsinga]

(In reply to comment #11)
> 
> *** This bug has been marked as a duplicate of bug 26993 ***

I disagree with this.

[Adam Barth]

> I disagree with this.

Why not make the API work for both synchronous and asynchronous use cases?  For example, in Firefox, this UI is handled by a non-modal infobar.

[Greg Bolsinga]

(In reply to comment #13)
> > I disagree with this.
> 
> Why not make the API work for both synchronous and asynchronous use cases?  For
> example, in Firefox, this UI is handled by a non-modal infobar.

Is there another example in WebCore I can follow? If not, I'd prefer to keep it synchronous, like all other ChromeClient calls I can find.

[Greg Bolsinga]

(In reply to comment #14)
> (In reply to comment #13)
> > > I disagree with this.
> > 
> > Why not make the API work for both synchronous and asynchronous use cases?  For
> > example, in Firefox, this UI is handled by a non-modal infobar.
> 
> Is there another example in WebCore I can follow? If not, I'd prefer to keep it
> synchronous, like all other ChromeClient calls I can find.

This way the ChromeClient can handle the asynchronousness itself if it needs it?

[Adam Barth]

(In reply to comment #15)
> This way the ChromeClient can handle the asynchronousness itself if it needs
> it?

Yes.  I believe after the patch in Bug 26993, you can call just call Geolocation::setIsAllowed synchronously if you like.

[Sam Weinig]

I think we should keep this asynchronous from WebCores prospective and fix any issues specific to the iPhone at a different level.

[Greg Bolsinga]

OK. This will be the first. Thanks!

[Eric Seidel (no email)]

Comment on attachment 33841 [details]
patch to implement bug fix

Clearing review flag.