[Subresource Filter] Move force activation logic to component

This CL moves the API and implementation via which devtools enables
forced activation of the subresource filter from //chrome to
//components. This move will enable easy reuse of this logic by WebLayer
in a followup CL. In this CL there is no behavioral change.

The relevant unittests are also ported from //chrome to //components.
It was not possible to port a test of the interaction with content
settings, as that interaction is //chrome-level. However, that check
(that content settings is updated when the component declares to the
client that the page is activated) is covered by other tests in
//chrome's subresource_filter_unittest.cc.

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

chrome/browser/devtools/protocol/page_handler.cc

@@ -4,7 +4,7 @@
 
 #include "chrome/browser/devtools/protocol/page_handler.h"
 
-#include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
+#include "components/subresource_filter/content/browser/devtools_interaction_tracker.h"
 #include "components/webapps/installable/installable_manager.h"
 #include "ui/gfx/image/image.h"
 
@@ -22,10 +22,15 @@ PageHandler::~PageHandler() {
 void PageHandler::ToggleAdBlocking(bool enabled) {
   if (!web_contents())
     return;
-  if (auto* client =
-          ChromeSubresourceFilterClient::FromWebContents(web_contents())) {
-    client->ToggleForceActivationInCurrentWebContents(enabled);
-  }
+
+  // Create the DevtoolsInteractionTracker lazily (note that this call is a
+  // no-op if the object was already created).
+  subresource_filter::DevtoolsInteractionTracker::CreateForWebContents(
+      web_contents());
+
+  subresource_filter::DevtoolsInteractionTracker::FromWebContents(
+      web_contents())
+      ->ToggleForceActivation(enabled);
 }
 
 protocol::Response PageHandler::Enable() {

chrome/browser/subresource_filter/chrome_subresource_filter_client.cc

@@ -107,11 +107,6 @@ ChromeSubresourceFilterClient::OnPageActivationComputed(
 
   subresource_filter::mojom::ActivationLevel effective_activation_level =
       initial_activation_level;
-  if (activated_via_devtools_) {
-    effective_activation_level =
-        subresource_filter::mojom::ActivationLevel::kEnabled;
-    *decision = subresource_filter::ActivationDecision::FORCED_ACTIVATION;
-  }
 
   if (profile_context_->ads_intervention_manager()->ShouldActivate(
           navigation_handle)) {
@@ -161,14 +156,6 @@ ChromeSubresourceFilterClient::GetSafeBrowsingDatabaseManager() {
                                : nullptr;
 }
 
-void ChromeSubresourceFilterClient::ToggleForceActivationInCurrentWebContents(
-    bool force_activation) {
-  if (!activated_via_devtools_ && force_activation)
-    subresource_filter::ContentSubresourceFilterThrottleManager::LogAction(
-        subresource_filter::SubresourceFilterAction::kForcedActivationEnabled);
-  activated_via_devtools_ = force_activation;
-}
-
 void ChromeSubresourceFilterClient::ShowUI(const GURL& url) {
 #if defined(OS_ANDROID)
   InfoBarService* infobar_service =

chrome/browser/subresource_filter/chrome_subresource_filter_client.h

@@ -58,11 +58,6 @@ class ChromeSubresourceFilterClient
   GetSafeBrowsingDatabaseManager() override;
   void OnReloadRequested() override;
 
-  // Should be called by devtools in response to a protocol command to enable ad
-  // blocking in this WebContents. Should only persist while devtools is
-  // attached.
-  void ToggleForceActivationInCurrentWebContents(bool force_activation);
-
  private:
   void ShowUI(const GURL& url);
 
@@ -78,11 +73,6 @@ class ChromeSubresourceFilterClient
   std::unique_ptr<subresource_filter::ProfileInteractionManager>
       profile_interaction_manager_;
 
-  // Corresponds to a devtools command which triggers filtering on all page
-  // loads. We must be careful to ensure this boolean does not persist after the
-  // devtools window is closed, which should be handled by the devtools system.
-  bool activated_via_devtools_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(ChromeSubresourceFilterClient);
 };
 

chrome/browser/subresource_filter/subresource_filter_unittest.cc

@@ -182,58 +182,6 @@ TEST_F(SubresourceFilterTest, RefreshMetadataOnActivation) {
   EXPECT_TRUE(GetSettingsManager()->GetSiteActivationFromMetadata(url));
 }
 
-TEST_F(SubresourceFilterTest, ToggleForceActivation) {
-  base::HistogramTester histogram_tester;
-  const GURL url("https://example.test/");
-
-  // Navigate initially, should be no activation.
-  SimulateNavigateAndCommit(url, main_rfh());
-  EXPECT_TRUE(CreateAndNavigateDisallowedSubframe(main_rfh()));
-  EXPECT_FALSE(GetSettingsManager()->GetSiteActivationFromMetadata(url));
-
-  // Simulate opening devtools and forcing activation.
-  GetClient()->ToggleForceActivationInCurrentWebContents(true);
-  histogram_tester.ExpectBucketCount(
-      kSubresourceFilterActionsHistogram,
-      subresource_filter::SubresourceFilterAction::kForcedActivationEnabled, 1);
-
-  SimulateNavigateAndCommit(url, main_rfh());
-  EXPECT_FALSE(CreateAndNavigateDisallowedSubframe(main_rfh()));
-
-  histogram_tester.ExpectBucketCount(
-      kSubresourceFilterActionsHistogram,
-      subresource_filter::SubresourceFilterAction::kUIShown, 1);
-
-  EXPECT_TRUE(GetSettingsManager()->GetSiteActivationFromMetadata(url));
-  histogram_tester.ExpectBucketCount(
-      "SubresourceFilter.PageLoad.ActivationDecision",
-      subresource_filter::ActivationDecision::FORCED_ACTIVATION, 1);
-
-  // Simulate closing devtools.
-  GetClient()->ToggleForceActivationInCurrentWebContents(false);
-
-  SimulateNavigateAndCommit(url, main_rfh());
-  EXPECT_TRUE(CreateAndNavigateDisallowedSubframe(main_rfh()));
-  histogram_tester.ExpectBucketCount(
-      kSubresourceFilterActionsHistogram,
-      subresource_filter::SubresourceFilterAction::kForcedActivationEnabled, 1);
-}
-
-TEST_F(SubresourceFilterTest, ToggleOffForceActivation_AfterCommit) {
-  base::HistogramTester histogram_tester;
-  GetClient()->ToggleForceActivationInCurrentWebContents(true);
-  const GURL url("https://example.test/");
-  SimulateNavigateAndCommit(url, main_rfh());
-  GetClient()->ToggleForceActivationInCurrentWebContents(false);
-
-  // Resource should be disallowed, since navigation commit had activation.
-  EXPECT_FALSE(CreateAndNavigateDisallowedSubframe(main_rfh()));
-
-  histogram_tester.ExpectBucketCount(
-      kSubresourceFilterActionsHistogram,
-      subresource_filter::SubresourceFilterAction::kUIShown, 1);
-}
-
 enum class AdBlockOnAbusiveSitesTest { kEnabled, kDisabled };
 
 TEST_F(SubresourceFilterTest, NotifySafeBrowsing) {

components/subresource_filter/content/browser/BUILD.gn

@@ -14,6 +14,8 @@ static_library("browser") {
     "content_activation_list_utils.h",
     "content_subresource_filter_throttle_manager.cc",
     "content_subresource_filter_throttle_manager.h",
+    "devtools_interaction_tracker.cc",
+    "devtools_interaction_tracker.h",
     "navigation_console_logger.cc",
     "navigation_console_logger.h",
     "page_load_statistics.cc",

components/subresource_filter/content/browser/devtools_interaction_tracker.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/subresource_filter/content/browser/devtools_interaction_tracker.h"
+
+#include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h"
+
+namespace subresource_filter {
+
+DevtoolsInteractionTracker::DevtoolsInteractionTracker(
+    content::WebContents* web_contents) {}
+
+DevtoolsInteractionTracker::~DevtoolsInteractionTracker() = default;
+
+void DevtoolsInteractionTracker::ToggleForceActivation(bool force_activation) {
+  if (!activated_via_devtools_ && force_activation)
+    ContentSubresourceFilterThrottleManager::LogAction(
+        SubresourceFilterAction::kForcedActivationEnabled);
+  activated_via_devtools_ = force_activation;
+}
+
+WEB_CONTENTS_USER_DATA_KEY_IMPL(DevtoolsInteractionTracker)
+
+}  // namespace subresource_filter

components/subresource_filter/content/browser/devtools_interaction_tracker.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_DEVTOOLS_INTERACTION_TRACKER_H_
+#define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_DEVTOOLS_INTERACTION_TRACKER_H_
+
+#include "content/public/browser/web_contents_user_data.h"
+
+namespace subresource_filter {
+
+// Can be used to track whether forced activation has been set by devtools
+// within a given WebContents.
+// Scoped to the lifetime of a WebContents.
+class DevtoolsInteractionTracker
+    : public content::WebContentsUserData<DevtoolsInteractionTracker> {
+ public:
+  explicit DevtoolsInteractionTracker(content::WebContents* web_contents);
+  ~DevtoolsInteractionTracker() override;
+
+  DevtoolsInteractionTracker(const DevtoolsInteractionTracker&) = delete;
+  DevtoolsInteractionTracker& operator=(const DevtoolsInteractionTracker&) =
+      delete;
+
+  // Should be called by devtools in response to a protocol command to enable ad
+  // blocking in this WebContents. Should only persist while devtools is
+  // attached.
+  void ToggleForceActivation(bool force_activation);
+
+  bool activated_via_devtools() { return activated_via_devtools_; }
+
+ private:
+  friend class content::WebContentsUserData<DevtoolsInteractionTracker>;
+
+  // Corresponds to a devtools command which triggers filtering on all page
+  // loads. We must be careful to ensure this boolean does not persist after the
+  // devtools window is closed, which should be handled by the devtools system.
+  bool activated_via_devtools_ = false;
+
+  WEB_CONTENTS_USER_DATA_KEY_DECL();
+};
+
+}  // namespace subresource_filter
+
+#endif  // COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_DEVTOOLS_INTERACTION_TRACKER_H_

components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc

@@ -15,6 +15,7 @@
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/traced_value.h"
 #include "components/subresource_filter/content/browser/content_activation_list_utils.h"
+#include "components/subresource_filter/content/browser/devtools_interaction_tracker.h"
 #include "components/subresource_filter/content/browser/navigation_console_logger.h"
 #include "components/subresource_filter/content/browser/subresource_filter_client.h"
 #include "components/subresource_filter/content/browser/subresource_filter_observer_manager.h"
@@ -194,8 +195,17 @@ void SubresourceFilterSafeBrowsingActivationThrottle::NotifyResult() {
     activation_level = mojom::ActivationLevel::kDisabled;
   }
 
+  auto* devtools_interaction_tracker =
+      DevtoolsInteractionTracker::FromWebContents(
+          navigation_handle()->GetWebContents());
+
+  if (devtools_interaction_tracker &&
+      devtools_interaction_tracker->activated_via_devtools()) {
+    activation_level = mojom::ActivationLevel::kEnabled;
+    activation_decision = ActivationDecision::FORCED_ACTIVATION;
+  }
+
   // Let the embedder get the last word when it comes to activation level.
-  // TODO(csharrison): Move all ActivationDecision code to the embedder.
   activation_level = client_->OnPageActivationComputed(
       navigation_handle(), activation_level, &activation_decision);
 

components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc

@@ -22,6 +22,7 @@
 #include "components/safe_browsing/core/db/database_manager.h"
 #include "components/safe_browsing/core/db/test_database_manager.h"
 #include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h"
+#include "components/subresource_filter/content/browser/devtools_interaction_tracker.h"
 #include "components/subresource_filter/content/browser/fake_safe_browsing_database_manager.h"
 #include "components/subresource_filter/content/browser/subresource_filter_client.h"
 #include "components/subresource_filter/content/browser/subresource_filter_observer_test_utils.h"
@@ -66,6 +67,7 @@ const char kSafeBrowsingCheckTime[] =
     "SubresourceFilter.SafeBrowsing.TotalCheckTime";
 const char kActivationListHistogram[] =
     "SubresourceFilter.PageLoad.ActivationList";
+const char kSubresourceFilterActionsHistogram[] = "SubresourceFilter.Actions2";
 
 class MockSubresourceFilterClient : public SubresourceFilterClient {
  public:
@@ -87,7 +89,6 @@ class MockSubresourceFilterClient : public SubresourceFilterClient {
   }
 
   MOCK_METHOD0(ShowNotification, void());
-  MOCK_METHOD0(ForceActivationInCurrentWebContents, bool());
   MOCK_METHOD2(OnAdsViolationTriggered,
                void(content::RenderFrameHost*,
                     subresource_filter::mojom::AdsViolation));
@@ -720,6 +721,63 @@ TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest, LogsUkmDryRun) {
   }
 }
 
+TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest,
+       ToggleForceActivation) {
+  auto* web_contents = RenderViewHostTestHarness::web_contents();
+  DevtoolsInteractionTracker::CreateForWebContents(web_contents);
+  auto* devtools_interaction_tracker =
+      DevtoolsInteractionTracker::FromWebContents(web_contents);
+
+  base::HistogramTester histogram_tester;
+  const GURL url("https://example.test/");
+
+  // Navigate initially, should be no activation.
+  SimulateNavigateAndCommit({url}, main_rfh());
+  EXPECT_CALL(*client(), ShowNotification()).Times(0);
+  EXPECT_TRUE(CreateAndNavigateDisallowedSubframe(main_rfh()));
+
+  // Simulate opening devtools and forcing activation.
+  devtools_interaction_tracker->ToggleForceActivation(true);
+  histogram_tester.ExpectBucketCount(
+      kSubresourceFilterActionsHistogram,
+      subresource_filter::SubresourceFilterAction::kForcedActivationEnabled, 1);
+
+  SimulateNavigateAndCommit({url}, main_rfh());
+  EXPECT_CALL(*client(), ShowNotification()).Times(1);
+  EXPECT_FALSE(CreateAndNavigateDisallowedSubframe(main_rfh()));
+
+  histogram_tester.ExpectBucketCount(
+      "SubresourceFilter.PageLoad.ActivationDecision",
+      subresource_filter::ActivationDecision::FORCED_ACTIVATION, 1);
+
+  // Simulate closing devtools.
+  devtools_interaction_tracker->ToggleForceActivation(false);
+
+  SimulateNavigateAndCommit({url}, main_rfh());
+  EXPECT_TRUE(CreateAndNavigateDisallowedSubframe(main_rfh()));
+  histogram_tester.ExpectBucketCount(
+      kSubresourceFilterActionsHistogram,
+      subresource_filter::SubresourceFilterAction::kForcedActivationEnabled, 1);
+}
+
+TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest,
+       ToggleOffForceActivation_AfterCommit) {
+  auto* web_contents = RenderViewHostTestHarness::web_contents();
+  DevtoolsInteractionTracker::CreateForWebContents(web_contents);
+  auto* devtools_interaction_tracker =
+      DevtoolsInteractionTracker::FromWebContents(web_contents);
+
+  base::HistogramTester histogram_tester;
+  devtools_interaction_tracker->ToggleForceActivation(true);
+  const GURL url("https://example.test/");
+  SimulateNavigateAndCommit({url}, main_rfh());
+  devtools_interaction_tracker->ToggleForceActivation(false);
+
+  // Resource should be disallowed, since navigation commit had activation.
+  EXPECT_CALL(*client(), ShowNotification()).Times(1);
+  EXPECT_FALSE(CreateAndNavigateDisallowedSubframe(main_rfh()));
+}
+
 TEST_P(SubresourceFilterSafeBrowsingActivationThrottleScopeTest,
        ActivateForScopeType) {
   const ActivationScopeTestData& test_data = GetParam();