Enable the subresource filter for frames with a doc.write-aborted load

Subframes which have their initial load aborted (e.g. due to a
Document.write() call) currently never have the subresource filter
activated. We change the behavior so that a failed provisional load
triggers filter creation if a filter has not been already set. As no
navigation has occurred, we use the parent's activation in this case.

In line with this change, we rename a number of variables and
functions. While this change will affect some SubresourceFilter.*
histograms, we don't version them as we expect the number of affected
frames to be small.

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

chrome/browser/subresource_filter/ad_tagging_browsertest.cc

@@ -87,6 +87,38 @@ class AdTaggingBrowserTest : public SubresourceFilterBrowserTest {
     return CreateDocWrittenFrameImpl(adapter, true /* ad_script */);
   }
 
+  // Creates a frame and aborts the initial load with a doc.write. Returns after
+  // navigation has completed.
+  content::RenderFrameHost* CreateFrameWithDocWriteAbortedLoad(
+      const content::ToRenderFrameHost& adapter) {
+    return CreateFrameWithDocWriteAbortedLoadImpl(adapter,
+                                                  false /* ad_script */);
+  }
+
+  // Creates a frame and aborts the initial load with a doc.write. The script
+  // creating the frame is an ad script. Returns after navigation has completed.
+  content::RenderFrameHost* CreateFrameWithDocWriteAbortedLoadFromAdScript(
+      const content::ToRenderFrameHost& adapter) {
+    return CreateFrameWithDocWriteAbortedLoadImpl(adapter,
+                                                  true /* ad_script */);
+  }
+
+  // Creates a frame and aborts the initial load with a window.stop. Returns
+  // after navigation has completed.
+  content::RenderFrameHost* CreateFrameWithWindowStopAbortedLoad(
+      const content::ToRenderFrameHost& adapter) {
+    return CreateFrameWithWindowStopAbortedLoadImpl(adapter,
+                                                    false /* ad_script */);
+  }
+
+  // Creates a frame and aborts the initial load with a window.stop. The script
+  // creating the frame is an ad script. Returns after navigation has completed.
+  content::RenderFrameHost* CreateFrameWithWindowStopAbortedLoadFromAdScript(
+      const content::ToRenderFrameHost& adapter) {
+    return CreateFrameWithWindowStopAbortedLoadImpl(adapter,
+                                                    true /* ad_script */);
+  }
+
   // Given a RenderFrameHost, navigates the page to the given |url| and waits
   // for the navigation to complete before returning.
   void NavigateFrame(content::RenderFrameHost* render_frame_host,
@@ -112,6 +144,14 @@ class AdTaggingBrowserTest : public SubresourceFilterBrowserTest {
       const content::ToRenderFrameHost& adapter,
       bool ad_script);
 
+  content::RenderFrameHost* CreateFrameWithDocWriteAbortedLoadImpl(
+      const content::ToRenderFrameHost& adapter,
+      bool ad_script);
+
+  content::RenderFrameHost* CreateFrameWithWindowStopAbortedLoadImpl(
+      const content::ToRenderFrameHost& adapter,
+      bool ad_script);
+
   uint32_t frame_count_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(AdTaggingBrowserTest);
@@ -159,6 +199,48 @@ content::RenderFrameHost* AdTaggingBrowserTest::CreateDocWrittenFrameImpl(
       web_contents, base::BindRepeating(&content::FrameMatchesName, name));
 }
 
