[mparch] Return early for a fenced frame navigation in SBTPopupBlocker

This CL returns early for a fenced frame navigation in
SafeBrowsingTriggeredPopupBlocker since a fenced frame is treated
as a subframe for SafeBrowsingTriggeredPopupBlocker.
It also adds a new test to ensure that it doesn't have a console
log by a fenced frame navigation.

Bug: 1289212
Change-Id: Id51e0f116e0bc944d1f4e1c4efaa965d0e3becf9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3448619
Reviewed-by: Kevin McNee <[email protected]>
Reviewed-by: Avi Drissman <[email protected]>
Commit-Queue: Julie Jeongeun Kim <[email protected]>
Cr-Commit-Position: refs/heads/main@{#975029}

chrome/browser/ui/blocked_content/safe_browsing_triggered_popup_blocker_browsertest.cc

@@ -13,6 +13,7 @@
 #include "base/containers/contains.h"
 #include "base/lazy_instance.h"
 #include "base/path_service.h"
+#include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
@@ -41,11 +42,13 @@
 #include "components/safe_browsing/core/browser/db/v4_embedded_test_server_util.h"
 #include "components/safe_browsing/core/browser/db/v4_test_util.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/back_forward_cache_util.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/fenced_frame_test_util.h"
 #include "content/public/test/prerender_test_util.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "net/dns/mock_host_resolver.h"
@@ -731,6 +734,31 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
               kSafeBrowsingTriggeredPopupBlocker)));
 }
 
+// Tests that the popup blocker UI is shown when a sub frame tries to
+// open a new window if the main frame is marked as abusive since
+// SafeBrowsingTriggeredPopupBlocker works based on a main frame.
+IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
+                       OpenNewWindowInSubFrame) {
+  content::BackForwardCacheDisabledTester back_forward_cache_tester;
+  const GURL a_url(embedded_test_server()->GetURL("a.com", "/iframe.html"));
+  ConfigureAsAbusive(a_url);
+
+  // Navigate to an abusive page.
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), a_url));
+
+  content::RenderFrameHost* main_frame =
+      browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
+
+  content::RenderFrameHost* sub_frame = content::ChildFrameAt(main_frame, 0);
+  EXPECT_NE(sub_frame, nullptr);
+  EXPECT_EQ(false, content::EvalJs(sub_frame, "!!window.open()"));
+
+  // Popup UI should be shown.
+  EXPECT_TRUE(
+      PageSpecificContentSettings::GetForFrame(web_contents()->GetMainFrame())
+          ->IsContentBlocked(ContentSettingsType::POPUPS));
+}
+
 class SafeBrowsingTriggeredPopupBlockerPrerenderingBrowserTest
     : public SafeBrowsingTriggeredPopupBlockerBrowserTest {
  public:
@@ -829,3 +857,70 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerPrerenderingBrowserTest,
       PageSpecificContentSettings::GetForFrame(web_contents()->GetMainFrame())
           ->IsContentBlocked(ContentSettingsType::POPUPS));
 }
+
+class SafeBrowsingTriggeredPopupBlockerFencedFrameBrowserTest
+    : public SafeBrowsingTriggeredPopupBlockerBrowserTest {
+ public:
+  SafeBrowsingTriggeredPopupBlockerFencedFrameBrowserTest() = default;
+  ~SafeBrowsingTriggeredPopupBlockerFencedFrameBrowserTest() override = default;
+
+  content::test::FencedFrameTestHelper& fenced_frame_test_helper() {
+    return fenced_frame_test_helper_;
+  }
+
+ protected:
+  content::test::FencedFrameTestHelper fenced_frame_test_helper_;
+};
+
+// The following two tests ensure that SafeBrowsingTriggeredPopupBlocker
+// isn't triggered for a fenced frame since it's treated as a subframe for
+// SafeBrowsingTriggeredPopupBlocker even though it's a main frame in a frame
+// tree.
+// This test ensures that opening a new window in a fenced frame doesn't trigger
+// the popup blocker when the primary page is not marked as abusive, even if the
+// fenced frame's URL is.
+IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerFencedFrameBrowserTest,
+                       ShouldNotTriggerPopupBlocker) {
+  auto* first_web_contents = web_contents();
+  // Load an initial page.
+  GURL initial_url(embedded_test_server()->GetURL("/simple.html"));
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), initial_url));
+
+  // Load a fenced frame and ensure that it doesn't trigger the popup blocker.
+  GURL fenced_url(embedded_test_server()->GetURL("/fenced_frames/title1.html"));
+  // Even though `fenced_url` is marked as abusive, it doesn't affect the popup
+  // blocker.
+  ConfigureAsAbusive(fenced_url);
+
+  // Loading a fenced frame should not trigger the popup blocker.
+  auto* fenced_frame_host = fenced_frame_test_helper().CreateFencedFrame(
+      first_web_contents->GetMainFrame(), fenced_url);
+  EXPECT_EQ(false, content::EvalJs(fenced_frame_host, "!!window.open()"));
+
+  // Check if the popup UI was shown from the previous web contents.
+  EXPECT_FALSE(PageSpecificContentSettings::GetForFrame(
+                   first_web_contents->GetMainFrame())
+                   ->IsContentBlocked(ContentSettingsType::POPUPS));
+}
+
+// This test ensures that the primary page has the popup blocker when
+// the primary page is marked as abusive and the fenced frame tries to open a
+// new window.
+IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerFencedFrameBrowserTest,
+                       ShouldTriggerPopupBlocker) {
+  // Load an initial page.
+  GURL initial_url(embedded_test_server()->GetURL("/simple.html"));
+  ConfigureAsAbusive(initial_url);
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), initial_url));
+
+  // Load a fenced frame.
+  GURL fenced_url(embedded_test_server()->GetURL("/fenced_frames/title1.html"));
+  auto* fenced_frame_host = fenced_frame_test_helper().CreateFencedFrame(
+      web_contents()->GetMainFrame(), fenced_url);
+  EXPECT_EQ(false, content::EvalJs(fenced_frame_host, "!!window.open()"));
+
+  // Popup UI should be shown.
+  EXPECT_TRUE(
+      PageSpecificContentSettings::GetForFrame(web_contents()->GetMainFrame())
+          ->IsContentBlocked(ContentSettingsType::POPUPS));
+}

