Skip to content

Commit

Permalink
Some WebIDL operations / attributes incorrectly use _current_ realm i…
Browse files Browse the repository at this point in the history
…nstead of _relevant_

https://bugs.webkit.org/show_bug.cgi?id=230941

Patch by Alexey Shvayka <[email protected]> on 2021-12-10
Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

Import WPT tests from web-platform-tests/wpt#32012.

* web-platform-tests/dom/events/Event-timestamp-cross-realm-getter-expected.txt: Added.
* web-platform-tests/dom/events/Event-timestamp-cross-realm-getter.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method.html: Added.
* web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method-expected.txt: Added.
* web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method.html: Added.
* web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method-expected.txt: Added.
* web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method.html: Added.
* web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method-expected.txt: Added.
* web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method.html: Added.

Source/WebCore:

This patch replaces _current_ global object with _relevant_, as per recommendation
for spec authors [1], for select WebIDL operations / attributes that satisfy all
the following conditions:

  1) it's an instance member: static ones and constructors can't use _relevant_;
  2) it's on standards track (not deprecated / WebKit-only / internal);
  3) the change is directly observable: global object is used for something
     beyond lifecycle / event loop / parsing CSS etc;
  4) the change either aligns WebKit with both Blink and Gecko,
     or the spec explicitly requires _relevant_ realm / settings object.

Most of the remaining [CallWith=GlobalObject] instances are correctly used for
converting JS arguments to WebIDL values; the rest, along with _current_ Document
and ScriptExecutionContext, either match the spec or replacing them with _relevant_
global object is not directly observable (see condition #3).

This change is aimed at fixing web-exposed APIs rather than performing a global cleanup.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#concept-current-everything

Tests: imported/w3c/web-platform-tests/dom/events/Event-timestamp-cross-realm-getter.html
       imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method.html
       imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method.html
       imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method.html
       imported/w3c/web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method.html
       imported/w3c/web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method.html
       imported/w3c/web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method.html

* Modules/indexeddb/IDBFactory.idl:
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-open (step 2)
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-deletedatabase (step 1)
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-databases (step 1)

* Modules/paymentrequest/PaymentRequest.idl:
https://www.w3.org/TR/payment-request/#show-method (steps 2-4)
https://www.w3.org/TR/payment-request/#can-make-payment-algorithm (before step 1)

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateCallWith):
* bindings/scripts/IDLAttributes.json:
* bindings/scripts/test/JS/JSTestObj.cpp:
* bindings/scripts/test/TestObj.idl:
* dom/Event.idl:
https://dom.spec.whatwg.org/#inner-event-creation-steps (step 3)

* dom/IdleDeadline.idl:
https://w3c.github.io/requestidlecallback/#the-requestidlecallback-method (step 1)

* page/History.idl:
https://html.spec.whatwg.org/multipage/history.html#dom-history-go (step 1)
https://html.spec.whatwg.org/multipage/history.html#dom-history-back (step 1)
https://html.spec.whatwg.org/multipage/history.html#dom-history-forward (step 1)

* page/DOMWindow.cpp:
(WebCore::DOMWindow::setTimeout):
(WebCore::DOMWindow::setInterval):
* page/DOMWindow.h:
* workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::setTimeout):
(WebCore::WorkerGlobalScope::setInterval):
* workers/WorkerGlobalScope.h:
Although condition #4 isn't satisfied for setTimeout() / setInterval() because
_current_ global object is used only for logging, replacing it with _relevant_
nicely cleans up method signatures.

* page/WindowOrWorkerGlobalScope.cpp:
(WebCore::WindowOrWorkerGlobalScope::structuredClone):
* page/WindowOrWorkerGlobalScope.h:
* page/WindowOrWorkerGlobalScope.idl:
https://html.spec.whatwg.org/multipage/webappapis.html#report-the-exception
https://html.spec.whatwg.org/multipage/structured-data.html#structured-cloning (step 2)


Canonical link: https://commits.webkit.org/245123@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286895 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Alexey Shvayka authored and webkit-commit-queue committed Dec 11, 2021
1 parent 131cc3c commit 4d36383
Show file tree
Hide file tree
Showing 32 changed files with 434 additions and 33 deletions.
24 changes: 24 additions & 0 deletions LayoutTests/imported/w3c/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,27 @@
2021-12-10 Alexey Shvayka <[email protected]>