+content::RenderFrameHost*
+AdTaggingBrowserTest::CreateFrameWithDocWriteAbortedLoadImpl(
+    const content::ToRenderFrameHost& adapter,
+    bool ad_script) {
+  content::RenderFrameHost* rfh = adapter.render_frame_host();
+  std::string name = GetUniqueFrameName();
+  std::string script =
+      base::StringPrintf("%s('%s');",
+                         ad_script ? "createAdFrameWithDocWriteAbortedLoad"
+                                   : "createFrameWithDocWriteAbortedLoad",
+                         name.c_str());
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(rfh);
+  content::TestNavigationObserver navigation_observer(web_contents, 1);
+  EXPECT_TRUE(content::ExecuteScript(rfh, script));
+  navigation_observer.Wait();
+  EXPECT_FALSE(navigation_observer.last_navigation_succeeded());
+  return content::FrameMatchingPredicate(
+      web_contents, base::BindRepeating(&content::FrameMatchesName, name));
+}
+
+content::RenderFrameHost*
+AdTaggingBrowserTest::CreateFrameWithWindowStopAbortedLoadImpl(
+    const content::ToRenderFrameHost& adapter,
+    bool ad_script) {
+  content::RenderFrameHost* rfh = adapter.render_frame_host();
+  std::string name = GetUniqueFrameName();
+  std::string script =
+      base::StringPrintf("%s('%s');",
+                         ad_script ? "createAdFrameWithWindowStopAbortedLoad"
+                                   : "createFrameWithWindowStopAbortedLoad",
+                         name.c_str());
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(rfh);
+  content::TestNavigationObserver navigation_observer(web_contents, 1);
+  EXPECT_TRUE(content::ExecuteScript(rfh, script));
+  navigation_observer.Wait();
+  EXPECT_FALSE(navigation_observer.last_navigation_succeeded());
+  return content::FrameMatchingPredicate(
+      web_contents, base::BindRepeating(&content::FrameMatchesName, name));
+}
+
 // Given a RenderFrameHost, navigates the page to the given |url| and waits
 // for the navigation to complete before returning.
 void AdTaggingBrowserTest::NavigateFrame(
@@ -328,6 +410,76 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, SameOriginFrameTagging) {
   EXPECT_TRUE(*observer.GetIsAdSubframe(ad_frame->GetFrameTreeNodeId()));
 }
 
+// Test that frames with an aborted initial load due to a doc.write are still
+// correctly tagged as ads or not.
+IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest,
+                       FramesWithDocWriteAbortedLoad_StillCorrectlyTagged) {
+  TestSubresourceFilterObserver observer(web_contents());
+
+  // Main frame.
+  ui_test_utils::NavigateToURL(browser(), GetURL("frame_factory.html"));
+
+  // Vanilla child.
+  content::RenderFrameHost* vanilla_frame_with_aborted_load =
+      CreateFrameWithDocWriteAbortedLoad(GetWebContents());
+  // We expect an empty Optional<bool> as no successful subframe navigation
+  // occurred and it is not set as an ad frame.
+  EXPECT_FALSE(observer
+                   .GetIsAdSubframe(
+                       vanilla_frame_with_aborted_load->GetFrameTreeNodeId())
+                   .has_value());
+
+  // Child created by ad script.
+  content::RenderFrameHost* ad_frame_with_aborted_load =
+      CreateFrameWithDocWriteAbortedLoadFromAdScript(GetWebContents());
+  EXPECT_TRUE(*observer.GetIsAdSubframe(
+      ad_frame_with_aborted_load->GetFrameTreeNodeId()));
+
+  // Child with ad parent.
+  content::RenderFrameHost* ad_frame = CreateSrcFrameFromAdScript(
+      GetWebContents(), GetURL("frame_factory.html"));
+  content::RenderFrameHost* child_frame_of_ad_with_aborted_load =
+      CreateFrameWithDocWriteAbortedLoad(ad_frame);
+  EXPECT_TRUE(*observer.GetIsAdSubframe(ad_frame->GetFrameTreeNodeId()));
+  EXPECT_TRUE(*observer.GetIsAdSubframe(
+      child_frame_of_ad_with_aborted_load->GetFrameTreeNodeId()));
+}
+
+// Test that frames with an aborted initial load due to a window.stop are still
+// correctly tagged as ads or not.
+IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest,
+                       FramesWithWindowStopAbortedLoad_StillCorrectlyTagged) {
+  TestSubresourceFilterObserver observer(web_contents());
+
+  // Main frame.
+  ui_test_utils::NavigateToURL(browser(), GetURL("frame_factory.html"));
+
+  // Vanilla child.
+  content::RenderFrameHost* vanilla_frame_with_aborted_load =
+      CreateFrameWithWindowStopAbortedLoad(GetWebContents());
+  // We expect an empty Optional<bool> as no successful subframe navigation
+  // occurred and it is not set as an ad frame.
+  EXPECT_FALSE(observer
+                   .GetIsAdSubframe(
+                       vanilla_frame_with_aborted_load->GetFrameTreeNodeId())
+                   .has_value());
+
+  // Child created by ad script.
+  content::RenderFrameHost* ad_frame_with_aborted_load =
+      CreateFrameWithWindowStopAbortedLoadFromAdScript(GetWebContents());
+  EXPECT_TRUE(*observer.GetIsAdSubframe(
+      ad_frame_with_aborted_load->GetFrameTreeNodeId()));
+
+  // Child with ad parent.
+  content::RenderFrameHost* ad_frame = CreateSrcFrameFromAdScript(
+      GetWebContents(), GetURL("frame_factory.html"));
+  content::RenderFrameHost* child_frame_of_ad_with_aborted_load =
+      CreateFrameWithWindowStopAbortedLoad(ad_frame);
+  EXPECT_TRUE(*observer.GetIsAdSubframe(ad_frame->GetFrameTreeNodeId()));
+  EXPECT_TRUE(*observer.GetIsAdSubframe(
+      child_frame_of_ad_with_aborted_load->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

@@ -658,6 +658,50 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
   observer.Wait();
 }
 
+// Test that resources in frames with an aborted initial load due to a doc.write
+// are still tagged as ads.
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       FrameWithDocWriteAbortedLoad_StillTaggedAsAd) {
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetWithRules({testing::CreateSuffixRule("ad=true")}));
+  // Block ad resources.
+  Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
+                       subresource_filter::ActivationScope::ALL_SITES);
+  ResetConfiguration(std::move(config));
+
+  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"));
+  // 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.
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       FrameWithWindowStopAbortedLoad_StillTaggedAsAd) {
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetWithRules({testing::CreateSuffixRule("ad=true")}));
+  // Block ad resources.
+  Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
+                       subresource_filter::ActivationScope::ALL_SITES);
+  ResetConfiguration(std::move(config));
+
+  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_ad_resource.html"));
+  // Check the load was blocked.
+  EXPECT_EQ(base::ASCIIToUTF16("failed"), title_watcher.WaitAndGetTitle());
+}
+
 // Tests checking how histograms are recorded. ---------------------------------
 
 namespace {

chrome/test/data/ad_tagging/ad_script.js

@@ -34,3 +34,27 @@ async function createDocWrittenAdFrame(name, base_url) {
   frame.contentDocument.write(doc_text);
   frame.contentDocument.close();
 }
+
+function createAdFrameWithDocWriteAbortedLoad(name) {
+  let frame = document.createElement('iframe');
+  frame.name = name;
+
+  // slow takes 100 seconds to load, plenty of time to overwrite the
+  // provisional load.
+  frame.src = '/slow?100';
+  document.body.appendChild(frame);
+  frame.contentDocument.open();
+  frame.contentDocument.write('<html><head></head><body></body></html>');
+  frame.contentDocument.close();
+}
+
+function createAdFrameWithWindowStopAbortedLoad(name) {
+  let frame = document.createElement('iframe');
+  frame.name = name;
+
+  // slow takes 100 seconds to load, plenty of time to overwrite the
+  // provisional load.
+  frame.src = '/slow?100';
+  document.body.appendChild(frame);
+  frame.contentWindow.stop();
+}

chrome/test/data/ad_tagging/create_frame.js

@@ -34,3 +34,27 @@ async function createDocWrittenFrame(name, base_url) {
   frame.contentDocument.write(doc_text);
   frame.contentDocument.close();
 }
+
+function createFrameWithDocWriteAbortedLoad(name) {
+  let frame = document.createElement('iframe');
+  frame.name = name;
+
+  // slow takes 100 seconds to load, plenty of time to overwrite the
+  // provisional load.
+  frame.src = '/slow?100';
+  document.body.appendChild(frame);
+  frame.contentDocument.open();
+  frame.contentDocument.write('<html><head></head><body></body></html>');
+  frame.contentDocument.close();
+}
+
+function createFrameWithWindowStopAbortedLoad(name) {
+  let frame = document.createElement('iframe');
+  frame.name = name;
+
+  // slow takes 100 seconds to load, plenty of time to overwrite the
+  // provisional load.
+  frame.src = '/slow?100';
+  document.body.appendChild(frame);
+  frame.contentWindow.stop();
+}

chrome/test/data/subresource_filter/docwrite_loads_ad_resource.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");
+  doc = iframe.contentDocument;
+  doc.open();
+  doc.write("<html><body>Rewritten. <img src='/ad_tagging/pixel.png?ad=true'"
+    + " onload='parent.document.title = \"loaded\";'"
+    + " onerror='parent.document.title = \"failed\";'></body></html>");
+  doc.close();
+</script>
+</body>
+</html>

chrome/test/data/subresource_filter/window_stop_loads_ad_resource.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.contentWindow.stop();
+  iframe.contentDocument.body.innerHTML = ("Rewritten."
+    + " <img src='/ad_tagging/pixel.png?ad=true'"
+    + " onload='parent.document.title = \"loaded\";'"
+    + " onerror='parent.document.title = \"failed\";'>");
+</script>
+</body>
+</html>

components/subresource_filter/content/renderer/subresource_filter_agent.cc

@@ -55,8 +55,8 @@ SubresourceFilterAgent::SubresourceFilterAgent(
 
 SubresourceFilterAgent::~SubresourceFilterAgent() {
   // Filter may outlive us, so reset the ad tracker.
-  if (filter_for_last_committed_load_)
-    filter_for_last_committed_load_->set_ad_resource_tracker(nullptr);
+  if (filter_for_last_created_document_)
+    filter_for_last_created_document_->set_ad_resource_tracker(nullptr);
 }
 
 GURL SubresourceFilterAgent::GetDocumentURL() {
@@ -67,14 +67,19 @@ bool SubresourceFilterAgent::IsMainFrame() {
   return render_frame()->IsMainFrame();
 }
 
-void SubresourceFilterAgent::SetSubresourceFilterForCommittedLoad(
+bool SubresourceFilterAgent::HasDocumentLoader() {
+  return render_frame()->GetWebFrame()->GetDocumentLoader();
+}
+
+void SubresourceFilterAgent::SetSubresourceFilterForCurrentDocument(
     std::unique_ptr<blink::WebDocumentSubresourceFilter> filter) {
   blink::WebLocalFrame* web_frame = render_frame()->GetWebFrame();
+  DCHECK(web_frame->GetDocumentLoader());
   web_frame->GetDocumentLoader()->SetSubresourceFilter(filter.release());
 }
 
 void SubresourceFilterAgent::
-    SignalFirstSubresourceDisallowedForCommittedLoad() {
+    SignalFirstSubresourceDisallowedForCurrentDocument() {
   GetSubresourceFilterHost()->DidDisallowFirstSubresource();
 }
 
@@ -96,7 +101,6 @@ void SubresourceFilterAgent::SetIsAdSubframe(
   render_frame()->GetWebFrame()->SetIsAdSubframe(ad_frame_type);
 }
 
-// static
 mojom::ActivationState SubresourceFilterAgent::GetParentActivationState(
     content::RenderFrame* render_frame) {
   blink::WebFrame* parent =
@@ -104,13 +108,13 @@ mojom::ActivationState SubresourceFilterAgent::GetParentActivationState(
   if (parent && parent->IsWebLocalFrame()) {
     auto* agent = SubresourceFilterAgent::Get(
         content::RenderFrame::FromWebFrame(parent->ToWebLocalFrame()));
-    if (agent && agent->filter_for_last_committed_load_)
-      return agent->filter_for_last_committed_load_->activation_state();
+    if (agent && agent->filter_for_last_created_document_)
+      return agent->filter_for_last_created_document_->activation_state();
   }
   return mojom::ActivationState();
 }
 
-void SubresourceFilterAgent::RecordHistogramsOnLoadCommitted(
+void SubresourceFilterAgent::RecordHistogramsOnFilterCreation(
     const mojom::ActivationState& activation_state) {
   // Note: mojom::ActivationLevel used to be called mojom::ActivationState, the
   // legacy name is kept for the histogram.
@@ -129,8 +133,8 @@ void SubresourceFilterAgent::RecordHistogramsOnLoadCommitted(
   }
 }
 
-void SubresourceFilterAgent::ResetInfoForNextCommit() {
-  activation_state_for_next_commit_ = mojom::ActivationState();
+void SubresourceFilterAgent::ResetInfoForNextDocument() {
+  activation_state_for_next_document_ = mojom::ActivationState();
 }
 
 mojom::SubresourceFilterHost*
@@ -150,7 +154,7 @@ void SubresourceFilterAgent::OnSubresourceFilterAgentRequest(
 void SubresourceFilterAgent::ActivateForNextCommittedLoad(
     mojom::ActivationStatePtr activation_state,
     blink::mojom::AdFrameType ad_frame_type) {
-  activation_state_for_next_commit_ = *activation_state;
+  activation_state_for_next_document_ = *activation_state;
   if (ad_frame_type != blink::mojom::AdFrameType::kNonAd) {
     SetIsAdSubframe(ad_frame_type);
   }
@@ -161,22 +165,21 @@ void SubresourceFilterAgent::OnDestruct() {
 }
 
 void SubresourceFilterAgent::DidCreateNewDocument() {
-  // Filter may outlive us, so reset the ad tracker.
-  if (filter_for_last_committed_load_)
-    filter_for_last_committed_load_->set_ad_resource_tracker(nullptr);
-  filter_for_last_committed_load_.reset();
-
-  // TODO(csharrison): Use WebURL and WebSecurityOrigin for efficiency here,
-  // which require changes to the unit tests.
+  // TODO(csharrison): Use WebURL and WebSecurityOrigin for efficiency here and
+  // in MaybeCreateFilterForDocument, which requires changes to the unit tests.
   const GURL& url = GetDocumentURL();
-
   if (first_document_) {
     first_document_ = false;
-
-    // Local subframes will first navigate to kAboutBlankURL. Frames created by
-    // the browser initialize the LocalFrame before creating
-    // RenderFrameObservers, so the about:blank document isn't observed. We only
-    // care about local subframes.
+    DCHECK(!filter_for_last_created_document_);
+
+    // Local subframes will first create an initial empty document (with url
+    // kAboutBlankURL) no matter what the src is set to (or if it is not set).
+    // There is then a second document created with url equal to the set src (or
+    // kAboutBlankURL if unset). See DidFailProvisionalLoad for handling a
+    // failure in the load of this second document. Frames created by the
+    // browser initialize the LocalFrame before creating RenderFrameObservers,
+    // so the initial empty document isn't observed. We only care about local
+    // subframes.
     if (url == url::kAboutBlankURL) {
       if (IsAdSubframe())
         SendFrameIsAdSubframe();
@@ -184,19 +187,30 @@ void SubresourceFilterAgent::DidCreateNewDocument() {
     }
   }
 
-  bool use_parent_activation = !IsMainFrame() && ShouldUseParentActivation(url);
+  MaybeCreateFilterForDocument(ShouldUseParentActivation(url));
+}
+
+void SubresourceFilterAgent::MaybeCreateFilterForDocument(
+    bool should_use_parent_activation) {
+  // Filter may outlive us, so reset the ad tracker.
+  if (filter_for_last_created_document_)
+    filter_for_last_created_document_->set_ad_resource_tracker(nullptr);
+  filter_for_last_created_document_.reset();
+
+  const GURL& url = GetDocumentURL();
 
   const mojom::ActivationState activation_state =
-      use_parent_activation ? GetParentActivationState(render_frame())
-                            : activation_state_for_next_commit_;
+      (!IsMainFrame() && should_use_parent_activation)
+          ? GetParentActivationState(render_frame())
+          : activation_state_for_next_document_;
 
-  ResetInfoForNextCommit();
+  ResetInfoForNextDocument();
 
   // Do not pollute the histograms for empty main frame documents.
   if (IsMainFrame() && !url.SchemeIsHTTPOrHTTPS() && !url.SchemeIsFile())
     return;
 
-  RecordHistogramsOnLoadCommitted(activation_state);
+  RecordHistogramsOnFilterCreation(activation_state);
   if (activation_state.activation_level == mojom::ActivationLevel::kDisabled ||
       !ruleset_dealer_->IsRulesetFileAvailable())
     return;
@@ -206,33 +220,45 @@ void SubresourceFilterAgent::DidCreateNewDocument() {
   if (!ruleset)
     return;
 
-  base::OnceClosure first_disallowed_load_callback(base::BindOnce(
-      &SubresourceFilterAgent::SignalFirstSubresourceDisallowedForCommittedLoad,
-      AsWeakPtr()));
+  base::OnceClosure first_disallowed_load_callback(
+      base::BindOnce(&SubresourceFilterAgent::
+                         SignalFirstSubresourceDisallowedForCurrentDocument,
+                     AsWeakPtr()));
   auto filter = std::make_unique<WebDocumentSubresourceFilterImpl>(
       url::Origin::Create(url), activation_state, std::move(ruleset),
       std::move(first_disallowed_load_callback));
   filter->set_ad_resource_tracker(ad_resource_tracker_.get());
-  filter_for_last_committed_load_ = filter->AsWeakPtr();
-  SetSubresourceFilterForCommittedLoad(std::move(filter));
+  filter_for_last_created_document_ = filter->AsWeakPtr();
+  SetSubresourceFilterForCurrentDocument(std::move(filter));
 }
 
 void SubresourceFilterAgent::DidFailProvisionalLoad() {
   // TODO(engedy): Add a test with `frame-ancestor` violation to exercise this.
-  ResetInfoForNextCommit();
+  ResetInfoForNextDocument();
+
+  // This is necessary to account for when an initial frame load is aborted,
+  // for example due to a document.write or window.stop. We skip this if
+  // |filter_for_last_created_document_|, maintaining the prior behavior when a
+  // filter has already been created. We also skip if there is no document
+  // loader as it may have been detached due to a cross-origin navigation.
+  if (!filter_for_last_created_document_ && HasDocumentLoader()) {
+    // Use the parent as a committed load has not occurred so an activation
+    // state won't have been sent.
+    MaybeCreateFilterForDocument(true);
+  }
 }
 
 void SubresourceFilterAgent::DidFinishLoad() {
-  if (!filter_for_last_committed_load_)
+  if (!filter_for_last_created_document_)
     return;
   const auto& statistics =
-      filter_for_last_committed_load_->filter().statistics();
+      filter_for_last_created_document_->filter().statistics();
   SendDocumentLoadStatistics(statistics);
 }
 
 void SubresourceFilterAgent::WillCreateWorkerFetchContext(
     blink::WebWorkerFetchContext* worker_fetch_context) {
-  if (!filter_for_last_committed_load_)
+  if (!filter_for_last_created_document_)
     return;
   if (!ruleset_dealer_->IsRulesetFileAvailable())
     return;
@@ -243,10 +269,10 @@ void SubresourceFilterAgent::WillCreateWorkerFetchContext(
   worker_fetch_context->SetSubresourceFilterBuilder(
       std::make_unique<WebDocumentSubresourceFilterImpl::BuilderImpl>(
           url::Origin::Create(GetDocumentURL()),
-          filter_for_last_committed_load_->filter().activation_state(),
+          filter_for_last_created_document_->filter().activation_state(),
           std::move(ruleset_file),
           base::BindOnce(&SubresourceFilterAgent::
-                             SignalFirstSubresourceDisallowedForCommittedLoad,
+                             SignalFirstSubresourceDisallowedForCurrentDocument,
                          AsWeakPtr())));
 }
 

components/subresource_filter/content/renderer/subresource_filter_agent.h

@@ -55,14 +55,19 @@ class SubresourceFilterAgent
 
   virtual bool IsMainFrame();
 
+  virtual bool HasDocumentLoader();
+
   // Injects the provided subresource |filter| into the DocumentLoader
-  // orchestrating the most recently committed load.
-  virtual void SetSubresourceFilterForCommittedLoad(
+  // orchestrating the most recently created document. If this method is called
+  // as the result of a new document element instead of a new document (e.g.
+  // due to a document.write), we reuse the previous DocumentLoader.
+  virtual void SetSubresourceFilterForCurrentDocument(
       std::unique_ptr<blink::WebDocumentSubresourceFilter> filter);
 
   // Informs the browser that the first subresource load has been disallowed for
-  // the most recently committed load. Not called if all resources are allowed.
-  virtual void SignalFirstSubresourceDisallowedForCommittedLoad();
+  // the most recently created document. Not called if all resources are
+  // allowed.
+  virtual void SignalFirstSubresourceDisallowedForCurrentDocument();
 
   // Sends statistics about the DocumentSubresourceFilter's work to the browser.
   virtual void SendDocumentLoadStatistics(
@@ -83,12 +88,12 @@ class SubresourceFilterAgent
  private:
   // Assumes that the parent will be in a local frame relative to this one, upon
   // construction.
-  static mojom::ActivationState GetParentActivationState(
+  virtual mojom::ActivationState GetParentActivationState(
       content::RenderFrame* render_frame);
 
-  void RecordHistogramsOnLoadCommitted(
+  void RecordHistogramsOnFilterCreation(
       const mojom::ActivationState& activation_state);
-  void ResetInfoForNextCommit();
+  void ResetInfoForNextDocument();
 
   mojom::SubresourceFilterHost* GetSubresourceFilterHost();
 
@@ -102,10 +107,12 @@ class SubresourceFilterAgent
   void DidFinishLoad() override;
   void WillCreateWorkerFetchContext(blink::WebWorkerFetchContext*) override;
 
+  void MaybeCreateFilterForDocument(bool should_use_parent_activation);
+
   // Owned by the ChromeContentRendererClient and outlives us.
   UnverifiedRulesetDealer* ruleset_dealer_;
 
-  mojom::ActivationState activation_state_for_next_commit_;
+  mojom::ActivationState activation_state_for_next_document_;
 
   // Tracks all ad resource observers.
   std::unique_ptr<AdResourceTracker> ad_resource_tracker_;
@@ -116,12 +123,12 @@ class SubresourceFilterAgent
 
   mojo::AssociatedReceiver<mojom::SubresourceFilterAgent> receiver_{this};
 
-  // If a document has been created for this frame before. The first document
+  // If a document hasn't been created for this frame before. The first document
   // for a new local subframe should be about:blank.
   bool first_document_ = true;
 
   base::WeakPtr<WebDocumentSubresourceFilterImpl>
-      filter_for_last_committed_load_;
+      filter_for_last_created_document_;
 
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterAgent);
 };

components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc

@@ -51,18 +51,21 @@ class SubresourceFilterAgentUnderTest : public SubresourceFilterAgent {
   ~SubresourceFilterAgentUnderTest() override = default;
 
   MOCK_METHOD0(GetDocumentURL, GURL());
-  MOCK_METHOD0(OnSetSubresourceFilterForCommittedLoadCalled, void());
-  MOCK_METHOD0(SignalFirstSubresourceDisallowedForCommittedLoad, void());
+  MOCK_METHOD0(OnSetSubresourceFilterForCurrentDocumentCalled, void());
+  MOCK_METHOD0(SignalFirstSubresourceDisallowedForCurrentDocument, void());
   MOCK_METHOD1(SendDocumentLoadStatistics,
                void(const mojom::DocumentLoadStatistics&));
   MOCK_METHOD0(SendFrameIsAdSubframe, void());
 
-  bool IsMainFrame() override { return true; }
+  bool IsMainFrame() override { return is_main_frame_; }
+  void SetIsMainFrame(bool value) { is_main_frame_ = value; }
 
-  void SetSubresourceFilterForCommittedLoad(
+  bool HasDocumentLoader() override { return true; }
+
+  void SetSubresourceFilterForCurrentDocument(
       std::unique_ptr<blink::WebDocumentSubresourceFilter> filter) override {
     last_injected_filter_ = std::move(filter);
-    OnSetSubresourceFilterForCommittedLoadCalled();
+    OnSetSubresourceFilterForCurrentDocumentCalled();
   }
 
   bool IsAdSubframe() override { return is_ad_subframe_; }
@@ -79,11 +82,24 @@ class SubresourceFilterAgentUnderTest : public SubresourceFilterAgent {
     return std::move(last_injected_filter_);
   }
 
+  void SetParentActivationState(mojom::ActivationLevel level) {
+    mojom::ActivationState state;
+    state.activation_level = level;
+    parent_activation_state_ = state;
+  }
+
   using SubresourceFilterAgent::ActivateForNextCommittedLoad;
 
  private:
+  mojom::ActivationState GetParentActivationState(
+      content::RenderFrame*) override {
+    return parent_activation_state_;
+  }
+
   std::unique_ptr<blink::WebDocumentSubresourceFilter> last_injected_filter_;
   bool is_ad_subframe_ = false;
+  bool is_main_frame_ = true;
+  mojom::ActivationState parent_activation_state_;
 
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterAgentUnderTest);
 };
@@ -161,22 +177,22 @@ class SubresourceFilterAgentTest : public ::testing::Test {
   void FinishLoad() { agent_as_rfo()->DidFinishLoad(); }
 
   void ExpectSubresourceFilterGetsInjected() {
-    EXPECT_CALL(*agent(), GetDocumentURL());
-    EXPECT_CALL(*agent(), OnSetSubresourceFilterForCommittedLoadCalled());
+    EXPECT_CALL(*agent(), GetDocumentURL()).Times(::testing::Between(1, 2));
+    EXPECT_CALL(*agent(), OnSetSubresourceFilterForCurrentDocumentCalled());
   }
 
   void ExpectNoSubresourceFilterGetsInjected() {
     EXPECT_CALL(*agent(), GetDocumentURL()).Times(::testing::AtLeast(0));
-    EXPECT_CALL(*agent(), OnSetSubresourceFilterForCommittedLoadCalled())
+    EXPECT_CALL(*agent(), OnSetSubresourceFilterForCurrentDocumentCalled())
         .Times(0);
   }
 
   void ExpectSignalAboutFirstSubresourceDisallowed() {
-    EXPECT_CALL(*agent(), SignalFirstSubresourceDisallowedForCommittedLoad());
+    EXPECT_CALL(*agent(), SignalFirstSubresourceDisallowedForCurrentDocument());
   }
 
   void ExpectNoSignalAboutFirstSubresourceDisallowed() {
-    EXPECT_CALL(*agent(), SignalFirstSubresourceDisallowedForCommittedLoad())
+    EXPECT_CALL(*agent(), SignalFirstSubresourceDisallowedForCurrentDocument())
         .Times(0);
   }
 
@@ -531,6 +547,22 @@ TEST_F(SubresourceFilterAgentTest,
   filter->ReportDisallowedLoad();
 }
 
+TEST_F(SubresourceFilterAgentTest, FailedInitialLoad_FilterInjectedOnFailure) {
+  ASSERT_NO_FATAL_FAILURE(
+      SetTestRulesetToDisallowURLsWithPathSuffix("somethingNotMatched"));
+
+  agent()->SetIsMainFrame(false);
+  agent()->SetParentActivationState(mojom::ActivationLevel::kEnabled);
+
+  ExpectNoSubresourceFilterGetsInjected();
+  EXPECT_CALL(*agent(), GetDocumentURL())
+      .WillRepeatedly(::testing::Return(GURL("about:blank")));
+  StartLoadAndSetActivationState(mojom::ActivationLevel::kEnabled);
+
+  ExpectSubresourceFilterGetsInjected();
+  agent_as_rfo()->DidFailProvisionalLoad();
+}
+
 TEST_F(SubresourceFilterAgentTest,
        DryRun_IsAssociatedWithAdSubframeforDocumentOrDedicatedWorker) {
   ASSERT_NO_FATAL_FAILURE(
@@ -573,7 +605,7 @@ TEST_F(SubresourceFilterAgentTest, DryRun_SendsFrameIsAdSubframe) {
       .WillOnce(::testing::Return(GURL("about:blank")));
   agent_as_rfo()->DidCreateNewDocument();
   EXPECT_CALL(*agent(), GetDocumentURL())
-      .WillOnce(::testing::Return(GURL("about:blank")));
+      .WillRepeatedly(::testing::Return(GURL("about:blank")));
   agent_as_rfo()->DidCreateNewDocument();
 }