[subresource_filter] Don't populate metadata if we're suppressing UI

For abusive enforcement we don't want to show up in settings or Page Info, so make sure we don't populate the metadata in cases where we'd ship with suppress_notifications. Bug: 756089 Change-Id: I31967339737005e432ded66f69e583b7b60635ab Reviewed-on: https://chromium-review.googlesource.com/646792 Reviewed-by: Shivani Sharma <[email protected]> Commit-Queue: Charlie Harrison <[email protected]> Cr-Commit-Position: refs/heads/master@{#499098}
nwjs · Sep 1, 2017 · fa6a5d8 · fa6a5d8
1 parent 94048e8
commit fa6a5d8
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 7 deletions.
diff --git a/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc b/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
@@ -132,15 +132,17 @@ void ChromeSubresourceFilterClient::OnNewNavigationStarted() {
 
 bool ChromeSubresourceFilterClient::OnPageActivationComputed(
     content::NavigationHandle* navigation_handle,
-    bool activated) {
+    bool activated,
+    bool suppressing_notifications) {
   const GURL& url(navigation_handle->GetURL());
   DCHECK(navigation_handle->IsInMainFrame());
 
   if (url.SchemeIsHTTPOrHTTPS()) {
     // With respect to persistent metadata, do not consider the site activated
-    // if it is forced via devtools.
+    // if it is forced via devtools, or if we are suppressing notifications.
     settings_manager_->ResetSiteMetadataBasedOnActivation(
-        url, activated && !activated_via_devtools_);
+        url,
+        activated && !activated_via_devtools_ && !suppressing_notifications);
   }
 
   // Return whether the activation should be whitelisted.

diff --git a/chrome/browser/subresource_filter/chrome_subresource_filter_client.h b/chrome/browser/subresource_filter/chrome_subresource_filter_client.h
@@ -111,7 +111,8 @@ class ChromeSubresourceFilterClient
   void ShowNotification() override;
   void OnNewNavigationStarted() override;
   bool OnPageActivationComputed(content::NavigationHandle* navigation_handle,
-                                bool activated) override;
+                                bool activated,
+                                bool suppressing_notifications) override;
   void WhitelistInCurrentWebContents(const GURL& url) override;
   subresource_filter::VerifiedRulesetDealer::Handle* GetRulesetDealer()
       override;

diff --git a/chrome/browser/subresource_filter/subresource_filter_unittest.cc b/chrome/browser/subresource_filter/subresource_filter_unittest.cc
@@ -268,3 +268,22 @@ TEST_F(SubresourceFilterTest, UIShown_LogsRappor) {
   // The host is the same as the etld+1 in this case.
   EXPECT_EQ(url.host(), sample_string);
 }
+
+TEST_F(SubresourceFilterTest, AbusiveEnforcement_NoMetadata) {
+  subresource_filter::Configuration config(
+      subresource_filter::ActivationLevel::ENABLED,
+      subresource_filter::ActivationScope::ACTIVATION_LIST,
+      subresource_filter::ActivationList::SUBRESOURCE_FILTER);
+  config.activation_options.should_disable_ruleset_rules = true;
+  config.activation_options.should_strengthen_popup_blocker = true;
+  config.activation_options.should_suppress_notifications = true;
+
+  scoped_configuration().ResetConfiguration(std::move(config));
+
+  GURL url("https://a.test");
+  ConfigureAsSubresourceFilterOnlyURL(url);
+  SimulateNavigateAndCommit(url, main_rfh());
+  EXPECT_TRUE(CreateAndNavigateDisallowedSubframe(main_rfh()));
+  EXPECT_EQ(nullptr, GetSettingsManager()->GetSiteMetadata(url));
+  EXPECT_FALSE(GetClient()->did_show_ui_for_navigation());
+}
diff --git a/components/subresource_filter/content/browser/subresource_filter_client.h b/components/subresource_filter/content/browser/subresource_filter_client.h
@@ -35,7 +35,8 @@ class SubresourceFilterClient {
   // Precondition: The navigation must be a main frame navigation.
   virtual bool OnPageActivationComputed(
       content::NavigationHandle* navigation_handle,
-      bool activated) = 0;
+      bool activated,
+      bool suppressing_notifications) = 0;
 
   // Adds |url| to a per-WebContents whitelist.
   virtual void WhitelistInCurrentWebContents(const GURL& url) = 0;

diff --git a/...ubresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc b/...ubresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc
@@ -210,7 +210,8 @@ void SubresourceFilterSafeBrowsingActivationThrottle::NotifyResult() {
   bool whitelisted = client_->OnPageActivationComputed(
       navigation_handle(),
       matched_configuration.activation_options.activation_level ==
-          ActivationLevel::ENABLED);
+          ActivationLevel::ENABLED,
+      matched_configuration.activation_options.should_suppress_notifications);
 
   // Only reset the activation decision reason if we would have activated.
   if (whitelisted && activation_decision == ActivationDecision::ACTIVATED) {

diff --git a/...e_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc b/...e_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc
@@ -86,7 +86,8 @@ class MockSubresourceFilterClient : public SubresourceFilterClient {
   }
 
   bool OnPageActivationComputed(content::NavigationHandle* handle,
-                                bool activated) override {
+                                bool activated,
+                                bool suppress_notifications) override {
     DCHECK(handle->IsInMainFrame());
     return whitelisted_hosts_.count(handle->GetURL().host());
   }