Some WebIDL operations / attributes incorrectly use _current_ realm instead of _relevant_
https://bugs.webkit.org/show_bug.cgi?id=230941

Reviewed by Sam Weinig.

Import WPT tests from https://github.com/web-platform-tests/wpt/pull/32012.

* web-platform-tests/dom/events/Event-timestamp-cross-realm-getter-expected.txt: Added.
* web-platform-tests/dom/events/Event-timestamp-cross-realm-getter.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method.html: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method-expected.txt: Added.
* web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method.html: Added.
* web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method-expected.txt: Added.
* web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method.html: Added.
* web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method-expected.txt: Added.
* web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method.html: Added.
* web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method-expected.txt: Added.
* web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method.html: Added.

2021-12-10 Alexey Shvayka <[email protected]>

Extend the scope where the Window's current event is set
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@


PASS event.timeStamp is initialized using event's relevant global object

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!doctype html>
<meta charset="utf-8">
<title>event.timeStamp is initialized using event's relevant global object</title>
<link rel="help" href="https://dom.spec.whatwg.org/#ref-for-dom-event-timestamp%E2%91%A1">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<body>
<script>
const t = async_test();
t.step_timeout(() => {
const iframeDelayed = document.createElement("iframe");
iframeDelayed.onload = t.step_func_done(() => {
// Use eval() to eliminate false-positive test result for WebKit builds before r280256,
// which invoked WebIDL accessors in context of lexical (caller) global object.
const timeStampExpected = iframeDelayed.contentWindow.eval(`new Event("foo").timeStamp`);
const eventDelayed = new iframeDelayed.contentWindow.Event("foo");

const {get} = Object.getOwnPropertyDescriptor(Event.prototype, "timeStamp");
assert_approx_equals(get.call(eventDelayed), timeStampExpected, 5, "via Object.getOwnPropertyDescriptor");

Object.setPrototypeOf(eventDelayed, Event.prototype);
assert_approx_equals(eventDelayed.timeStamp, timeStampExpected, 5, "via Object.setPrototypeOf");
});
document.body.append(iframeDelayed);
}, 1000);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@


PASS history.back() uses this's associated document's browsing context to determine if navigation is allowed

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!doctype html>
<meta charset="utf-8">
<title>history.back() uses this's associated document's browsing context to determine if navigation is allowed</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/history.html#dom-history-back">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<iframe id="sandboxedIframe" srcdoc="hello" sandbox="allow-scripts allow-same-origin"></iframe>
<script>
const t = async_test();

t.step(() => {
history.pushState({}, null, "?prev");
history.pushState({}, null, "?current");

sandboxedIframe.contentWindow.history.back.call(history);
});

window.onpopstate = t.step_func_done(() => {
assert_equals(location.search, "?prev");
});
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@


PASS history.forward() uses this's associated document's browsing context to determine if navigation is allowed

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!doctype html>
<meta charset="utf-8">
<title>history.forward() uses this's associated document's browsing context to determine if navigation is allowed</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/history.html#dom-history-forward">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<iframe id="sandboxedIframe" srcdoc="hello" sandbox="allow-scripts allow-same-origin"></iframe>
<script>
const t = async_test();

t.step(() => {
history.pushState({}, null, "?prev");
history.pushState({}, null, "?current");
history.back();
});

let isCrossRealmForward = false;
window.onpopstate = t.step_func(() => {
if (isCrossRealmForward) {
assert_equals(location.search, "?current");
t.done();
} else {
sandboxedIframe.contentWindow.history.forward.call(history);
isCrossRealmForward = true;
}
});
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@


PASS history.go() uses this's associated document's browsing context to determine if navigation is allowed

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!doctype html>
<meta charset="utf-8">
<title>history.go() uses this's associated document's browsing context to determine if navigation is allowed</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/history.html#dom-history-go">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<iframe id="sandboxedIframe" srcdoc="hello" sandbox="allow-scripts allow-same-origin"></iframe>
<script>
const t = async_test();

