Commit 14750589 authored by Alex Moshchuk's avatar Alex Moshchuk Committed by Commit Bot

Add test expectations around changing minimum BrowsingInstance ID.

This modifies an existing test,
DynamicIsolatedOriginTest.NewBrowsingInstanceInOldProcess, to
explicitly exercise a scenario where a process's minimum
BrowsingInstance ID changes, and where this may affect a
CanAccessDataForOrigin decision.  In particular, this would happen if
the new minimum BrowsingInstance ID makes a particular origin eligible
for isolation.

Currently, the minimum BrowsingInstance ID stored for a process in
ChildProcessSecurityPolicy does not change even if the corresponding
BrowsingInstance goes away, so the outcome of CanAccessDataForOrigin
can't change.  This will change once multiple BrowsingInstances are
tracked for each process, rather than just a minimum ID, in issue
1135539.  In particular, after this change, CanAccessDataForOrigin may
start denying access to an origin which is considered isolated in all
of the process's remaining BrowsingInstances, due to citadel
enforcement.

Bug: 1135539
Change-Id: I8d388f6099f224172b8a77129362de8300e37ecc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2528128Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825683}
parent 36792673
......@@ -3506,11 +3506,30 @@ IN_PROC_BROWSER_TEST_F(DynamicIsolatedOriginTest,
EXPECT_NE(new_child->current_frame_host()->GetSiteInstance(),
child->current_frame_host()->GetSiteInstance());
// Make sure the bar.com iframe in the old foo.com process can still access
// bar.com cookies.
// The old foo.com process should still be able to access bar.com data,
// since it isn't locked to a specific site.
int old_process_id = root->current_frame_host()->GetProcess()->GetID();
EXPECT_TRUE(policy->CanAccessDataForOrigin(old_process_id, bar_url));
// In particular, make sure the bar.com iframe in the old foo.com process can
// still access bar.com cookies.
EXPECT_TRUE(ExecuteScript(
child, "document.cookie = 'foo=bar;SameSite=None;Secure';"));
EXPECT_EQ("foo=bar", EvalJs(child, "document.cookie"));
// Now close the first window. This destroys the first BrowsingInstance and
// leaves only the newer BrowsingInstance (with a foo.com main frame) in the
// old process.
shell()->Close();
// TODO(wjmaclean, alexmos): Currently, the process retains its minimum
// BrowsingInstance ID even after that BrowsingInstance goes away, so the
// process will maintain its access to bar.com, even if it only contains
// BrowsingInstances where bar.com is considered isolated and cannot reuse
// the old process. Once we track all BrowsingInstances in each process, we
// can tighten this to EXPECT_FALSE on platforms that support citadel
// enforcements (i.e., Android).
EXPECT_TRUE(policy->CanAccessDataForOrigin(old_process_id, bar_url));
}
// Verify that a process locked to foo.com is not reused for a navigation to
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment