<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#c7">Comment # 7</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:david_quesada@apple.com" title="David Quesada <david_quesada@apple.com>"> <span class="fn">David Quesada</span></a>
</span></b>
<pre>(In reply to Alex Christensen from <a href="show_bug.cgi?id=170362#c6">comment #6</a>)
<span class="quote">> 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.</span >
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 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.</span >
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 class="quote">>
> > Source/WebKit2/UIProcess/ios/WKGeolocationProviderIOS.mm:185
> > + URL requestFrameURL(URL(), request.frame->url());
>
> Can't we just call the copy constructor here?</span >
Sorry, I lazily copied this from the synchronous delegate call. That would make it just "URL requestFrameURL(request.frame->url());" , right?</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>