[webkit-reviews] review denied: [Bug 93977] Implement testRunner.numberOfPendingGeolocationPermissionRequests() : [Attachment 203964] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied  review:
Bug 93977: Implement testRunner.numberOfPendingGeolocationPermissionRequests()
https://bugs.webkit.org/show_bug.cgi?id=93977

Attachment 203964: Patch
https://bugs.webkit.org/attachment.cgi?id=203964&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=203964&action=review


This is wrong.

I think you missed what this test does.

On reload, WebKit should call GeolocationClient::cancelPermissionRequest()
which should in turn cancel the permissionRequest on the UIProcess side. This
part is not wired to anything which is why that does not work.
You are just breaking the test to make it pass.

> LayoutTests/ChangeLog:4
> +
> +	   The testsRunner should request for Geolocation Permissions.
> +	   Unskip test.

The description goes under the title. See the other changelogs.

>
LayoutTests/fast/dom/Geolocation/script-tests/page-reload-cancel-permission-req
uests.js:15
> +    if (!isReload) {
> +	 testRunner.setGeolocationPermission(true);
> +    }

Wrong indent.

You are breaking the test by doing this. Why do you do that?