Enable subresource filter for children of frames with aborted first load

Subframes which have their initial load aborted (e.g. due to a
document.write() call) never have the subresource filter activated on
the browser side. We now skip over these frames when ascending the frame
tree so that they inherit the appropriate activation from the frame's
parent. We do not handle the case of a main frame with an aborted load.
This will be fixed by crbug.com/1055558. Frames with aborted non-initial
loads keep their previous activation.

In addition, activated_frame_hosts_ is renamed to frame_host_filter_map_
to reflect the inclusion of nullptr for frame hosts that should inherit
the activation of its parent (which might not be activated). Some tests
are modified to watch for a title change that occurs after all scripts
are loaded. This allows those scripts' functions to be used safely.

The subresource filter was enabled on the frames with an aborted initial
load themselves by crrev.com/c/2064474.

Bug: 1052362
Change-Id: I9121be0f9154e346ca2602c30475b68e4da9f560
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081523
Commit-Queue: Alex Turner <alexmt@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754730}

chrome/browser/subresource_filter/ad_tagging_browsertest.cc

@@ -210,14 +210,15 @@ AdTaggingBrowserTest::CreateFrameWithDocWriteAbortedLoadImpl(
                          ad_script ? "createAdFrameWithDocWriteAbortedLoad"
                                    : "createFrameWithDocWriteAbortedLoad",
                          name.c_str());
-  content::WebContents* web_contents =
-      content::WebContents::FromRenderFrameHost(rfh);
-  content::TestNavigationObserver navigation_observer(web_contents, 1);
+  // The executed scripts set the title to be the frame name when they have
+  // finished loading.
+  content::TitleWatcher title_watcher(GetWebContents(),
+                                      base::ASCIIToUTF16(name));
   EXPECT_TRUE(content::ExecuteScript(rfh, script));
-  navigation_observer.Wait();
-  EXPECT_FALSE(navigation_observer.last_navigation_succeeded());
+  EXPECT_EQ(base::ASCIIToUTF16(name), title_watcher.WaitAndGetTitle());
   return content::FrameMatchingPredicate(
-      web_contents, base::BindRepeating(&content::FrameMatchesName, name));
+      content::WebContents::FromRenderFrameHost(rfh),
+      base::BindRepeating(&content::FrameMatchesName, name));
 }
 
 content::RenderFrameHost*
@@ -231,14 +232,15 @@ AdTaggingBrowserTest::CreateFrameWithWindowStopAbortedLoadImpl(
                          ad_script ? "createAdFrameWithWindowStopAbortedLoad"
                                    : "createFrameWithWindowStopAbortedLoad",
                          name.c_str());
-  content::WebContents* web_contents =
-      content::WebContents::FromRenderFrameHost(rfh);
-  content::TestNavigationObserver navigation_observer(web_contents, 1);
+  // The executed scripts set the title to be the frame name when they have
+  // finished loading.
+  content::TitleWatcher title_watcher(GetWebContents(),
+                                      base::ASCIIToUTF16(name));
   EXPECT_TRUE(content::ExecuteScript(rfh, script));
-  navigation_observer.Wait();
-  EXPECT_FALSE(navigation_observer.last_navigation_succeeded());
+  EXPECT_EQ(base::ASCIIToUTF16(name), title_watcher.WaitAndGetTitle());
   return content::FrameMatchingPredicate(
-      web_contents, base::BindRepeating(&content::FrameMatchesName, name));
+      content::WebContents::FromRenderFrameHost(rfh),
+      base::BindRepeating(&content::FrameMatchesName, name));
 }
 
 // Given a RenderFrameHost, navigates the page to the given |url| and waits
@@ -480,6 +482,90 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest,
       child_frame_of_ad_with_aborted_load->GetFrameTreeNodeId()));
 }
 
