<html>
<head>
<base href="https://bugs.webkit.org/">
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Add SPI for handling geolocation authorization requests"
href="https://bugs.webkit.org/show_bug.cgi?id=170362#c8">Comment # 8</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Add SPI for handling geolocation authorization requests"
href="https://bugs.webkit.org/show_bug.cgi?id=170362">bug 170362</a>
from <span class="vcard"><a class="email" href="mailto:achristensen@apple.com" title="Alex Christensen <achristensen@apple.com>"> <span class="fn">Alex Christensen</span></a>
</span></b>
<pre>(In reply to David Quesada from <a href="show_bug.cgi?id=170362#c7">comment #7</a>)
<span class="quote">> (In reply to Alex Christensen from <a href="show_bug.cgi?id=170362#c6">comment #6</a>)
> > Comment on <span class=""><a href="attachment.cgi?id=306015&action=diff" name="attach_306015" title="Patch">attachment 306015</a> <a href="attachment.cgi?id=306015&action=edit" title="Patch">[details]</a></span>
> > Patch
> >
> > View in context:
> > <a href="https://bugs.webkit.org/attachment.cgi?id=306015&action=review">https://bugs.webkit.org/attachment.cgi?id=306015&action=review</a>
> >
> > 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.</span >
If we're sure nothing uses it, we can remove the calls to it and mark it as dead somehow.
<span class="quote">>
> > 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?</span >
DumpRenderTree has a MockGeolocationProvider. We should use something similar, or even move that to WebCoreTestSupport.
<span class="quote">>
> >
> > > 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?</span >
auto requestFrameURL = request.frame->url();
I guess that's the operator=, not the copy constructor.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>