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

[WPT] Move/merge COEP/COOP dispatcher framework to /common #29684

Merged
merged 1 commit into from
Sep 16, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jul 16, 2021

To reduce duplication and prepare for using this framework for BFCache
(#28950),
this CL merges two sets of dispatcher/executor files under COEP and COOP
and move them to /common.

Relevant discussion is also in
web-platform-tests/rfcs#89.

Most of the changes are simple path renaming, except for:

  • Service worker's scope is also moved to
    /common/dispatcher/ in:
    /wpt/html/cross-origin-embedder-policy/credentialless/service-worker-coep-credentialless-proxy.tentative.https.html
    /wpt/html/cross-origin-embedder-policy/credentialless/service-worker-coep-none-proxy.tentative.https.html
    /wpt/html/cross-origin-opener-policy/popup-coop-by-sw.https.html
    because the service workers should control executors.
  • Diffs between COEP and COOP dispatchers are merged, but are trivial
    (e.g. some functionality exists only one of them, like
    6 concurrent accesses to the server, retrying on failure,
    Access-Control-Allow-Credentials in dispatcher, etc.).
  • Reporting-related part of dispatcher.js is moved to
    /wpt/html/cross-origin-opener-policy/reporting/resources/reporting-common.js.
  • README.md about the dispatcher is moved and added.
  • /wpt/html/cross-origin-embedder-policy/credentialless/resources/cacheable-response.py
    is also merged into dispatcher.py, because
    they should access the same stash and already have common code.
  • Stash paths are moved to '/common/dispatcher'.
  • executer.js and sw_executer.js are moved to
    executer-worker.js and executer-service-worker.js, respectively,
    to clarify they are worker scripts, rather than helpers.
  • Timeout in receive() is removed because no one uses that parameter.
  • Duplicated/unused const declarations are removed.

Bug: 1107415
Change-Id: I0d28e7f4b4cca6599562ac4766a326880139028d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3033199
Commit-Queue: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Arthur Sonzogni <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/main@{#921511}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

To reduce duplication and prepare for using this framework for BFCache
(#28950),
this CL merges two sets of dispatcher/executor files under COEP and COOP
and move them to `/common`.

Relevant discussion is also in
web-platform-tests/rfcs#89.

Most of the changes are simple path renaming, except for:

- Service worker's scope is also moved to
  `/common/dispatcher/` in:
  /wpt/html/cross-origin-embedder-policy/credentialless/service-worker-coep-credentialless-proxy.tentative.https.html
  /wpt/html/cross-origin-embedder-policy/credentialless/service-worker-coep-none-proxy.tentative.https.html
  /wpt/html/cross-origin-opener-policy/popup-coop-by-sw.https.html
  because the service workers should control executors.
- Diffs between COEP and COOP dispatchers are merged, but are trivial
  (e.g. some functionality exists only one of them, like
  6 concurrent accesses to the server, retrying on failure,
  Access-Control-Allow-Credentials in dispatcher, etc.).
- Reporting-related part of `dispatcher.js` is moved to
  /wpt/html/cross-origin-opener-policy/reporting/resources/reporting-common.js.
- README.md about the dispatcher is moved and added.
- /wpt/html/cross-origin-embedder-policy/credentialless/resources/cacheable-response.py
  is also merged into dispatcher.py, because
  they should access the same stash and already have common code.
- Stash paths are moved to '/common/dispatcher'.
- `executer.js` and `sw_executer.js` are moved to
  `executer-worker.js` and `executer-service-worker.js`, respectively,
  to clarify they are worker scripts, rather than helpers.
- Timeout in receive() is removed because no one uses that parameter.
- Duplicated/unused const declarations are removed.

Bug: 1107415
Change-Id: I0d28e7f4b4cca6599562ac4766a326880139028d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3033199
Commit-Queue: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Arthur Sonzogni <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/main@{#921511}
@hiroshige-g
Copy link
Contributor

I think the failures on wpt.fyi runs and chrome-dev-stability are due to existing flakiness.

@WeizhongX
Copy link
Contributor

This test is flaky in chromium.

WPT Command: python3 ./wpt run --channel=dev --verify --verify-no-chaos-mode --verify-repeat-loop=0 --verify-repeat-restart=10 --github-checks-text-file=/home/test/artifacts/checkrun.md --affected base_head --log-mach-level=info --log-mach=- -y --no-pause --no-restart-on-unexpected --install-fonts --no-headless --verify-log-full --enable-swiftshader chrome

Some affected tests had inconsistent (flaky) results:

Unstable results

Test Subtest Results Messages
/html/cross-origin-opener-policy/navigate-to-aboutblank.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/navigate-to-aboutblank.https.html Navigate to about:blank from iframe with opener.top COOP: |header(Cross-Origin-Opener-Policy,same-origin), iframe origin: https://web-platform.test:8443, openee COOP: |header(Cross-Origin-Opener-Policy,same-origin), openee origin: https://web-platform.test:8443. PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/navigate-to-aboutblank.https.html Navigate to about:blank from iframe with opener.top COOP: |header(Cross-Origin-Opener-Policy,same-origin)|header(Cross-Origin-Embedder-Policy,require-corp), iframe origin: https://web-platform.test:8443, openee COOP: |header(Cross-Origin-Opener-Policy,same-origin)|header(Cross-Origin-Embedder-Policy,require-corp), openee origin: https://web-platform.test:8443. PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/navigate-to-aboutblank.https.html Navigate to about:blank from iframe with opener.top COOP: |header(Cross-Origin-Opener-Policy,same-origin-allow-popups), iframe origin: https://web-platform.test:8443, openee COOP: |header(Cross-Origin-Opener-Policy,same-origin-allow-popups), openee origin: https://web-platform.test:8443. PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/navigate-to-aboutblank.https.html Navigate to about:blank from iframe with opener.top COOP: |header(Cross-Origin-Opener-Policy,same-origin-allow-popups), iframe origin: https://web-platform.test:8443, openee COOP: |header(Cross-Origin-Opener-Policy,unsafe-none), openee origin: https://www1.web-platform.test:8443. PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/navigate-to-aboutblank.https.html Navigate to about:blank from iframe with opener.top COOP: |header(Cross-Origin-Opener-Policy,same-origin-allow-popups), iframe origin: https://www1.web-platform.test:8443, openee COOP: |header(Cross-Origin-Opener-Policy,unsafe-none), openee origin: https://www1.web-platform.test:8443. PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/popup-redirect-same-origin-allow-popups.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/popup-redirect-same-origin-allow-popups.https.html Same origin popup redirects to same-origin with same-origin-allow-popups PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/popup-redirect-same-origin-allow-popups.https.html Cross origin popup redirects to same-origin with same-origin-allow-popups PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-from-coop-page-to-other_coop-ro.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-from-coop-page-to-other_coop-ro.https.html access-from-coop-page-to-other (COOP-RO) PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-from-coop-page-to-other_coop-ro_cross-origin.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-from-coop-page-to-other_coop-ro_cross-origin.https.html access-from-coop-page-to-other (COOP-RO) PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-to-coop-page-from-opener_coop-ro.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-to-coop-page-from-opener_coop-ro.https.html access-to-coop-page-from-opener, same-origin PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-to-coop-page-from-opener_coop-ro.https.html access-to-coop-page-from-opener, same-origin + redirect PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-to-coop-page-from-other_coop-ro.https.html   CRASH: 2/10, OK: 8/10  
/html/cross-origin-opener-policy/reporting/access-reporting/access-to-coop-page-from-other_coop-ro.https.html access-to-coop-page-from-other (COOP-RO) PASS: 8/10, MISSING: 2/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-blur.https.html   CRASH: 2/10, OK: 8/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-blur.https.html same-origin > w => w.blur() PASS: 8/10, MISSING: 2/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-blur.https.html cross-origin > w => w.blur() PASS: 8/10, MISSING: 2/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-close.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-close.https.html same-origin > w => w.close() PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-close.https.html cross-origin > w => w.close() PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-focus.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-focus.https.html same-origin > w => w.focus() PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-focus.https.html cross-origin > w => w.focus() PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-frames.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-frames.https.html same-origin > w => w.frames PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-frames.https.html cross-origin > w => w.frames PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-location-get.https.html   CRASH: 2/10, OK: 8/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-location-get.https.html same-origin > w => w.location PASS: 8/10, MISSING: 2/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-location-get.https.html cross-origin > w => w.location PASS: 8/10, MISSING: 2/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-opener-get.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-opener-get.https.html same-origin > w => w.opener PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-opener-get.https.html cross-origin > w => w.opener PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-top.https.html   CRASH: 3/10, OK: 7/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-top.https.html same-origin > w => w.top PASS: 7/10, MISSING: 3/10  
/html/cross-origin-opener-policy/reporting/access-reporting/property-top.https.html cross-origin > w => w.top PASS: 7/10, MISSING: 3/10  
/html/cross-origin-opener-policy/reporting/access-reporting/report-to-both_coop-ro.https.html   CRASH: 1/10, OK: 9/10  
/html/cross-origin-opener-policy/reporting/access-reporting/report-to-both_coop-ro.https.html Access from opener PASS: 9/10, MISSING: 1/10  
/html/cross-origin-opener-policy/reporting/access-reporting/report-to-both_coop-ro.https.html Access from openee PASS: 9/10, MISSING: 1/10  

These may be pre-existing or new flakes. Please try to reproduce (see the above WPT command, though some flags may not be needed when running locally) and determine if your change introduced the flake. If you are unable to reproduce the problem, please tag @web-platform-tests/wpt-core-team in a comment for help.

@WeizhongX
Copy link
Contributor

@past can you admin merge this? The author explained those are existing flakiness, so I guess we don't need to file a crbug for him/her.

@hiroshige-g
Copy link
Contributor

Probably https://bugs.chromium.org/p/chromium/issues/detail?id=1205423 is for the same issue. cc/ @ArthurSonzogni

@past past merged commit bb06b9c into master Sep 16, 2021
@past past deleted the chromium-export-cl-3033199 branch September 16, 2021 16:20
@past
Copy link
Member

past commented Sep 16, 2021

Merged, thanks!

Gabisampaio pushed a commit to Gabisampaio/wpt that referenced this pull request Nov 18, 2021
…orm-tests#29684)

To reduce duplication and prepare for using this framework for BFCache
(web-platform-tests#28950),
this CL merges two sets of dispatcher/executor files under COEP and COOP
and move them to `/common`.

Relevant discussion is also in
web-platform-tests/rfcs#89.

Most of the changes are simple path renaming, except for:

- Service worker's scope is also moved to
  `/common/dispatcher/` in:
  /wpt/html/cross-origin-embedder-policy/credentialless/service-worker-coep-credentialless-proxy.tentative.https.html
  /wpt/html/cross-origin-embedder-policy/credentialless/service-worker-coep-none-proxy.tentative.https.html
  /wpt/html/cross-origin-opener-policy/popup-coop-by-sw.https.html
  because the service workers should control executors.
- Diffs between COEP and COOP dispatchers are merged, but are trivial
  (e.g. some functionality exists only one of them, like
  6 concurrent accesses to the server, retrying on failure,
  Access-Control-Allow-Credentials in dispatcher, etc.).
- Reporting-related part of `dispatcher.js` is moved to
  /wpt/html/cross-origin-opener-policy/reporting/resources/reporting-common.js.
- README.md about the dispatcher is moved and added.
- /wpt/html/cross-origin-embedder-policy/credentialless/resources/cacheable-response.py
  is also merged into dispatcher.py, because
  they should access the same stash and already have common code.
- Stash paths are moved to '/common/dispatcher'.
- `executer.js` and `sw_executer.js` are moved to
  `executer-worker.js` and `executer-service-worker.js`, respectively,
  to clarify they are worker scripts, rather than helpers.
- Timeout in receive() is removed because no one uses that parameter.
- Duplicated/unused const declarations are removed.

Bug: 1107415
Change-Id: I0d28e7f4b4cca6599562ac4766a326880139028d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3033199
Commit-Queue: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Arthur Sonzogni <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/main@{#921511}

Co-authored-by: Hiroshige Hayashizaki <[email protected]>
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.

5 participants