+// Test that the children of a frame with its initial load aborted due to a
+// doc.write are reported correctly as vanilla or ad frames.
+IN_PROC_BROWSER_TEST_F(
+    AdTaggingBrowserTest,
+    ChildrenOfFrameWithDocWriteAbortedLoad_StillCorrectlyTagged) {
+  TestSubresourceFilterObserver observer(web_contents());
+
+  // Main frame.
+  ui_test_utils::NavigateToURL(browser(), GetURL("frame_factory.html"));
+
+  // Create a frame and abort its initial load in vanilla script. The children
+  // of this vanilla frame should be taggged correctly.
+  content::RenderFrameHost* vanilla_frame_with_aborted_load =
+      CreateFrameWithDocWriteAbortedLoad(GetWebContents());
+
+  content::RenderFrameHost* vanilla_child_of_vanilla = CreateSrcFrame(
+      vanilla_frame_with_aborted_load, GetURL("frame_factory.html"));
+  EXPECT_FALSE(*observer.GetIsAdSubframe(
+      vanilla_child_of_vanilla->GetFrameTreeNodeId()));
+
+  content::RenderFrameHost* ad_child_of_vanilla = CreateSrcFrameFromAdScript(
+      vanilla_frame_with_aborted_load, GetURL("frame_factory.html"));
+  EXPECT_TRUE(
+      *observer.GetIsAdSubframe(ad_child_of_vanilla->GetFrameTreeNodeId()));
+
+  // Create a frame and abort its initial load in ad script. The children of
+  // this ad frame should be tagged as ads.
+  content::RenderFrameHost* ad_frame_with_aborted_load =
+      CreateFrameWithDocWriteAbortedLoadFromAdScript(GetWebContents());
+  EXPECT_TRUE(*observer.GetIsAdSubframe(
+      ad_frame_with_aborted_load->GetFrameTreeNodeId()));
+
+  content::RenderFrameHost* vanilla_child_of_ad =
+      CreateSrcFrame(ad_frame_with_aborted_load, GetURL("frame_factory.html"));
+  EXPECT_TRUE(
+      *observer.GetIsAdSubframe(vanilla_child_of_ad->GetFrameTreeNodeId()));
+
+  content::RenderFrameHost* ad_child_of_ad = CreateSrcFrameFromAdScript(
+      ad_frame_with_aborted_load, GetURL("frame_factory.html"));
+  EXPECT_TRUE(*observer.GetIsAdSubframe(ad_child_of_ad->GetFrameTreeNodeId()));
+}
+
+// Test that the children of a frame with its initial load aborted due to a
+// window.stop are reported correctly as vanilla or ad frames.
+IN_PROC_BROWSER_TEST_F(
+    AdTaggingBrowserTest,
+    ChildrenOfFrameWithWindowStopAbortedLoad_StillCorrectlyTagged) {
+  TestSubresourceFilterObserver observer(web_contents());
+
+  // Main frame.
+  ui_test_utils::NavigateToURL(browser(), GetURL("frame_factory.html"));
+
+  // Create a frame and abort its initial load in vanilla script. The children
+  // of this vanilla frame should be taggged correctly.
+  content::RenderFrameHost* vanilla_frame_with_aborted_load =
+      CreateFrameWithWindowStopAbortedLoad(GetWebContents());
+
+  content::RenderFrameHost* vanilla_child_of_vanilla = CreateSrcFrame(
+      vanilla_frame_with_aborted_load, GetURL("frame_factory.html"));
+  EXPECT_FALSE(*observer.GetIsAdSubframe(
+      vanilla_child_of_vanilla->GetFrameTreeNodeId()));
+
+  content::RenderFrameHost* ad_child_of_vanilla = CreateSrcFrameFromAdScript(
+      vanilla_frame_with_aborted_load, GetURL("frame_factory.html"));
+  EXPECT_TRUE(
+      *observer.GetIsAdSubframe(ad_child_of_vanilla->GetFrameTreeNodeId()));
+
+  // Create a frame and abort its initial load in ad script. The children of
+  // this ad frame should be tagged as ads.
+  content::RenderFrameHost* ad_frame_with_aborted_load =
+      CreateFrameWithWindowStopAbortedLoadFromAdScript(GetWebContents());
+  EXPECT_TRUE(*observer.GetIsAdSubframe(
+      ad_frame_with_aborted_load->GetFrameTreeNodeId()));
+
+  content::RenderFrameHost* vanilla_child_of_ad =
+      CreateSrcFrame(ad_frame_with_aborted_load, GetURL("frame_factory.html"));
+  EXPECT_TRUE(
+      *observer.GetIsAdSubframe(vanilla_child_of_ad->GetFrameTreeNodeId()));
+
+  content::RenderFrameHost* ad_child_of_ad = CreateSrcFrameFromAdScript(
+      ad_frame_with_aborted_load, GetURL("frame_factory.html"));
+  EXPECT_TRUE(*observer.GetIsAdSubframe(ad_child_of_ad->GetFrameTreeNodeId()));
+}
+
 void ExpectWindowOpenUkmEntry(const ukm::TestUkmRecorder& ukm_recorder,
                               bool from_main_frame,
                               const GURL& main_frame_url,

chrome/browser/subresource_filter/subresource_filter_browsertest.cc

@@ -659,45 +659,150 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
 }
 
 // Test that resources in frames with an aborted initial load due to a doc.write
