Skip to content

Commit

Permalink
Revert "Origin isolation: a new strategy for window.originIsolationRe…
Browse files Browse the repository at this point in the history
…stricted"

This reverts commit 7ac0b9ed68d57270b680e39b9e21e1ede1b8c774.

Reason for revert: Possible cause of flaky navigation/error-related test failures on https://ci.chromium.org/p/chromium/builders/ci/Linux%20Tests and https://ci.chromium.org/p/chromium/builders/ci/Mac10.10%20Tests, see e.g. https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8873201719841161216/+/steps/content_browsertests_on__none__GPU_on_Mac_on_Mac-10.10/0/logs/Flaky_failure:_All__x2f_NavigationControllerBrowserTest.ErrorPageReplacement__x2f_0__status_CRASH_SUCCESS_/0 and https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8873205275892139456/+/steps/storage_service_unsandboxed_content_browsertests/0/logs/Deterministic_failure:_All__x2f_SitePerProcessIgnoreCertErrorsBrowserTest.SubresourceWithCertificateErrors__x2f_0__status_FAILURE_/0.

If this revert does not help, I will reland.
Original change's description:
> Origin isolation: a new strategy for window.originIsolationRestricted
>
> In https://chromium-review.googlesource.com/c/chromium/src/+/2243994 I
> introduced an implementation for window.originIsolationRestricted which
> pipes the isolation state from NavigationRequest to the navigated-to
> LocalDOMWindow. However, this does not work for the case of the initial
> about:blank, where no navigation is performed. Furthermore, it does not
> match the spec, where the Window property just reflects an agent
> cluster-wide property.
>
> This CL introduces an alternate approach, more similar to what is done
> for self.crossOriginIsolated in
> https://chromium-review.googlesource.com/c/chromium/src/+/2247463, which
> is another agent cluster-wide value. The origin isolation state is
> stored in the renderer-side Agent class. Then the LocalDOMWindow getter
> can just pick it up from the surrounding agent, as in the spec. Note
> that unlike the implementation for self.crossOriginIsolated, the value
> is per-Agent instead of static (process-wide).
>
> Currently the value is set several times per agent (roughly once on
> every navigation). This is redundant, but we don't yet have a good place
> to set it once (i.e., we don't have a browser-side "time of agent
> creation"). If that gets fixed, we can likely stop piping the value
> through navigation params. See
> https://docs.google.com/document/d/1MTnmyWAoAIKDH4yWaRthIUdi05MsjlML8gctvKP7-h8/edit
> for discussions around fixing that.
>
> This fixes the issue with about:blank iframes embedded in origin-isolated
> pages reporting false, because the agent's origin-isolated boolean was
> previously set to true by the containing frame.
>
> This does not yet fix the issue with data: URLs reporting false, tested in
> external/wpt/origin-isolation/getter-special-cases/data-url.https.html.
> However, that will be doable as a followup, by changing the
> navigation-time computation to pass true for them instead of false.
> (Currently it passes false because data: URLs don't get their own
> process, but it should pass true because they _do_ get their own agent
> cluster.)
>
> Bug: 1095653
> Change-Id: I8dfa8fc4a4766efc0611d43a255673662c422776
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2300237
> Commit-Queue: Domenic Denicola <domenic@chromium.org>
> Reviewed-by: Charlie Reis <creis@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#793799}

TBR=dcheng@chromium.org,creis@chromium.org,alexmos@chromium.org,domenic@chromium.org,wjmaclean@chromium.org

Change-Id: Ia9174b00ac61178cff9bf24801182d23779399c5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1095653
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2333651
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793885}
  • Loading branch information
pkasting authored and chromium-wpt-export-bot committed Aug 1, 2020
1 parent 0e4384b commit 43899f5
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions origin-isolation/about-blank.https.sub.html
Expand Up @@ -34,20 +34,23 @@
const iframe = document.createElement("iframe");
document.body.append(iframe);

// Now create and add the script, but don't navigate anywhere (since we want
// to stay on the initial about:blank).
// Now use document.write() to get the child frame script in there, without
// actually navigating anywhere.
// We need to absolutize the URL to since about:blank doesn't have a base URL.
const scriptURL = (new URL("./resources/child-frame-script.mjs", import.meta.url)).href;
const script = iframe.contentDocument.createElement("script");
script.type = "module";
script.src = scriptURL;
iframe.contentDocument.write(`<script type="module" src="${scriptURL}"></scr` + `ipt>`);
iframe.contentDocument.close();

await new Promise((resolve, reject) => {
// Note that since this code runs during the same event loop turn as the
// contentDocument.write() above, we know that the load/error events will
// not have been fired at this time. (The spec guarantees they are fired
// from queued tasks.)
const script = iframe.contentDocument.querySelector("script");
script.onload = resolve;
script.onerror = () => reject(
new Error("Could not load the child frame script into the about:blank page")
);
iframe.contentDocument.body.append(script);
});

await setBothDocumentDomains(iframe.contentWindow);
Expand Down

0 comments on commit 43899f5

Please sign in to comment.