Refactor of ExtensionNavigationThrottle
Functional changes here are:
- Treating an extension-origin "about:blank" iframe as extension,
for the purposes of the web_accessible_resources ancestor
check.
- Treating nested URLs as non-web_accessible_resources for
the purposes of subframe navigation (this fixes 661324)
These should both be minor, and are incidental.
Pure refactoring here is:
- Preferring url::Origin() checks (which leads to the functional
changes above).
- Renaming of variables for clarity.
- Hoisting the extension lookup to the top of the function, and
eliminating !extension and !registry checks after dealing
with the lookup result.
- A more efficient lookup of the parent frame, in a way
that shouldn't require an "this is safe" apology comment.
- Reflow control logic in anticipation of adding new BLOCK
cases for platform apps (in a follow-on CL).
BUG=661324
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
Review-Url: https://codereview.chromium.org/2830893002
Cr-Commit-Position: refs/heads/master@{#473717}
diff --git a/chrome/browser/extensions/process_manager_browsertest.cc b/chrome/browser/extensions/process_manager_browsertest.cc
index a178d84..88c5b96 100644
--- a/chrome/browser/extensions/process_manager_browsertest.cc
+++ b/chrome/browser/extensions/process_manager_browsertest.cc
@@ -839,34 +839,18 @@
}
// Navigate second subframe to each nested URL from the main frame (i.e.,
- // from non-extension process). This should be blocked.
- //
- // TODO(alexmos): This is also temporarily allowed under PlzNavigate, because
- // currently this particular blocking happens in
- // ChromeContentBrowserClientExtensionsPart::ShouldAllowOpenURL, which isn't
- // triggered below under PlzNavigate (since there'll be no transfer). Once
- // the blob/filesystem URL checks in ExtensionNavigationThrottle are updated
- // to apply to all frames and not just main frames, the PlzNavigate exception
- // below can be removed. See https://crbug.com/661324.
+ // from non-extension process). These should be canceled.
for (size_t i = 0; i < arraysize(nested_urls); i++) {
EXPECT_TRUE(content::NavigateIframeToURL(tab, "frame2", nested_urls[i]));
content::RenderFrameHost* second_frame = ChildFrameAt(main_frame, 1);
- if (!content::IsBrowserSideNavigationEnabled()) {
- EXPECT_NE(nested_urls[i], second_frame->GetLastCommittedURL());
- EXPECT_FALSE(extension_origin.IsSameOriginWith(
- second_frame->GetLastCommittedOrigin()));
- EXPECT_NE("foo", GetTextContent(second_frame));
- EXPECT_EQ(1u,
- pm->GetRenderFrameHostsForExtension(extension->id()).size());
- EXPECT_EQ(1u, pm->GetAllFrames().size());
- } else {
- EXPECT_EQ(nested_urls[i], second_frame->GetLastCommittedURL());
- EXPECT_EQ(extension_origin, second_frame->GetLastCommittedOrigin());
- EXPECT_EQ("foo", GetTextContent(second_frame));
- EXPECT_EQ(2u,
- pm->GetRenderFrameHostsForExtension(extension->id()).size());
- EXPECT_EQ(2u, pm->GetAllFrames().size());
- }
+
+ EXPECT_NE(nested_urls[i], second_frame->GetLastCommittedURL());
+ EXPECT_FALSE(extension_origin.IsSameOriginWith(
+ second_frame->GetLastCommittedOrigin()));
+ EXPECT_NE("foo", GetTextContent(second_frame));
+ EXPECT_EQ(1u, pm->GetRenderFrameHostsForExtension(extension->id()).size());
+ EXPECT_EQ(1u, pm->GetAllFrames().size());
+
EXPECT_TRUE(
content::NavigateIframeToURL(tab, "frame2", GURL(url::kAboutBlankURL)));
}
