[Webkit-unassigned] [Bug 170362] Add SPI for handling geolocation authorization requests

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 5 12:49:06 PDT 2017


https://bugs.webkit.org/show_bug.cgi?id=170362

--- Comment #8 from Alex Christensen <achristensen at apple.com> ---
(In reply to David Quesada from comment #7)
> (In reply to Alex Christensen from comment #6)
> > Comment on attachment 306015 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=306015&action=review
> > 
> > Test failure seems unrelated.  This looks good.  It seems like we had a
> > synchronous callback and this adds an asynchronous one.
> 
> As far as I know, nothing uses the synchronous method (specifically, it is
> not being used for the purpose for which it was added), so I would like to
> get rid of it. But removing SPI probably isn't something I can just bundle
> in with another change.
If we're sure nothing uses it, we can remove the calls to it and mark it as dead somehow.
> 
> > You should make an API test that opens a page that requests geolocation data
> > and asynchronously call the decisionHandler with a callOnMainThread in the
> > delegate callback that calls the decisionHandler.  Then we can do things
> > like test the behavior if the delegate callback isn't there, test the
> > behavior if we call the callback twice, make sure there are no
> > use-after-free bugs on the asan bots, make sure the expected behavior
> > happens in JavaScript when a request is accepted and denied, etc.
> 
> I can look into doing this. I wasn't sure API tests would work
> out-of-the-box for this, since we would possibly have to either (1) entitle
> TestWebKitAPI for location services access or (2) expose more about
> WKGeolocationProviderIOS and WebGeolocationCoreLocationProvider so the API
> tests can configure WKGeolocationProviderIOS to go through its motions
> without actually touching CoreLocation.
> 
> If we didn't make either of these changes, the API tests would fail or get
> stuck at the system location prompt when TestWebKitAPI tries to request
> location permission. If we only did (1), there may be flakiness when running
> the tests if geolocation isn’t available on the machine running the
> simulator and tests. (2) is more architectural work than I would have liked
> to do for this, but I’ll make these changes if this is the best approach.
> What are your thoughts?
DumpRenderTree has a MockGeolocationProvider.  We should use something similar, or even move that to WebCoreTestSupport.
> 
> > 
> > > Source/WebKit2/UIProcess/ios/WKGeolocationProviderIOS.mm:185
> > > +            URL requestFrameURL(URL(), request.frame->url());
> > 
> > Can't we just call the copy constructor here?
> 
> Sorry, I lazily copied this from the synchronous delegate call. That would
> make it just "URL requestFrameURL(request.frame->url());" , right?
auto requestFrameURL = request.frame->url();
I guess that's the operator=, not the copy constructor.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170405/f22ebf3d/attachment-0001.html>


More information about the webkit-unassigned mailing list