components/blocked_content/safe_browsing_triggered_popup_blocker.cc

@@ -18,6 +18,7 @@
 #include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/back_forward_cache.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/frame_type.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
@@ -115,8 +116,11 @@ SafeBrowsingTriggeredPopupBlocker::SafeBrowsingTriggeredPopupBlocker(
 
 void SafeBrowsingTriggeredPopupBlocker::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
-  if (!navigation_handle->IsInMainFrame())
+  if (!navigation_handle->IsInMainFrame() ||
+      navigation_handle->GetNavigatingFrameType() ==
+          content::FrameType::kFencedFrameRoot) {
     return;
+  }
 
   absl::optional<SubresourceFilterLevel> level;
   NavigationHandleData* data =
@@ -171,6 +175,11 @@ void SafeBrowsingTriggeredPopupBlocker::OnSafeBrowsingChecksComplete(
     const subresource_filter::SubresourceFilterSafeBrowsingClient::CheckResult&
         result) {
   DCHECK(navigation_handle->IsInMainFrame());
+  // TODO(crbug.com/1263541): Replace it with DCHECK.
+  if (navigation_handle->GetNavigatingFrameType() ==
+      content::FrameType::kFencedFrameRoot) {
+    return;
+  }
   absl::optional<safe_browsing::SubresourceFilterLevel> match_level;
   if (result.threat_type ==
       safe_browsing::SBThreatType::SB_THREAT_TYPE_SUBRESOURCE_FILTER) {

components/blocked_content/safe_browsing_triggered_popup_blocker_unittest.cc

@@ -38,6 +38,7 @@
 #include "content/public/test/test_navigation_throttle_inserter.h"
 #include "content/public/test/test_renderer_host.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/mojom/frame/frame.mojom.h"
 #include "third_party/blink/public/mojom/window_features/window_features.mojom.h"
 #include "ui/base/page_transition_types.h"
@@ -548,4 +549,44 @@ TEST_F(SafeBrowsingTriggeredPopupBlockerTest, NonPrimaryFrameTree) {
   }
 }
 
+class SafeBrowsingTriggeredPopupBlockerFencedFrameTest
+    : public SafeBrowsingTriggeredPopupBlockerTest {
+ public:
+  SafeBrowsingTriggeredPopupBlockerFencedFrameTest() {
+    scoped_feature_list_.InitAndEnableFeatureWithParameters(
+        blink::features::kFencedFrames, {{"implementation_type", "mparch"}});
+  }
+  ~SafeBrowsingTriggeredPopupBlockerFencedFrameTest() override = default;
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+// Ensures that the popup blocker is not triggered by a fenced frame.
+TEST_F(SafeBrowsingTriggeredPopupBlockerFencedFrameTest,
+       ShouldNotTriggerPopupBlocker) {
+  const GURL url("https://example.test/");
+  MarkUrlAsAbusiveEnforce(url);
+  NavigateAndCommit(url);
+
+  // The popup blocker is triggered for a primary page.
+  EXPECT_TRUE(
+      popup_blocker()->ShouldApplyAbusivePopupBlocker(main_rfh()->GetPage()));
+
+  content::RenderFrameHost* fenced_frame_root =
+      content::RenderFrameHostTester::For(main_rfh())->AppendFencedFrame();
+
+  // Navigate a fenced frame.
+  const GURL fenced_frame_url("https://fencedframe.test");
+  MarkUrlAsAbusiveEnforce(fenced_frame_url);
+  std::unique_ptr<content::NavigationSimulator> navigation_simulator =
+      content::NavigationSimulator::CreateForFencedFrame(fenced_frame_url,
+                                                         fenced_frame_root);
+  navigation_simulator->Commit();
+
+  // The popup blocker is not triggered for a fenced frame.
+  EXPECT_FALSE(popup_blocker()->ShouldApplyAbusivePopupBlocker(
+      fenced_frame_root->GetPage()));
+}
+
 }  // namespace blocked_content