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

Infra: remove old service worker code from testharness.js #21162

Merged
merged 1 commit into from Mar 2, 2020

Conversation

@annevk
Copy link
Member

annevk commented Jan 14, 2020

No description provided.

@wpt-pr-bot wpt-pr-bot requested review from gsnedders and jgraham Jan 14, 2020
@gsnedders gsnedders closed this Jan 24, 2020
@gsnedders gsnedders deleted the annevk/remove-old-code branch Jan 24, 2020
@gsnedders gsnedders restored the annevk/remove-old-code branch Jan 24, 2020
@Hexcles Hexcles reopened this Jan 24, 2020
@gsnedders gsnedders removed their assignment Jan 31, 2020
@annevk annevk requested a review from wanderview Feb 19, 2020
@jgraham
jgraham approved these changes Mar 2, 2020
Copy link
Contributor

jgraham left a comment

I don't know if there's a spec change here, but having one codepath rather than two seems strictly better.

@annevk annevk merged commit ed0c310 into master Mar 2, 2020
10 of 12 checks passed
10 of 12 checks passed
build-and-publish
Details
build-and-publish
Details
detect-deployment
Details
Azure Pipelines Build #20200114.28 failed
Details
Azure Pipelines (infrastructure/ tests: macOS) infrastructure/ tests: macOS failed
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Community-TC (pull_request) TaskGroup: success
Details
wpt.fyi - safari[experimental] Safari results
Details
@annevk annevk deleted the annevk/remove-old-code branch Mar 2, 2020
@stephenmcgruer
Copy link
Contributor

stephenmcgruer commented Mar 2, 2020

So it looks like this was added in 821d133, with no details or Chrome bug link as to why, but the comment clearly suggests that there was a Blink bug that made this necessary.

@annevk do we know that any such bug has been fixed? Did we run a trigger run of this commit to see what changes it makes to the WPT results on Chrome? It would have been nice to be able to provide some of that information in the commit message, to help future-us not have to perform code archaeology* years down the line to understand why a change was made.

  • Like the archaeology I just had to perform for that comment, since it didn't link any form of bug to track this claimed Blink problem.
@annevk
Copy link
Member Author

annevk commented Mar 2, 2020

I thought I checked with someone if that comment was still true for Chrome, but you're right that I didn't really do a good job of documenting that, including for myself. So I guess this is all we have and if that's not enough, we should revert.

@stephenmcgruer
Copy link
Contributor

stephenmcgruer commented Mar 2, 2020

We can check the wpt.fyi results before and after (I will take the action item to do that), which should be sufficient.

@annevk
Copy link
Member Author

annevk commented Mar 2, 2020

Thanks @stephenmcgruer! Won't happen again.

@stephenmcgruer
Copy link
Contributor

stephenmcgruer commented Mar 2, 2020

Diff is https://wpt.fyi/results/?diff&filter=ADC&run_id=451860003&run_id=423660009

There are a handful of TIMEOUT differences in service-worker/:

https://wpt.fyi/results/service-workers/service-worker/client-navigate.https.html?diff&filter=ADC&run_id=451860003&run_id=423660009
https://wpt.fyi/results/service-workers/service-worker/windowclient-navigate.https.html?diff&filter=ADC&run_id=451860003&run_id=423660009

But I'm not sure if these are real or flake. Might be worth running locally with/without the change and seeing if we can reproduce the diff, or we can just wait and see if they go away in subsequent runs.

@stephenmcgruer
Copy link
Contributor

stephenmcgruer commented Mar 2, 2020

Looks like the Chromium import of this change also marked a few service-worker tests as TIMEOUT now - https://chromium-review.googlesource.com/c/chromium/src/+/2083053/4/third_party/blink/web_tests/TestExpectations

chromium-wpt-export-bot added a commit that referenced this pull request Apr 20, 2020
This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] #21162
[2] #22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
chromium-wpt-export-bot added a commit that referenced this pull request Apr 21, 2020
This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] #21162
[2] #22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760974}
pull bot pushed a commit to FreddyZeng/chromium that referenced this pull request Apr 21, 2020
This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760974}
chromium-wpt-export-bot added a commit that referenced this pull request Apr 22, 2020
This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] #21162
[2] #22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760974}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 1, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request May 1, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 1, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request May 1, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request May 3, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazuchromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105

UltraBlame original commit: a1a73fefe890d9fd377e15fcac599508ea28e1c1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request May 3, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazuchromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105

UltraBlame original commit: e4f2dd72c208f41d4cccb483eeead7523e60e0fc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request May 3, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazuchromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105

UltraBlame original commit: a1a73fefe890d9fd377e15fcac599508ea28e1c1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request May 3, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazuchromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105

UltraBlame original commit: e4f2dd72c208f41d4cccb483eeead7523e60e0fc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request May 3, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazuchromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105

UltraBlame original commit: a1a73fefe890d9fd377e15fcac599508ea28e1c1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request May 3, 2020
…igate.https.html, a=testonly

Automatic update from web-platform-tests
ServiceWorker: Fix timeout on client-navigate.https.html

This CL fixes timeout on client-navigate.https.html, and removes unused
code in testharness.js.

This change is a follow-up for:
[1] web-platform-tests/wpt#21162
[2] web-platform-tests/wpt#22086

After the change [1], fetch_tests_from_workers() helper uses
ExtendableMessageEvent.source instead of MessageChannel to communicate
between a window and a service worker. This change works well for most
of cases, but fails some tests.

The fetch_tests_from_workers() helper takes a service worker object to
post a "connect" message to the worker, and then passes the worker
object to RemoteContext in testharness.js. RemoteContext adds an
onmessage event handler on self.navigator.serviceWorker. This works well
as long as the given worker object is associated with `self`. However,
these failing tests pass the worker object associated with the inner
frame to the helper. In this case, the helper uses an inner frame's
service worker object for postMessage(), while the helper waits on the
main frame's navigator.serviceWorker.onmessage. Consequently
event.source is the inner frame, and a reply from the service worker is
dispatched on the inner frame's navigator.serviceWorker.onmessage. This
results in timeout.

To fix this, this CL makes the main frame pass its own service worker
object instead of the inner frame's service worker object. Those
objects should be equal other than associated contexts, and this change
doesn't affect what the tests verify.

Bug: 1057682
Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760
Reviewed-by: Makoto Shimazu <shimazuchromium.org>
Commit-Queue: Hiroki Nakagawa <nhirokichromium.org>
Cr-Commit-Position: refs/heads/master{#760974}

--

wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707
wpt-pr: 23105

UltraBlame original commit: e4f2dd72c208f41d4cccb483eeead7523e60e0fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.