-// are still tagged as ads.
+// are still disallowed.
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
-                       FrameWithDocWriteAbortedLoad_StillTaggedAsAd) {
+                       FrameWithDocWriteAbortedLoad_ResourceStillDisallowed) {
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetWithRules({testing::CreateSuffixRule("ad=true")}));
-  // Block ad resources.
+
+  // Block disallowed resources.
   Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
                        subresource_filter::ActivationScope::ALL_SITES);
   ResetConfiguration(std::move(config));
 
+  // Watches for title set by onload and onerror callbacks of tested resource
   content::TitleWatcher title_watcher(web_contents(),
                                       base::ASCIIToUTF16("failed"));
   title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16("loaded"));
 
   ui_test_utils::NavigateToURL(
-      browser(), embedded_test_server()->GetURL(
-                     "/subresource_filter/docwrite_loads_ad_resource.html"));
+      browser(),
+      embedded_test_server()->GetURL(
+          "/subresource_filter/docwrite_loads_disallowed_resource.html"));
+
   // Check the load was blocked.
   EXPECT_EQ(base::ASCIIToUTF16("failed"), title_watcher.WaitAndGetTitle());
 }
 
 // Test that resources in frames with an aborted initial load due to a
-// window.stop are still tagged as ads.
+// window.stop are still disallowed.
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
-                       FrameWithWindowStopAbortedLoad_StillTaggedAsAd) {
+                       FrameWithWindowStopAbortedLoad_ResourceStillDisallowed) {
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetWithRules({testing::CreateSuffixRule("ad=true")}));
-  // Block ad resources.
+
+  // Block disallowed resources.
   Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
                        subresource_filter::ActivationScope::ALL_SITES);
   ResetConfiguration(std::move(config));
 
+  // Watches for title set by onload and onerror callbacks of tested resource
   content::TitleWatcher title_watcher(web_contents(),
                                       base::ASCIIToUTF16("failed"));
   title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16("loaded"));
 
