Changeset 281520 in webkit


Ignore:
Timestamp:
Aug 24, 2021 3:03:34 PM (11 months ago)
Author:
Chris Dumez
Message:

Geolocation API should callback with error if doc is not fully active
​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 ​https://github.com/web-platform-tests/wpt/pull/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:

  • 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:

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:
Location:
trunk
Files:
4 added
20 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r281519 r281520  
     12021-08-24  Chris Dumez  <cdumez@apple.com>
     2
     3        Geolocation API should callback with error if doc is not fully active
     4        https://bugs.webkit.org/show_bug.cgi?id=228319
     5        <rdar://problem/81450315>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Update / rebaseline existing layout tests to reflect behavior change.
     10
     11        * fast/dom/Geolocation/callback-to-deleted-context-expected.txt:
     12        * fast/dom/Geolocation/callback-to-deleted-context.html:
     13        * fast/dom/Geolocation/disconnected-frame-already-expected.txt:
     14        * fast/dom/Geolocation/disconnected-frame-already.html:
     15        * fast/dom/Geolocation/disconnected-frame-expected.txt:
     16        * fast/dom/Geolocation/disconnected-frame-permission-denied-expected.txt:
     17        * fast/dom/Geolocation/disconnected-frame-permission-denied.html:
     18        * fast/dom/Geolocation/disconnected-frame.html:
     19        * fast/dom/Geolocation/resources/callback-to-deleted-context-inner1.html:
     20        * fast/events/detached-svg-parent-window-events-expected.txt:
     21        * fast/events/detached-svg-parent-window-events.html:
     22
    1232021-08-24  Eric Hutchison  <ehutchison@apple.com>
    224

  • trunk/LayoutTests/fast/dom/Geolocation/callback-to-deleted-context-expected.txt

    r231450 r281520  
    55
    66
    7 PASS Success callback invoked
     7PASS No callbacks invoked
    88PASS successfullyParsed is true
    99

  • trunk/LayoutTests/fast/dom/Geolocation/callback-to-deleted-context.html

    r217390 r281520  
    1414function onSecondIframeLoaded() {
    1515    window.setTimeout(function() {
    16         testFailed('No callbacks invoked');
     16        testPassed('No callbacks invoked');
    1717        finishJSTest();
    1818    }, 500);

  • trunk/LayoutTests/fast/dom/Geolocation/disconnected-frame-already-expected.txt

    r65329 r281520  
    44
    55
    6 Method called on Geolocation object with disconnected Frame.
     6PASS Error callback invoked
    77PASS successfullyParsed is true
    88

  • trunk/LayoutTests/fast/dom/Geolocation/disconnected-frame-already.html

    r217390 r281520  
    2424        finishJSTest();
    2525    }, function(e) {
    26         testFailed('Error callback invoked unexpectedly');
     26        testPassed('Error callback invoked');
    2727        finishJSTest();
    2828    });

  • trunk/LayoutTests/fast/dom/Geolocation/disconnected-frame-expected.txt

    r231450 r281520  
    55
    66
    7 PASS No callbacks invoked
     7PASS Error callback invoked
    88PASS successfullyParsed is true
    99

  • trunk/LayoutTests/fast/dom/Geolocation/disconnected-frame-permission-denied-expected.txt

    r231450 r281520  
    88PASS error.message is "User denied Geolocation"
    99
    10 PASS No callbacks invoked
     10PASS Error callback invoked
    1111PASS successfullyParsed is true
    1212

  • trunk/LayoutTests/fast/dom/Geolocation/disconnected-frame-permission-denied.html

    r217390 r281520  
    3737        finishJSTest();
    3838    }, function(e) {
    39         testFailed('Error callback invoked unexpectedly');
     39        testPassed('Error callback invoked');
    4040        finishJSTest();
    4141    });
    4242    setTimeout(function() {
    43         testPassed('No callbacks invoked');
     43        testFailed('No callbacks invoked');
    4444        finishJSTest();
    45     }, 100);
     45    }, 1000);
    4646}
    4747

  • trunk/LayoutTests/fast/dom/Geolocation/disconnected-frame.html

    r217390 r281520  
    2424        finishJSTest();
    2525    }, function(e) {
    26         testFailed('Error callback invoked unexpectedly');
     26        testPassed('Error callback invoked');
    2727        finishJSTest();
    2828    });
    2929    setTimeout(function() {
    30         testPassed('No callbacks invoked');
     30        testFailed('No callbacks invoked');
    3131        finishJSTest();
    32     }, 100);
     32    }, 1000);
    3333}
    3434

  • trunk/LayoutTests/fast/dom/Geolocation/resources/callback-to-deleted-context-inner1.html

    r127923 r281520  
    1414          // object attempts to invoke the callback.
    1515          window.parent.navigator.geolocation.getCurrentPosition(function() {
    16               parent.testPassed('Success callback invoked');
     16              parent.testFailed('Success callback invoked unexpectedly');
    1717              parent.finishJSTest();
    1818          }, function() {

  • trunk/LayoutTests/fast/events/detached-svg-parent-window-events-expected.txt

    r259900 r281520  
    1 CONSOLE MESSAGE: error
    21This tests creating a disconnected SVG element with resize event handler. The event handler should not get dispatched unless the element is connected
    32
    … …  
    54
    65
     6PASS Exception thrown
    77PASS didFireResize is false
    88PASS didFireOnError is false

  • trunk/LayoutTests/fast/events/detached-svg-parent-window-events.html

    r257897 r281520  
    2222                        finishJSTest();
    2323                    });
    24                     iframe.contentWindow.eval('throw "error"');
     24                    try {
     25                        iframe.contentWindow.eval('throw "error"');
     26                    } catch (e) {
     27                        testPassed("Exception thrown");
     28                    }
    2529                });
    2630            };

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r281516 r281520  
     12021-08-24  Chris Dumez  <cdumez@apple.com>
     2
     3        Geolocation API should callback with error if doc is not fully active
     4        https://bugs.webkit.org/show_bug.cgi?id=228319
     5        <rdar://problem/81450315>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        * web-platform-tests/geolocation-API/PositionOptions.https-expected.txt:
     10        Rebaseline WPT test now that more checks are passing.
     11
     12        * web-platform-tests/geolocation-API/non-fully-active.https-expected.txt: Added.
     13        * web-platform-tests/geolocation-API/non-fully-active.https.html: Added.
     14        * web-platform-tests/geolocation-API/resources/iframe.html: Added.
     15        Import test coverage from WPT https://github.com/web-platform-tests/wpt/pull/29799.
     16
     17        * web-platform-tests/resources/testdriver-vendor.js:
     18        (window.test_driver_internal.set_permission):
     19        Add support for test_driver.set_permission("geolocation", "granted") so that Geolocation
     20        WPT tests can run with our infrastructure.
     21
    1222021-08-24  Chris Dumez  <cdumez@apple.com>
    223

  • trunk/LayoutTests/imported/w3c/web-platform-tests/geolocation-API/PositionOptions.https-expected.txt

    r270048 r281520  
    22PASS Call getCurrentPosition with wrong type for enableHighAccuracy. No exception expected.
    33PASS Call watchPosition with wrong type for enableHighAccuracy. No exception expected.
    4 FAIL PositionOptions tests promise_test: Unhandled rejection with value: object "Error: unimplemented"
     4PASS PositionOptions tests
     5PASS Set timeout and maximumAge to 0, check that timeout error raised (getCurrentPosition)
     6PASS Set timeout and maximumAge to 0, check that timeout error raised (watchPosition)
     7PASS Check that a negative timeout value is equivalent to a 0 timeout value (getCurrentLocation)
     8PASS Check that a negative timeout value is equivalent to a 0 timeout value (watchPosition)
    59

  • trunk/LayoutTests/imported/w3c/web-platform-tests/resources/testdriver-vendor.js

    r273136 r281520  
    294294        return dispatchMouseActions(pointerSource.actions, pointerType);
    295295};
     296
     297window.test_driver_internal.set_permission = function(permission_params, context=null)
     298{
     299    if (window.testRunner && permission_params.descriptor.name == "geolocation") {
     300        setInterval(() => {
     301            window.testRunner.setMockGeolocationPosition(51.478, -0.166, 100);
     302        }, 100);
     303        testRunner.setGeolocationPermission(permission_params.state == "granted");
     304        return Promise.resolve();
     305    }
     306    return Promise.reject(new Error("unimplemented"));
     307};
     308

  • trunk/Source/WebCore/ChangeLog

    r281516 r281520  
     12021-08-24  Chris Dumez  <cdumez@apple.com>
     2
     3        Geolocation API should callback with error if doc is not fully active
     4        https://bugs.webkit.org/show_bug.cgi?id=228319
     5        <rdar://problem/81450315>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Test: imported/w3c/web-platform-tests/geolocation-API/non-fully-active.https.html
     10
     11        * Modules/geolocation/Geolocation.cpp:
     12        (WebCore::Geolocation::getCurrentPosition):
     13        (WebCore::Geolocation::watchPosition):
     14        Schedule a task to call the error callback if the document is not fully active, as
     15        per the specification:
     16        - https://github.com/w3c/geolocation-api/issues/96
     17        - https://github.com/w3c/geolocation-api/pull/97
     18
     19        * bindings/js/JSDOMConvertCallbacks.h:
     20        (WebCore::Converter<IDLCallbackFunction<T>>::convert):
     21        (WebCore::Converter<IDLCallbackInterface<T>>::convert):
     22        Make sure we use the incumbent global object when constructing callback functions /
     23        interfaces, as per the Web IDL specification:
     24        - https://heycam.github.io/webidl/#es-callback-interface
     25        - https://heycam.github.io/webidl/#es-callback-function
     26        Without this, the geolocation API would be unable to call its error callback when in
     27        a detached frame because the callback's global object would be the geolocation object's
     28        global object.
     29
     30        * dom/IdleCallbackController.cpp:
     31        (WebCore::IdleCallbackController::invokeIdleCallbacks):
     32        Make sure we don't fire requestIdleCallback callbacks in detached iframes, to maintain
     33        pre-existing behavior and keep layout tests passing. I had to make this change because
     34        callback interfaces / functions are now using a different global object and can now
     35        get called in cases when they previously couldn't.
     36
     37        * dom/TaskSource.h:
     38        As Geolocation task source for the HTML5 event loop, as per the Geolocation
     39        specification.
     40
    1412021-08-24  Chris Dumez  <cdumez@apple.com>
    242

  • trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp

    r278253 r281520  
    3232
    3333#include "Document.h"
     34#include "EventLoop.h"
    3435#include "FeaturePolicy.h"
    3536#include "Frame.h"
    … …  
    300301void Geolocation::getCurrentPosition(Ref<PositionCallback>&& successCallback, RefPtr<PositionErrorCallback>&& errorCallback, PositionOptions&& options)
    301302{
    302     if (!frame())
    303         return;
     303    if (!document() || !document()->isFullyActive()) {
     304        if (errorCallback && errorCallback->scriptExecutionContext()) {
     305            errorCallback->scriptExecutionContext()->eventLoop().queueTask(TaskSource::Geolocation, [errorCallback] {
     306                errorCallback->handleEvent(GeolocationPositionError::create(GeolocationPositionError::POSITION_UNAVAILABLE, "Document is not fully active"_s));
     307            });
     308        }
     309        return;
     310    }
    304311
    305312    auto notifier = GeoNotifier::create(*this, WTFMove(successCallback), WTFMove(errorCallback), WTFMove(options));
    … …  
    311318int Geolocation::watchPosition(Ref<PositionCallback>&& successCallback, RefPtr<PositionErrorCallback>&& errorCallback, PositionOptions&& options)
    312319{
    313     if (!frame())
     320    if (!document() || !document()->isFullyActive()) {
     321        if (errorCallback && errorCallback->scriptExecutionContext()) {
     322            errorCallback->scriptExecutionContext()->eventLoop().queueTask(TaskSource::Geolocation, [errorCallback] {
     323                errorCallback->handleEvent(GeolocationPositionError::create(GeolocationPositionError::POSITION_UNAVAILABLE, "Document is not fully active"_s));
     324            });
     325        }
    314326        return 0;
     327    }
    315328
    316329    auto notifier = GeoNotifier::create(*this, WTFMove(successCallback), WTFMove(errorCallback), WTFMove(options));
    … …  
    694707    }
    695708
    696     RefPtr<GeolocationPosition> position = lastPosition();
    697     ASSERT(position);
    698 
    699     makeSuccessCallbacks(*position);
     709    if (RefPtr position = lastPosition())
     710        makeSuccessCallbacks(*position);
    700711}
    701712

  • trunk/Source/WebCore/bindings/js/JSDOMConvertCallbacks.h

    r260848 r281520  
    4545            return nullptr;
    4646        }
     47
     48        JSDOMGlobalObject* incumbentGlobalObject = &globalObject;
     49        if (auto* globalObject = JSC::CallFrame::globalObjectOfClosestCodeBlock(vm, vm.topCallFrame))
     50            incumbentGlobalObject = JSC::jsCast<JSDOMGlobalObject*>(globalObject);
    4751       
    48         return T::create(JSC::asObject(value), &globalObject);
     52        return T::create(JSC::asObject(value), incumbentGlobalObject);
    4953    }
    5054};
    … …  
    8084        }
    8185
    82         return T::create(JSC::asObject(value), &globalObject);
     86        JSDOMGlobalObject* incumbentGlobalObject = &globalObject;
     87        if (auto* globalObject = JSC::CallFrame::globalObjectOfClosestCodeBlock(vm, vm.topCallFrame))
     88            incumbentGlobalObject = JSC::jsCast<JSDOMGlobalObject*>(globalObject);
     89
     90        return T::create(JSC::asObject(value), incumbentGlobalObject);
    8391    }
    8492};

  • trunk/Source/WebCore/dom/IdleCallbackController.cpp

    r252607 r281520  
    110110void IdleCallbackController::invokeIdleCallbacks(MonotonicTime deadline)
    111111{
    112     if (!m_document)
     112    if (!m_document || !m_document->frame())
    113113        return;
    114114

  • trunk/Source/WebCore/dom/TaskSource.h

    r275241 r281520  
    3333    FileReading,
    3434    FontLoading,
     35    Geolocation,
    3536    IdleTask,
    3637    IndexedDB,
Note: See TracChangeset for help on using the changeset viewer.