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

Origin isolation: implement window.originIsolationRestricted #24194

Merged
merged 1 commit into from
Jun 26, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jun 17, 2020

See WICG/origin-agent-cluster#24 and
WICG/origin-agent-cluster#30 for background,
and whatwg/html#5545 for the specification.

Failing test expectations include:

Note that per ongoing discussion in
WICG/origin-agent-cluster#31 the naming of this
API, as well as its edge-case behavior (e.g. for sandboxed iframes) will
likely change.

Bug: 1042415
Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782672}

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.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-2243994 branch 2 times, most recently from 9408d50 to 1c2b652 Compare June 25, 2020 19:12
See WICG/origin-agent-cluster#24 and
WICG/origin-agent-cluster#30 for background,
and whatwg/html#5545 for the specification.

Failing test expectations include:

- We implement (3) from
  WICG/origin-agent-cluster#24
  instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking
  at https://crbug.com/1095653.
- The initial about:blank, as well as removed iframes, are not properly
  returning true, so about-blank and removing-iframes are failing. Also
  tracking at https://crbug.com/1095653.
- data: URLs are not [SecureContext] in Chromium
  (https://crbug.com/1095656) so getter-data-url fails.

Note that per ongoing discussion in
WICG/origin-agent-cluster#31 the naming of this
API, as well as its edge-case behavior (e.g. for sandboxed iframes) will
likely change.

Bug: 1042415
Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782672}
@LukeZielinski
Copy link
Contributor

One of the test is flaky in Chrome dev (more details below). It seems to crash periodically. Logs show one potentially useful line of output from Chrome:
2:45.23 pid:9968 [1593121997.834][SEVERE]: Unable to receive message from renderer

Unstable results

Test Subtest Results Messages
/origin-isolation/removing-iframes.sub.https.html ERROR: 5/10, OK: 5/10
/origin-isolation/removing-iframes.sub.https.html Before: messageerror event must occur PASS: 5/10, MISSING: 5/10
/origin-isolation/removing-iframes.sub.https.html Before: setting document.domain must not give sync access PASS: 5/10, MISSING: 5/10
/origin-isolation/removing-iframes.sub.https.html parent: originIsolationRestricted must equal true FAIL: 5/10, MISSING: 5/10 assert_equals: expected (boolean) true but got (undefined) undefined
/origin-isolation/removing-iframes.sub.https.html child1: originIsolationRestricted must equal false FAIL: 5/10, MISSING: 5/10 assert_equals: expected (boolean) false but got (undefined) undefined
/origin-isolation/removing-iframes.sub.https.html Remove the iframe and insert new ones PASS: 5/10, MISSING: 5/10
/origin-isolation/removing-iframes.sub.https.html Parent to child2: messageerror event must occur PASS: 5/10, MISSING: 5/10
/origin-isolation/removing-iframes.sub.https.html Parent to child2: setting document.domain must not give sync access PASS: 5/10, MISSING: 5/10
/origin-isolation/removing-iframes.sub.https.html Parent to child3: messageerror event must occur PASS: 5/10, MISSING: 5/10
/origin-isolation/removing-iframes.sub.https.html Parent to child3: setting document.domain must not give sync access PASS: 5/10, MISSING: 5/10
/origin-isolation/removing-iframes.sub.https.html child2 to child3: message event must occur FAIL: 5/10, MISSING: 5/10 assert_equals: expected "WebAssembly.Module message received" but got "messageerror"
/origin-isolation/removing-iframes.sub.https.html child2 to child3: setting document.domain must give sync access FAIL: 5/10, MISSING: 5/10 assert_equals: expected "accessed document successfully" but got "SecurityError"
/origin-isolation/removing-iframes.sub.https.html child3 to child2: message event must occur FAIL: 5/10, MISSING: 5/10 assert_equals: expected "WebAssembly.Module message received" but got "messageerror"
/origin-isolation/removing-iframes.sub.https.html child3 to child2: setting document.domain must give sync access FAIL: 5/10, MISSING: 5/10 assert_equals: expected "accessed document successfully" but got "SecurityError"
/origin-isolation/removing-iframes.sub.https.html child2: originIsolationRestricted must equal false FAIL: 5/10, MISSING: 5/10 assert_equals: expected (boolean) false but got (undefined) undefined
/origin-isolation/removing-iframes.sub.https.html `child

@LukeZielinski
Copy link
Contributor

Filed crbug.com/1099718 to track the flake. @Hexcles could you please admin merge?

@stephenmcgruer
Copy link
Contributor

2:45.23 pid:9968 [1593121997.834][SEVERE]: Unable to receive message from renderer

I suspect that is just recording exactly that the renderer has crashed. We continue to be unable to provide folks with useful logs to debug these problems :(

@Hexcles Hexcles merged commit 9e23aa4 into master Jun 26, 2020
@Hexcles Hexcles deleted the chromium-export-cl-2243994 branch June 26, 2020 14:41
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.

6 participants