Skip to content

[worklets] Queue a task to reject addModule() #967

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 7, 2019
Merged

[worklets] Queue a task to reject addModule() #967

merged 1 commit into from
Oct 7, 2019

Conversation

nhiroki
Copy link
Contributor

@nhiroki nhiroki commented Oct 2, 2019

This is a follow-up patch for #958. A task to reject addModule() should
be queued with script's error to rethrow to run on the thread where
addModule() is called.

This fixes #509

@nhiroki
[worklets] Queue a task to reject addModule()
1437d0f 
This is a follow-up patch for w3c#958. A task to reject addModule() should
be queued with script's error to rethrow to run on the thread where
addModule() is called.
@nhiroki
Copy link
Contributor Author

nhiroki commented Oct 2, 2019

@hiroshige-g @bfgeek Hi, can you review this?

@chromium-wpt-export-bot chromium-wpt-export-bot mentioned this pull request Oct 2, 2019
Merged
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 2, 2019
@nhiroki @chromium-wpt-export-bot
Worklet: Reject addModule() with script's error to rethrow if available
cff0faf 
This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
hiroshige-g
hiroshige-g reviewed Oct 2, 2019
View reviewed changes
Copy link

@hiroshige-g hiroshige-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, given that we missed this bug in the previous PR, it might be better to split out the postTask + promise-related blocks that should be executed on the main thread to a helper method.

worklets/Overview.bs

1. If |pendingTaskStruct|'s <a for="pending tasks struct">counter</a> is not <b>-1</b>, then
run these steps:

1. Set |pendingTaskStruct|'s <a for="pending tasks struct">counter</a> to <b>-1</b>.

2. Reject |promise| with |script|'s <a>error to rethrow</a>.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the intention of this change is to reference the local argument passed to the task (i.e. avoid dereferencing |script| here), but <a>error to rethrow</a> still references #concept-script's error to rethrow.

@bfgeek bfgeek merged commit 36c2ea1 into w3c:master Oct 7, 2019
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 8, 2019
@nhiroki @chromium-wpt-export-bot
Worklet: Reject addModule() with script's error to rethrow if available
ad0b751 
This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 8, 2019
@nhiroki @chromium-wpt-export-bot
Worklet: Reject addModule() with script's error to rethrow if available
c82e2f3 
This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 8, 2019
@nhiroki
Worklet: Reject addModule() with script's error to rethrow if available
906be73 
This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703606}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 8, 2019
@nhiroki @chromium-wpt-export-bot
Worklet: Reject addModule() with script's error to rethrow if available
11b16a8 
This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703606}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 8, 2019
@nhiroki @chromium-wpt-export-bot
Worklet: Reject addModule() with script's error to rethrow if available
162858b 
This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703606}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 17, 2019
@nhiroki @moz-wptsync-bot
Bug 1585564 [wpt PR 19462] - Worklet: Reject addModule() with script'…
e4078a7 
…s error to rethrow if available, a=testonly

Automatic update from web-platform-tests
Worklet: Reject addModule() with script's error to rethrow if available

This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703606}

--

wpt-commits: 162858b9961ab26dbbe27e07bea2d6e6d10702df
wpt-pr: 19462
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Oct 18, 2019
@nhiroki @moz-wptsync-bot
Bug 1585564 [wpt PR 19462] - Worklet: Reject addModule() with script'…
bc55887 
…s error to rethrow if available, a=testonly

Automatic update from web-platform-tests
Worklet: Reject addModule() with script's error to rethrow if available

This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703606}

--

wpt-commits: 162858b9961ab26dbbe27e07bea2d6e6d10702df
wpt-pr: 19462
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 19, 2019
@marco-c
Bug 1585564 [wpt PR 19462] - Worklet: Reject addModule() with script'…
d10b485 
…s error to rethrow if available, a=testonly

Automatic update from web-platform-tests
Worklet: Reject addModule() with script's error to rethrow if available

This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshigechromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#703606}

--

wpt-commits: 162858b9961ab26dbbe27e07bea2d6e6d10702df
wpt-pr: 19462

UltraBlame original commit: be5a525dc0ae96f3bca68b403b22e4bb3bfc56b0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 19, 2019
@marco-c
Bug 1585564 [wpt PR 19462] - Worklet: Reject addModule() with script'…
3123871 
…s error to rethrow if available, a=testonly

Automatic update from web-platform-tests
Worklet: Reject addModule() with script's error to rethrow if available

This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshigechromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#703606}

--

wpt-commits: 162858b9961ab26dbbe27e07bea2d6e6d10702df
wpt-pr: 19462

UltraBlame original commit: be5a525dc0ae96f3bca68b403b22e4bb3bfc56b0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 19, 2019
@marco-c
Bug 1585564 [wpt PR 19462] - Worklet: Reject addModule() with script'…
d4e650b 
…s error to rethrow if available, a=testonly

Automatic update from web-platform-tests
Worklet: Reject addModule() with script's error to rethrow if available

This CL makes Worklet#addModule() fail with a more specific error object
compared to before.

Before this change, Worklet::addModule() was rejected with AbortError regardless
of an actual error reason, and that made it difficult for developers to debug
worklets. This behavior was defined in the Worklets spec, but it was changed
recently (see the links below).

After this change, Worklet::addModule() is rejected with script's error to
rethrow when it is available as the new spec defines.

Spec changes:
- w3c/css-houdini-drafts#958
- w3c/css-houdini-drafts#967

Chromestatus:
- https://www.chromestatus.com/feature/5116796497559552

Bug: 782066
Change-Id: Iabd30eff28ed1bffed9219898649a89b8abebbb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1258785
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshigechromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#703606}

--

wpt-commits: 162858b9961ab26dbbe27e07bea2d6e6d10702df
wpt-pr: 19462

UltraBlame original commit: be5a525dc0ae96f3bca68b403b22e4bb3bfc56b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hiroshige-g hiroshige-g hiroshige-g left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[worklets] addModule() should reject a promise with a more specific exception
3 participants
@nhiroki @hiroshige-g @bfgeek