+  ui_test_utils::NavigateToURL(
+      browser(),
+      embedded_test_server()->GetURL(
+          "/subresource_filter/window_stop_loads_disallowed_resource.html"));
+
+  // Check the load was blocked.
+  EXPECT_EQ(base::ASCIIToUTF16("failed"), title_watcher.WaitAndGetTitle());
+}
+
+// Test that a frame with an aborted initial load due to a frame deletion does
+// not cause a crash.
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       FrameDeletedDuringLoad_DoesNotCrash) {
+  // Watches for title set by end of frame deletion script.
+  content::TitleWatcher title_watcher(web_contents(),
+                                      base::ASCIIToUTF16("done"));
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
-                     "/subresource_filter/window_stop_loads_ad_resource.html"));
+                     "/subresource_filter/delete_loading_frame.html"));
+
+  // Wait for the script to complete.
+  EXPECT_EQ(base::ASCIIToUTF16("done"), title_watcher.WaitAndGetTitle());
+}
+
+// Test that an allowed resource in the child of a frame with its initial load
+// aborted due to a doc.write is not blocked.
+IN_PROC_BROWSER_TEST_F(
+    SubresourceFilterBrowserTest,
+    ChildOfFrameWithAbortedLoadLoadsAllowedResource_ResourceLoaded) {
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetWithRules({testing::CreateSuffixRule("ad=true")}));
+
+  // Block disallowed resources.
+  Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
+                       subresource_filter::ActivationScope::ALL_SITES);
+  ResetConfiguration(std::move(config));
+
+  // Watches for title set by onload and onerror callbacks of tested resource.
+  content::TitleWatcher title_watcher(web_contents(),
+                                      base::ASCIIToUTF16("failed"));
+  title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16("loaded"));
+
+  ui_test_utils::NavigateToURL(
+      browser(),
+      embedded_test_server()->GetURL("/subresource_filter/"
+                                     "docwrite_creates_subframe.html"));
+
+  content::RenderFrameHost* frame = FindFrameByName("grandchild");
+
+  EXPECT_TRUE(ExecJs(frame, R"SCRIPT(
+      let image = document.createElement('img');
+      image.src = 'pixel.png';
+      image.onload = function() {
+        top.document.title='loaded';
+      };
+      image.onerror = function() {
+        top.document.title='failed';
+      };
+      document.body.appendChild(image);
+  )SCRIPT"));
+
+  // Check the load wasn't blocked.
+  EXPECT_EQ(base::ASCIIToUTF16("loaded"), title_watcher.WaitAndGetTitle());
+}
+
+// Test that a disallowed resource in the child of a frame with its initial load
+// aborted due to a doc.write is blocked.
+IN_PROC_BROWSER_TEST_F(
+    SubresourceFilterBrowserTest,
+    ChildOfFrameWithAbortedLoadLoadsDisallowedResource_ResourceBlocked) {
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetWithRules({testing::CreateSuffixRule("ad=true")}));
+
+  // Block disallowed resources.
+  Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
+                       subresource_filter::ActivationScope::ALL_SITES);
+  ResetConfiguration(std::move(config));
+
+  // Watches for title set by onload and onerror callbacks of tested resource.
+  content::TitleWatcher title_watcher(web_contents(),
+                                      base::ASCIIToUTF16("failed"));
+  title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16("loaded"));
+
+  ui_test_utils::NavigateToURL(
+      browser(),
+      embedded_test_server()->GetURL("/subresource_filter/"
+                                     "docwrite_creates_subframe.html"));
+
+  content::RenderFrameHost* frame = FindFrameByName("grandchild");
+
+  EXPECT_TRUE(ExecJs(frame, R"SCRIPT(
+      let image = document.createElement('img');
+      image.src = 'pixel.png?ad=true';
+      image.onload = function() {
+        top.document.title='loaded';
+      };
+      image.onerror = function() {
+        top.document.title='failed';
+      };
+      document.body.appendChild(image);
+  )SCRIPT"));
+
   // Check the load was blocked.
   EXPECT_EQ(base::ASCIIToUTF16("failed"), title_watcher.WaitAndGetTitle());
 }

chrome/test/data/ad_tagging/ad_script.js

@@ -44,7 +44,14 @@ function createAdFrameWithDocWriteAbortedLoad(name) {
   frame.src = '/slow?100';
   document.body.appendChild(frame);
   frame.contentDocument.open();
-  frame.contentDocument.write('<html><head></head><body></body></html>');
+  // We load the scripts in frame_factory.html to allow subframe creation,
+  // setting the title so we know when all scripts have loaded.
+  frame.contentDocument.write(
+      '<html><head>' +
+      '<script src="create_frame.js"></script>' +
+      '<script src="ad_script.js"></script>' +
+      '<script onload="top.document.title = window.name" ' +
+      'src="ad_script_2.js"></script></head><body></body></html>');
   frame.contentDocument.close();
 }
 