t.step(() => {
history.pushState({}, null, "?prev=2");
history.pushState({}, null, "?prev=1");
history.pushState({}, null, "?current");

sandboxedIframe.contentWindow.history.go.call(history, -2);
});

window.onpopstate = t.step_func_done(() => {
assert_equals(location.search, "?prev=2");
});
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CONSOLE MESSAGE: TypeError: foo


PASS self.reportError() dispatches an "error" event for this's relevant global object

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<!doctype html>
<meta charset="utf-8">
<title>self.reportError() dispatches an "error" event for this's relevant global object</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/webappapis.html#dom-reporterror">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<body>
<script>
setup({ allow_uncaught_exception: true });

async_test(t => {
window.addEventListener("error", t.unreached_func("'error' event should not be dispatched for top window!"));

const iframe = document.createElement("iframe");
iframe.onload = t.step_func_done(() => {
let eventFired = false;
const error = new TypeError("foo");
const otherWindow = iframe.contentWindow;
otherWindow.addEventListener("error", t.step_func(event => {
assert_equals(event.error, error);
eventFired = true;
}));

window.reportError.call(otherWindow, error);
assert_true(eventFired);
});
document.body.append(iframe);
});
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@


PASS Object instance
PASS Array instance
PASS Date instance
PASS RegExp instance

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!doctype html>
<title>self.structuredClone() uses this's relevant Realm for deserialization</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/structured-data.html#structured-cloning">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<body>
<script>
const iframe = document.createElement("iframe");
iframe.onload = () => {
const otherWindow = iframe.contentWindow;
for (const key of ["Object", "Array", "Date", "RegExp"]) {
test(() => {
const cloned = otherWindow.structuredClone.call(window, new otherWindow[key]);
assert_true(cloned instanceof window[key]);
}, `${key} instance`);
}
};
document.body.append(iframe);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@


PASS IdleDeadline::timeRemaining() uses relevant global object as a high-res timestamp origin

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!doctype html><!-- webkit-test-runner [ RequestIdleCallbackEnabled=true ] -->
<meta charset="utf-8">
<meta name="timeout" content="long">
<title>IdleDeadline::timeRemaining() uses relevant global object as a high-res timestamp origin</title>
<link rel="help" href="https://w3c.github.io/requestidlecallback/#dom-idledeadline-timeremaining">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<body>
<script>
const t = async_test();
t.step_timeout(() => {
const iframeDelayed = document.createElement("iframe");
iframeDelayed.onload = t.step_func(() => {
requestIdleCallback(t.step_func_done(deadline => {
assert_approx_equals(
iframeDelayed.contentWindow.IdleDeadline.prototype.timeRemaining.call(deadline),
deadline.timeRemaining(),
5,
);
}));
});
document.body.append(iframeDelayed);
}, 1000);
</script>
79 changes: 79 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,82 @@
2021-12-10 Alexey Shvayka <[email protected]>

Some WebIDL operations / attributes incorrectly use _current_ realm instead of _relevant_
https://bugs.webkit.org/show_bug.cgi?id=230941

Reviewed by Sam Weinig.

This patch replaces _current_ global object with _relevant_, as per recommendation
for spec authors [1], for select WebIDL operations / attributes that satisfy all
the following conditions:

1) it's an instance member: static ones and constructors can't use _relevant_;
2) it's on standards track (not deprecated / WebKit-only / internal);
3) the change is directly observable: global object is used for something
beyond lifecycle / event loop / parsing CSS etc;
4) the change either aligns WebKit with both Blink and Gecko,
or the spec explicitly requires _relevant_ realm / settings object.

