Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-fully active doc test for Geolocation #29799

Merged
merged 10 commits into from
Jul 30, 2021
Merged

Conversation

marcoscaceres
Copy link
Contributor

@marcoscaceres marcoscaceres commented Jul 27, 2021

Test for:
w3c/geolocation#97

@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request besides its author. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

@marcoscaceres marcoscaceres requested review from reillyeon and removed request for yellowdoge July 29, 2021 00:58
geolocation-API/iframe.html Outdated Show resolved Hide resolved
geolocation-API/non-fully-active.https.html Outdated Show resolved Hide resolved
geolocation-API/non-fully-active.https.html Outdated Show resolved Hide resolved
geolocation-API/non-fully-active.https.html Outdated Show resolved Hide resolved
Co-authored-by: Reilly Grant <reillyeon@users.noreply.github.com>
@marcoscaceres marcoscaceres merged commit c85aa33 into master Jul 30, 2021
@marcoscaceres marcoscaceres deleted the geo-fully-active branch July 30, 2021 02:18
@marcoscaceres
Copy link
Contributor Author

Thanks for all the great suggestions @reillyeon! Much improved.

Looking forward to seeing how it's implemented in Chrome!

webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request Aug 24, 2021
https://bugs.webkit.org/show_bug.cgi?id=228319
<rdar://problem/81450315>

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

* web-platform-tests/geolocation-API/PositionOptions.https-expected.txt:
Rebaseline WPT test now that more checks are passing.

* web-platform-tests/geolocation-API/non-fully-active.https-expected.txt: Added.
* web-platform-tests/geolocation-API/non-fully-active.https.html: Added.
* web-platform-tests/geolocation-API/resources/iframe.html: Added.
Import test coverage from WPT web-platform-tests/wpt#29799.

* web-platform-tests/resources/testdriver-vendor.js:
(window.test_driver_internal.set_permission):
Add support for test_driver.set_permission("geolocation", "granted") so that Geolocation
WPT tests can run with our infrastructure.

Source/WebCore:

Test: imported/w3c/web-platform-tests/geolocation-API/non-fully-active.https.html

* Modules/geolocation/Geolocation.cpp:
(WebCore::Geolocation::getCurrentPosition):
(WebCore::Geolocation::watchPosition):
Schedule a task to call the error callback if the document is not fully active, as
per the specification:
- w3c/geolocation#96
- w3c/geolocation#97

* bindings/js/JSDOMConvertCallbacks.h:
(WebCore::Converter<IDLCallbackFunction<T>>::convert):
(WebCore::Converter<IDLCallbackInterface<T>>::convert):
Make sure we use the incumbent global object when constructing callback functions /
interfaces, as per the Web IDL specification:
- https://heycam.github.io/webidl/#es-callback-interface
- https://heycam.github.io/webidl/#es-callback-function
Without this, the geolocation API would be unable to call its error callback when in
a detached frame because the callback's global object would be the geolocation object's
global object.

* dom/IdleCallbackController.cpp:
(WebCore::IdleCallbackController::invokeIdleCallbacks):
Make sure we don't fire requestIdleCallback callbacks in detached iframes, to maintain
pre-existing behavior and keep layout tests passing. I had to make this change because
callback interfaces / functions are now using a different global object and can now
get called in cases when they previously couldn't.

* dom/TaskSource.h:
As Geolocation task source for the HTML5 event loop, as per the Geolocation
specification.

LayoutTests:

Update / rebaseline existing layout tests to reflect behavior change.

* fast/dom/Geolocation/callback-to-deleted-context-expected.txt:
* fast/dom/Geolocation/callback-to-deleted-context.html:
* fast/dom/Geolocation/disconnected-frame-already-expected.txt:
* fast/dom/Geolocation/disconnected-frame-already.html:
* fast/dom/Geolocation/disconnected-frame-expected.txt:
* fast/dom/Geolocation/disconnected-frame-permission-denied-expected.txt:
* fast/dom/Geolocation/disconnected-frame-permission-denied.html:
* fast/dom/Geolocation/disconnected-frame.html:
* fast/dom/Geolocation/resources/callback-to-deleted-context-inner1.html:
* fast/events/detached-svg-parent-window-events-expected.txt:
* fast/events/detached-svg-parent-window-events.html:


Canonical link: https://commits.webkit.org/240893@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@281520 268f45cc-cd09-0410-ab3c-d52691b4dbfc
bertogg pushed a commit to Igalia/webkit that referenced this pull request Sep 6, 2021
https://bugs.webkit.org/show_bug.cgi?id=228319
<rdar://problem/81450315>

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

* web-platform-tests/geolocation-API/PositionOptions.https-expected.txt:
Rebaseline WPT test now that more checks are passing.

* web-platform-tests/geolocation-API/non-fully-active.https-expected.txt: Added.
* web-platform-tests/geolocation-API/non-fully-active.https.html: Added.
* web-platform-tests/geolocation-API/resources/iframe.html: Added.
Import test coverage from WPT web-platform-tests/wpt#29799.

* web-platform-tests/resources/testdriver-vendor.js:
(window.test_driver_internal.set_permission):
Add support for test_driver.set_permission("geolocation", "granted") so that Geolocation
WPT tests can run with our infrastructure.

Source/WebCore:

Test: imported/w3c/web-platform-tests/geolocation-API/non-fully-active.https.html

* Modules/geolocation/Geolocation.cpp:
(WebCore::Geolocation::getCurrentPosition):
(WebCore::Geolocation::watchPosition):
Schedule a task to call the error callback if the document is not fully active, as
per the specification:
- w3c/geolocation#96
- w3c/geolocation#97

* bindings/js/JSDOMConvertCallbacks.h:
(WebCore::Converter<IDLCallbackFunction<T>>::convert):
(WebCore::Converter<IDLCallbackInterface<T>>::convert):
Make sure we use the incumbent global object when constructing callback functions /
interfaces, as per the Web IDL specification:
- https://heycam.github.io/webidl/#es-callback-interface
- https://heycam.github.io/webidl/#es-callback-function
Without this, the geolocation API would be unable to call its error callback when in
a detached frame because the callback's global object would be the geolocation object's
global object.

* dom/IdleCallbackController.cpp:
(WebCore::IdleCallbackController::invokeIdleCallbacks):
Make sure we don't fire requestIdleCallback callbacks in detached iframes, to maintain
pre-existing behavior and keep layout tests passing. I had to make this change because
callback interfaces / functions are now using a different global object and can now
get called in cases when they previously couldn't.

* dom/TaskSource.h:
As Geolocation task source for the HTML5 event loop, as per the Geolocation
specification.

LayoutTests:

Update / rebaseline existing layout tests to reflect behavior change.

* fast/dom/Geolocation/callback-to-deleted-context-expected.txt:
* fast/dom/Geolocation/callback-to-deleted-context.html:
* fast/dom/Geolocation/disconnected-frame-already-expected.txt:
* fast/dom/Geolocation/disconnected-frame-already.html:
* fast/dom/Geolocation/disconnected-frame-expected.txt:
* fast/dom/Geolocation/disconnected-frame-permission-denied-expected.txt:
* fast/dom/Geolocation/disconnected-frame-permission-denied.html:
* fast/dom/Geolocation/disconnected-frame.html:
* fast/dom/Geolocation/resources/callback-to-deleted-context-inner1.html:
* fast/events/detached-svg-parent-window-events-expected.txt:
* fast/events/detached-svg-parent-window-events.html:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@281520 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants