[Subresource Filter] Handle user reload entirely within component

This CL changes the subresource filter to handle the action of the user
initiating a reload of a page with blocked ads entirely within the
component, rather than delegating to the embedder. This change
concretely means that WebLayer now obtains this behavior for free. To
test proper integration in WebLayer, this CL also adds browsertests of
the expected behavior in //weblayer. We disable these browsertests on
Windows due to https://crbug.com/1152429.

Bug: 1116095
Change-Id: I270aff6251d0a498af77b9748bf129036e4d3541
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2606345
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844144}

chrome/browser/subresource_filter/chrome_subresource_filter_client.cc

@@ -78,14 +78,6 @@ ChromeSubresourceFilterClient* ChromeSubresourceFilterClient::FromWebContents(
       throttle_manager->client());
 }
 
-void ChromeSubresourceFilterClient::OnReloadRequested() {
-  // TODO(crbug.com/1116095): Once ContentSubresourceFilterThrottleManager knows
-  // about ProfileInteractionManager, this method can move entirely into
-  // ContentSubresourceFilterThrottleManager::OnReloadRequested() and
-  // SubresourceFilterClient::OnReloadRequested() can be eliminated.
-  profile_interaction_manager_->OnReloadRequested();
-}
-
 void ChromeSubresourceFilterClient::ShowNotification() {
   const GURL& top_level_url = web_contents_->GetLastCommittedURL();
   if (profile_context_->settings_manager()->ShouldShowUIForSite(

chrome/browser/subresource_filter/chrome_subresource_filter_client.h

@@ -52,7 +52,6 @@ class ChromeSubresourceFilterClient
   GetSafeBrowsingDatabaseManager() override;
   subresource_filter::ProfileInteractionManager* GetProfileInteractionManager()
       override;
-  void OnReloadRequested() override;
 
  private:
   void ShowUI(const GURL& url);

chrome/browser/subresource_filter/subresource_filter_settings_browsertest.cc

@@ -183,7 +183,8 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterSettingsBrowserTest,
 
   // Allowlist via a reload.
   content::TestNavigationObserver navigation_observer(web_contents(), 1);
-  ChromeSubresourceFilterClient::FromWebContents(web_contents())
+  subresource_filter::ContentSubresourceFilterThrottleManager::FromWebContents(
+      web_contents())
       ->OnReloadRequested();
   navigation_observer.Wait();
 
@@ -202,7 +203,8 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterSettingsBrowserTest,
 
   // Allowlist via a reload.
   content::TestNavigationObserver navigation_observer(web_contents(), 1);
-  ChromeSubresourceFilterClient::FromWebContents(web_contents())
+  subresource_filter::ContentSubresourceFilterThrottleManager::FromWebContents(
+      web_contents())
       ->OnReloadRequested();
   navigation_observer.Wait();
 

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc

@@ -492,7 +492,9 @@ ContentSubresourceFilterThrottleManager::LoadPolicyForLastCommittedNavigation(
 }
 
 void ContentSubresourceFilterThrottleManager::OnReloadRequested() {
-  client_->OnReloadRequested();
+  if (auto* profile_interaction_manager =
+          client_->GetProfileInteractionManager())
+    profile_interaction_manager->OnReloadRequested();
 }
 
 // static

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager_unittest.cc

@@ -194,7 +194,6 @@ class TestSubresourceFilterClient : public SubresourceFilterClient {
       override {
     return nullptr;
   }
-  void OnReloadRequested() override {}
 
   void CreateSafeBrowsingDatabaseManager() {
     database_manager_ =

components/subresource_filter/content/browser/subresource_filter_client.h

@@ -42,10 +42,6 @@ class SubresourceFilterClient {
   // longer calls into it, replacing this method with a getter for
   // SubresourceFilterProfileContext.
   virtual ProfileInteractionManager* GetProfileInteractionManager() = 0;
-
-  // Invoked when the user has requested a reload of a page with blocked ads
-  // (e.g., via an infobar).
-  virtual void OnReloadRequested() = 0;
 };
 
 }  // namespace subresource_filter

components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc

@@ -120,7 +120,6 @@ class MockSubresourceFilterClient : public SubresourceFilterClient {
       const scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>());
   MOCK_METHOD0(GetProfileInteractionManager,
                subresource_filter::ProfileInteractionManager*());
-  MOCK_METHOD0(OnReloadRequested, void());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockSubresourceFilterClient);

weblayer/browser/subresource_filter_browsertest.cc

@@ -13,6 +13,8 @@
 #include "components/subresource_filter/core/browser/subresource_filter_constants.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "net/dns/mock_host_resolver.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "weblayer/browser/browser_process.h"
@@ -85,6 +87,10 @@ class SubresourceFilterBrowserTest : public WebLayerBrowserTest {
       delete;
 
   void SetUpOnMainThread() override {
+    // This test suite does "cross-site" navigations to various domains that
+    // must all resolve to localhost.
+    host_resolver()->AddRule("*", "127.0.0.1");
+
     ASSERT_TRUE(embedded_test_server()->Start());
   }
 
@@ -424,4 +430,80 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, InfoBarPresentation) {
 }
 #endif
 
+// Flaky on Windows. See https://crbug.com/1152429
+#if defined(OS_WIN)
+#define MAYBE_ContentSettingsAllowlistViaReload_DoNotActivate \
+  DISABLED_ContentSettingsAllowlistViaReload_DoNotActivate
+#else
+#define MAYBE_ContentSettingsAllowlistViaReload_DoNotActivate \
+  ContentSettingsAllowlistViaReload_DoNotActivate
+#endif
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       MAYBE_ContentSettingsAllowlistViaReload_DoNotActivate) {
+  auto* web_contents = static_cast<TabImpl*>(shell()->tab())->web_contents();
+
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
+  GURL test_url(
+      embedded_test_server()->GetURL("/frame_with_included_script.html"));
+  ActivateSubresourceFilterInWebContentsForURL(web_contents, test_url);
+
+  NavigateAndWaitForCompletion(test_url, shell());
+  EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
+
+  // Allowlist via a reload.
+  content::TestNavigationObserver navigation_observer(web_contents, 1);
+  subresource_filter::ContentSubresourceFilterThrottleManager::FromWebContents(
+      web_contents)
+      ->OnReloadRequested();
+  navigation_observer.Wait();
+
+  EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
+}
+
+// Flaky on Windows. See https://crbug.com/1152429
+#if defined(OS_WIN)
+#define MAYBE_ContentSettingsAllowlistViaReload_AllowlistIsByDomain \
+  DISABLED_ContentSettingsAllowlistViaReload_AllowlistIsByDomain
+#else
+#define MAYBE_ContentSettingsAllowlistViaReload_AllowlistIsByDomain \
+  ContentSettingsAllowlistViaReload_AllowlistIsByDomain
+#endif
+IN_PROC_BROWSER_TEST_F(
+    SubresourceFilterBrowserTest,
+    MAYBE_ContentSettingsAllowlistViaReload_AllowlistIsByDomain) {
+  auto* web_contents = static_cast<TabImpl*>(shell()->tab())->web_contents();
+
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
+  GURL test_url(
+      embedded_test_server()->GetURL("/frame_with_included_script.html"));
+  ActivateSubresourceFilterInWebContentsForURL(web_contents, test_url);
+
+  NavigateAndWaitForCompletion(test_url, shell());
+  EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
+
+  // Allowlist via a reload.
+  content::TestNavigationObserver navigation_observer(web_contents, 1);
+  subresource_filter::ContentSubresourceFilterThrottleManager::FromWebContents(
+      web_contents)
+      ->OnReloadRequested();
+  navigation_observer.Wait();
+
+  EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
+
+  // Another navigation to the same domain should be allowed too.
+  NavigateAndWaitForCompletion(
+      embedded_test_server()->GetURL("/frame_with_included_script.html?query"),
+      shell());
+  EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
+
+  // A cross site blocklisted navigation should stay activated, however.
+  GURL a_url(embedded_test_server()->GetURL(
+      "a.com", "/frame_with_included_script.html"));
+  ActivateSubresourceFilterInWebContentsForURL(web_contents, a_url);
+  NavigateAndWaitForCompletion(a_url, shell());
+  EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents->GetMainFrame()));
+}
+
 }  // namespace weblayer

weblayer/browser/subresource_filter_client_impl.cc

@@ -78,12 +78,6 @@ void SubresourceFilterClientImpl::CreateThrottleManagerWithClientForWebContents(
           std::make_unique<SubresourceFilterClientImpl>(web_contents), dealer);
 }
 
-void SubresourceFilterClientImpl::OnReloadRequested() {
-  // TODO(crbug.com/1116095): Bring up this flow on Android when user requests
-  // it via the infobar.
-  NOTIMPLEMENTED();
-}
-
 void SubresourceFilterClientImpl::ShowNotification() {
 #if defined(OS_ANDROID)
   // TODO(crbug.com/1116095): Move ChromeSubresourceFilterClient::ShowUI()'s

weblayer/browser/subresource_filter_client_impl.h

@@ -50,7 +50,6 @@ class SubresourceFilterClientImpl
   GetSafeBrowsingDatabaseManager() override;
   subresource_filter::ProfileInteractionManager* GetProfileInteractionManager()
       override;
-  void OnReloadRequested() override;
 
   // Sets the SafeBrowsingDatabaseManager instance used to |database_manager|.
   void set_database_manager_for_testing(