@@ -57,4 +64,21 @@ function createAdFrameWithWindowStopAbortedLoad(name) {
   frame.src = '/slow?100';
   document.body.appendChild(frame);
   frame.contentWindow.stop();
+
+  // We load the scripts in frame_factory.html to allow subframe creation.
+  let script1 = document.createElement('script');
+  script1.src = 'create_frame.js';
+  frame.contentDocument.head.appendChild(script1);
+
+  let script2 = document.createElement('script');
+  script2.src = 'ad_script.js';
+  frame.contentDocument.head.appendChild(script2);
+
+  let script3 = document.createElement('script');
+  script3.src = 'ad_script_2.js';
+  // Set title so we know when all scripts have loaded.
+  script3.onload = function() {
+    top.document.title = name;
+  };
+  frame.contentDocument.head.appendChild(script3);
 }

chrome/test/data/ad_tagging/create_frame.js

@@ -44,7 +44,14 @@ function createFrameWithDocWriteAbortedLoad(name) {
   frame.src = '/slow?100';
   document.body.appendChild(frame);
   frame.contentDocument.open();
-  frame.contentDocument.write('<html><head></head><body></body></html>');
+  // We load the scripts in frame_factory.html to allow subframe creation,
+  // setting the title so we know when all scripts have loaded.
+  frame.contentDocument.write(
+      '<html><head>' +
+      '<script src="create_frame.js"></script>' +
+      '<script src="ad_script.js"></script>' +
+      '<script onload="top.document.title = window.name" ' +
+      'src="ad_script_2.js"></script></head><body></body></html>');
   frame.contentDocument.close();
 }
 
@@ -57,4 +64,21 @@ function createFrameWithWindowStopAbortedLoad(name) {
   frame.src = '/slow?100';
   document.body.appendChild(frame);
   frame.contentWindow.stop();
+
+  // We load the scripts in frame_factory.html to allow subframe creation.
+  let script1 = document.createElement('script');
+  script1.src = 'create_frame.js';
+  frame.contentDocument.head.appendChild(script1);
+
+  let script2 = document.createElement('script');
+  script2.src = 'ad_script.js';
+  frame.contentDocument.head.appendChild(script2);
+
+  let script3 = document.createElement('script');
+  script3.src = 'ad_script_2.js';
+  // Set title so we know when all scripts have loaded.
+  script3.onload = function() {
+    top.document.title = name;
+  };
+  frame.contentDocument.head.appendChild(script3);
 }

chrome/test/data/subresource_filter/delete_loading_frame.html

+<html>
+<body>
+<iframe src="/slow?100" id="iframe"></iframe>
+<script>
+  // slow takes 100 seconds to load, plenty of time to overwrite the
+  // provisional load.
+  iframe = document.getElementById("iframe");
+  iframe.parentElement.removeChild(iframe);
+  document.title = "done";
+</script>
+</body>
+</html>

chrome/test/data/subresource_filter/docwrite_creates_subframe.html

+<html>
+<body>
+<iframe src="/slow?100" id="iframe"></iframe>
+<script>
+  // slow takes 100 seconds to load, plenty of time to overwrite the
+  // provisional load.
+  iframe = document.getElementById("iframe");
+  let doc = iframe.contentDocument;
+
+  doc.open();
+  doc.write("<html><body>Rewritten. <iframe name='grandchild'></iframe></body></html>");
+  doc.close();
+</script>
+</body>
+</html>

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc

@@ -26,6 +26,7 @@
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/navigation_throttle.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 "net/base/net_errors.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
@@ -58,7 +59,8 @@ void ContentSubresourceFilterThrottleManager::OnSubresourceFilterGoingAway() {
 
 void ContentSubresourceFilterThrottleManager::RenderFrameDeleted(
     content::RenderFrameHost* frame_host) {
-  activated_frame_hosts_.erase(frame_host);
+  frame_host_filter_map_.erase(frame_host);
+  navigated_frames_.erase(frame_host);
   ad_frames_.erase(frame_host);
   navigation_load_policies_.erase(frame_host);
   DestroyRulesetHandleIfNoLongerUsed();
@@ -152,25 +154,53 @@ void ContentSubresourceFilterThrottleManager::ReadyToCommitNavigation(
 
 void ContentSubresourceFilterThrottleManager::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
-  // Do nothing if the navigation finished in the same document. Just make sure
-  // to not leak throttle pointers.
-  if (!navigation_handle->HasCommitted() ||
-      navigation_handle->IsSameDocument()) {
-    ongoing_activation_throttles_.erase(navigation_handle);
+  // Make sure not to leak throttle pointers.
+  ActivationStateComputingNavigationThrottle* throttle = nullptr;
+  auto throttle_it = ongoing_activation_throttles_.find(navigation_handle);
+  if (throttle_it != ongoing_activation_throttles_.end()) {
+    throttle = throttle_it->second;
+    ongoing_activation_throttles_.erase(throttle_it);
+  }
+
+  // Do nothing if the navigation finished in the same document.
+  if (navigation_handle->IsSameDocument()) {
+    return;
+  }
+  if (!navigation_handle->HasCommitted()) {
+    // TODO(crbug.com/1055558): Handle the case of an aborted main frame load.
+
+    // If the initial load was aborted, the frame's activation will never have
+    // been set and should instead be inherited from its parents. Reuse the
+    // previous activation in the case of a non-initial aborted load.
+    if (!navigation_handle->IsInMainFrame() &&
+        navigation_handle->GetNetErrorCode() == net::ERR_ABORTED) {
+      // Cannot get the RFH from navigation_handle due to the aborted load.
+      content::RenderFrameHost* frame_host =
+          navigation_handle->GetWebContents()->UnsafeFindFrameByFrameTreeNodeId(
+              navigation_handle->GetFrameTreeNodeId());
+
+      // The RenderFrameHost will still exist as, even if a frame is destroyed,
+      // the NavigationHandle is destroyed (resulting in a call to
+      // DidFinishNavigation) before the RenderFrameHost is.
+      DCHECK(frame_host);
+      if (navigated_frames_.insert(frame_host).second) {
+        DCHECK(!base::Contains(frame_host_filter_map_, frame_host));
+        frame_host_filter_map_[frame_host] = nullptr;
+      }
+    }
     return;
   }
 
-  auto throttle_it = ongoing_activation_throttles_.find(navigation_handle);
   std::unique_ptr<AsyncDocumentSubresourceFilter> filter;
-  if (throttle_it != ongoing_activation_throttles_.end()) {
-    ActivationStateComputingNavigationThrottle* throttle = throttle_it->second;
+  if (throttle) {
     CHECK_EQ(navigation_handle, throttle->navigation_handle());
     filter = throttle->ReleaseFilter();
-    ongoing_activation_throttles_.erase(throttle_it);
   }
 
   content::RenderFrameHost* frame_host =
       navigation_handle->GetRenderFrameHost();
+  navigated_frames_.insert(frame_host);
+
   if (navigation_handle->IsInMainFrame()) {
     current_committed_load_has_notified_disallowed_load_ = false;
     statistics_.reset();
@@ -192,20 +222,20 @@ void ContentSubresourceFilterThrottleManager::DidFinishNavigation(
                               level);
   }
 
-  // Make sure |activated_frame_hosts_| is updated or cleaned up depending on
+  // Make sure |frame_host_filter_map_| is updated or cleaned up depending on
   // this navigation's activation state.
   if (filter) {
     base::OnceClosure disallowed_callback(base::BindOnce(
         &ContentSubresourceFilterThrottleManager::MaybeShowNotification,
         weak_ptr_factory_.GetWeakPtr()));
     filter->set_first_disallowed_load_callback(std::move(disallowed_callback));
-    activated_frame_hosts_[frame_host] = std::move(filter);
+    frame_host_filter_map_[frame_host] = std::move(filter);
   } else {
-    activated_frame_hosts_.erase(frame_host);
+    frame_host_filter_map_.erase(frame_host);
 
     // If this is for a special url that did not go through the navigation
     // throttles, then based on the parent's activation state, possibly add this
-    // to activated_frame_hosts_.
+    // to frame_host_filter_map_.
     MaybeActivateSubframeSpecialUrls(navigation_handle);
   }
 
@@ -356,11 +386,11 @@ ContentSubresourceFilterThrottleManager::GetParentFrameFilter(
   DCHECK(parent);
 
   // Filter will be null for those special url navigations that were added in
-  // MaybeActivateSubframeSpecialUrls. Return the filter of the first parent
-  // with a non-null filter.
+  // MaybeActivateSubframeSpecialUrls and for subframes with no committed
+  // navigation. Return the filter of the first parent with a non-null filter.
   while (parent) {
-    auto it = activated_frame_hosts_.find(parent);
-    if (it == activated_frame_hosts_.end())
+    auto it = frame_host_filter_map_.find(parent);
+    if (it == frame_host_filter_map_.end())
       return nullptr;
 
     if (it->second)
@@ -368,9 +398,9 @@ ContentSubresourceFilterThrottleManager::GetParentFrameFilter(
     parent = it->first->GetParent();
   }
 
-  // Since null filter is only possible for special navigations of iframes, the
-  // above loop should have found a filter for at least the top level frame,
-  // thus making this unreachable.
+  // Since a null filter is only possible for special navigations of iframes and
+  // aborted loads in a subframe, the above loop should have found a filter for
+  // at least the top level frame, thus making this unreachable.
   NOTREACHED();
   return nullptr;
 }
@@ -381,8 +411,8 @@ void ContentSubresourceFilterThrottleManager::MaybeShowNotification() {
 
   // This shouldn't happen normally, but in the rare case that an IPC from a
   // previous page arrives late we should guard against it.
-  auto it = activated_frame_hosts_.find(web_contents()->GetMainFrame());
-  if (it == activated_frame_hosts_.end() ||
+  auto it = frame_host_filter_map_.find(web_contents()->GetMainFrame());
+  if (it == frame_host_filter_map_.end() ||
       it->second->activation_state().activation_level !=
           mojom::ActivationLevel::kEnabled) {
     return;
@@ -400,7 +430,7 @@ ContentSubresourceFilterThrottleManager::EnsureRulesetHandle() {
 
 void ContentSubresourceFilterThrottleManager::
     DestroyRulesetHandleIfNoLongerUsed() {
-  if (activated_frame_hosts_.size() + ongoing_activation_throttles_.size() ==
+  if (frame_host_filter_map_.size() + ongoing_activation_throttles_.size() ==
       0u) {
     ruleset_handle_.reset();
   }
@@ -455,8 +485,8 @@ void ContentSubresourceFilterThrottleManager::MaybeActivateSubframeSpecialUrls(
 
   content::RenderFrameHost* parent = navigation_handle->GetParentFrame();
   DCHECK(parent);
-  if (base::Contains(activated_frame_hosts_, parent))
-    activated_frame_hosts_[frame_host] = nullptr;
+  if (base::Contains(frame_host_filter_map_, parent))
+    frame_host_filter_map_[frame_host] = nullptr;
 }
 
 }  // namespace subresource_filter

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h

@@ -159,10 +159,17 @@ class ContentSubresourceFilterThrottleManager
 
   // For each RenderFrameHost where the last committed load has subresource
   // filtering activated, owns the corresponding AsyncDocumentSubresourceFilter.
-  // It is possible for a frame to have a null filter.
+  // A null filter indicates that the filter should be inherited from its
+  // parent if the parent has one. This is possible if the last load was a
+  // special navigation (see MaybeActivateSubframeSpecialUrls) or if no
+  // navigations have committed.
   std::map<content::RenderFrameHost*,
            std::unique_ptr<AsyncDocumentSubresourceFilter>>
-      activated_frame_hosts_;
+      frame_host_filter_map_;
+
+  // Set of RenderFrameHosts that have had at least one committed or aborted
+  // navigation. Main frames with only aborted navigations are not included.
+  std::set<content::RenderFrameHost*> navigated_frames_;
 
   // For each ongoing navigation that requires activation state computation,
   // keeps track of the throttle that is carrying out that computation, so that