Most of the remaining [CallWith=GlobalObject] instances are correctly used for
converting JS arguments to WebIDL values; the rest, along with _current_ Document
and ScriptExecutionContext, either match the spec or replacing them with _relevant_
global object is not directly observable (see condition #3).

This change is aimed at fixing web-exposed APIs rather than performing a global cleanup.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#concept-current-everything

Tests: imported/w3c/web-platform-tests/dom/events/Event-timestamp-cross-realm-getter.html
imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_back_cross_realm_method.html
imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_forward_cross_realm_method.html
imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_go_cross_realm_method.html
imported/w3c/web-platform-tests/html/webappapis/scripting/reporterror-cross-realm-method.html
imported/w3c/web-platform-tests/html/webappapis/structured-clone/structured-clone-cross-realm-method.html
imported/w3c/web-platform-tests/requestidlecallback/callback-timeRemaining-cross-realm-method.html

* Modules/indexeddb/IDBFactory.idl:
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-open (step 2)
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-deletedatabase (step 1)
https://www.w3.org/TR/IndexedDB/#dom-idbfactory-databases (step 1)

* Modules/paymentrequest/PaymentRequest.idl:
https://www.w3.org/TR/payment-request/#show-method (steps 2-4)
https://www.w3.org/TR/payment-request/#can-make-payment-algorithm (before step 1)

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateCallWith):
* bindings/scripts/IDLAttributes.json:
* bindings/scripts/test/JS/JSTestObj.cpp:
* bindings/scripts/test/TestObj.idl:
* dom/Event.idl:
https://dom.spec.whatwg.org/#inner-event-creation-steps (step 3)

* dom/IdleDeadline.idl:
https://w3c.github.io/requestidlecallback/#the-requestidlecallback-method (step 1)

* page/History.idl:
https://html.spec.whatwg.org/multipage/history.html#dom-history-go (step 1)
https://html.spec.whatwg.org/multipage/history.html#dom-history-back (step 1)
https://html.spec.whatwg.org/multipage/history.html#dom-history-forward (step 1)

* page/DOMWindow.cpp:
(WebCore::DOMWindow::setTimeout):
(WebCore::DOMWindow::setInterval):
* page/DOMWindow.h:
* workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::setTimeout):
(WebCore::WorkerGlobalScope::setInterval):
* workers/WorkerGlobalScope.h:
Although condition #4 isn't satisfied for setTimeout() / setInterval() because
_current_ global object is used only for logging, replacing it with _relevant_
nicely cleans up method signatures.

* page/WindowOrWorkerGlobalScope.cpp:
(WebCore::WindowOrWorkerGlobalScope::structuredClone):
* page/WindowOrWorkerGlobalScope.h:
* page/WindowOrWorkerGlobalScope.idl:
https://html.spec.whatwg.org/multipage/webappapis.html#report-the-exception
https://html.spec.whatwg.org/multipage/structured-data.html#structured-cloning (step 2)

2021-12-10 Devin Rousso <[email protected]>

WKWebView doesn’t respond to -copyFont: and -pasteFont:
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/Modules/indexeddb/IDBFactory.idl
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
SkipVTableValidation,
Exposed=(Window,Worker)
] interface IDBFactory {
[NewObject, CallWith=ScriptExecutionContext] IDBOpenDBRequest open(DOMString name, optional [EnforceRange] unsigned long long version);
[NewObject, CallWith=ScriptExecutionContext] IDBOpenDBRequest deleteDatabase(DOMString name);
[NewObject, CallWith=RelevantScriptExecutionContext] IDBOpenDBRequest open(DOMString name, optional [EnforceRange] unsigned long long version);
[NewObject, CallWith=RelevantScriptExecutionContext] IDBOpenDBRequest deleteDatabase(DOMString name);

[CallWith=ScriptExecutionContext] Promise<sequence<IDBDatabaseInfo>> databases();
[CallWith=RelevantScriptExecutionContext] Promise<sequence<IDBDatabaseInfo>> databases();

[CallWith=GlobalObject] short cmp(any first, any second);
};
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/Modules/paymentrequest/PaymentRequest.idl
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
] interface PaymentRequest : EventTarget {
[CallWith=Document] constructor(sequence<PaymentMethodData> methodData, PaymentDetailsInit details, optional PaymentOptions options);

[CallWith=Document] Promise<PaymentResponse> show(optional Promise<PaymentDetailsUpdate> detailsPromise);
[CallWith=RelevantDocument] Promise<PaymentResponse> show(optional Promise<PaymentDetailsUpdate> detailsPromise);
Promise<undefined> abort();
[CallWith=Document] Promise<boolean> canMakePayment();
[CallWith=RelevantDocument] Promise<boolean> canMakePayment();

readonly attribute DOMString id;
readonly attribute PaymentAddress? shippingAddress;
Expand Down
Loading

0 comments on commit 4d36383

Please sign in to comment.