[subresource_filter] Implement the "Smart" UI on Android

chrome/browser/subresource_filter/subresource_filter_browsertest.cc

 // Copyright 2016 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 <map>
 #include <memory>
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/path_service.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/histogram_tester.h"
+#include "base/test/simple_test_clock.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/v4_test_utils.h"
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
+#include "chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h"
+#include "chrome/browser/subresource_filter/subresource_filter_content_settings_manager_factory.h"
 #include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
@@ skipping 228 matching lines @@
 
   void SetUpOnMainThread() override {
     SetUpActivationFeature();
     base::FilePath test_data_dir;
     PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
     embedded_test_server()->ServeFilesFromDirectory(test_data_dir);
     host_resolver()->AddSimulatedFailure("host-with-dns-lookup-failure");
     host_resolver()->AddRule("*", "127.0.0.1");
     content::SetupCrossSiteRedirector(embedded_test_server());
     ASSERT_TRUE(embedded_test_server()->Start());
+
+    settings_manager_ =
+        SubresourceFilterContentSettingsManagerFactory::EnsureForProfile(
+            browser()->profile());
+#if defined(OS_ANDROID)
+    EXPECT_TRUE(settings_manager->should_use_smart_ui());
+#endif
   }
 
   virtual void SetUpActivationFeature() {
     ToggleFeatures(base::MakeUnique<ScopedSubresourceFilterFeatureToggle>(
         base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
         kActivationScopeActivationList, kActivationListPhishingInterstitial,
         measure_performance_ ? "1" : "0", "" /* suppress_notifications */,
         whitelist_site_on_reload_ ? "true" : "false"));
   }
 
@@ skipping 18 matching lines @@
 
   void ConfigureAsSubresourceFilterOnlyURL(const GURL& url) {
     MarkUrlAsMatchingListWithId(url, safe_browsing::GetUrlSubresourceFilterId(),
                                 safe_browsing::ThreatPatternType::NONE);
   }
 
   content::WebContents* web_contents() {
     return browser()->tab_strip_model()->GetActiveWebContents();
   }
 
+  SubresourceFilterContentSettingsManager* settings_manager() {
+    return settings_manager_;
+  }
+
   content::RenderFrameHost* FindFrameByName(const std::string& name) {
     for (content::RenderFrameHost* frame : web_contents()->GetAllFrames()) {
       if (frame->GetFrameName() == name)
         return frame;
     }
     return nullptr;
   }
 
   bool WasParsedScriptElementLoaded(content::RenderFrameHost* rfh) {
     DCHECK(rfh);
@@ skipping 69 matching lines @@
   TestRulesetPublisher test_ruleset_publisher_;
   const bool measure_performance_;
   const bool whitelist_site_on_reload_;
 
   std::unique_ptr<safe_browsing::TestSafeBrowsingServiceFactory> sb_factory_;
   // Owned by the V4Database.
   safe_browsing::TestV4DatabaseFactory* v4_db_factory_;
   // Owned by the V4GetHashProtocolManager.
   safe_browsing::TestV4GetHashProtocolManagerFactory* v4_get_hash_factory_;
 
+  // Owned by the profile.
+  SubresourceFilterContentSettingsManager* settings_manager_;
+
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterBrowserTestImpl);
 };
 
 class SubresourceFilterBrowserTest : public SubresourceFilterBrowserTestImpl {
  public:
   SubresourceFilterBrowserTest()
       : SubresourceFilterBrowserTestImpl(false, false) {}
 };
 
 // TODO(pkalinnikov): It should be possible to have only one fixture, i.e.,
@@ skipping 421 matching lines @@
   // page load right after start-up.
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 }
 
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
                        PromptShownAgainOnNextNavigation) {
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
   GURL url(GetTestUrl(kTestFrameSetPath));
+  GURL a_url(embedded_test_server()->GetURL(
+      "a.com", "/subresource_filter/frame_with_included_script.html"));
   ConfigureAsPhishingURL(url);
   base::HistogramTester tester;
   ui_test_utils::NavigateToURL(browser(), url);
   tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown,
                            1);
   // Check that the bubble is not shown again for this navigation.
   EXPECT_FALSE(IsDynamicScriptElementLoaded(FindFrameByName("five")));
   tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown,
                            1);
-  // Check that bubble is shown for new navigation.
-  ui_test_utils::NavigateToURL(browser(), url);
+  // Check that bubble is shown for new navigation. Must be cross site to avoid
+  // triggering smart UI on Android.
+  ConfigureAsPhishingURL(a_url);
+  ui_test_utils::NavigateToURL(browser(), a_url);
   tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown,
                            2);
 }
 
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
                        CrossSiteSubFrameActivationWithoutWhitelist) {
   GURL a_url(embedded_test_server()->GetURL(
       "a.com", "/subresource_filter/frame_cross_site_set.html"));
   ConfigureAsPhishingURL(a_url);
   ASSERT_NO_FATAL_FAILURE(
@@ skipping 110 matching lines @@
   EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   // A cross site blacklisted navigation should stay activated, however.
   GURL a_url(embedded_test_server()->GetURL(
       "a.com", "/subresource_filter/frame_with_included_script.html"));
   ConfigureAsPhishingURL(a_url);
   ui_test_utils::NavigateToURL(browser(), a_url);
   EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 }
 
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       SiteDeactivated_ContentSettingCleared) {
+  HostContentSettingsMap* settings_map =
+      HostContentSettingsMapFactory::GetForProfile(browser()->profile());
+  ContentSettingsForOneType host_settings;
+  settings_map->GetSettingsForOneType(
+      ContentSettingsType::CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER,
+      std::string(), &host_settings);
+
+  // Originally should just have a single default rule: * -> Allowed.
+  EXPECT_EQ(1u, host_settings.size());
+
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
+  GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
+  ConfigureAsPhishingURL(url);
+
+  EXPECT_EQ(CONTENT_SETTING_ALLOW, settings_manager()->GetContentSetting(url));
+  ui_test_utils::NavigateToURL(browser(), url);
+  EXPECT_EQ(CONTENT_SETTING_ALLOW, settings_manager()->GetContentSetting(url));
+
+  // Should have two rules now, one explicitly setting the origin of |url| to

[msramek, 2017/04/13 16:48:32]

This looks really wrong to me. I have one setting, I navigate somewhere, now I
have two settings.

There are two issues:

1. Privacy. There is no precedent to content setting exceptions being added
automatically. It's always manually through the settings UI or through accepting
a permission prompt. This is an important guarantee to keep, because content
settings reveal your browsing behavior yet are not deleted together with
browsing history.

2. As was explained to me, you're explicitly setting the exception to the same
value as default in order to force it to appear in the page info dialog. That's
a fragile dependency on an implementation detail of that dialog.

Basically, the permissions team decided back in the day that showing all
settings in page info is too much, and that a reasonable design would be to show
only non-default settings. If their design doesn't fit your usecase, please tell
them! Instead of working around it.

And in short-term, please try to special-case your content setting in page info.
I don't own that code, but I vaguely remember that it does a bunch of
special-casing already, so I'm sure that's allowed.

+  // Allow.
+  settings_map->GetSettingsForOneType(
+      ContentSettingsType::CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER,
+      std::string(), &host_settings);
+  EXPECT_EQ(2u, host_settings.size());
+
+  // Configure a different URL as the phishing URL, which resets |url|.
+  ConfigureAsPhishingURL(GURL("https://example.other/"));
+
+  // Re-navigating to |url| should reset the content setting.
+  ui_test_utils::NavigateToURL(browser(), url);
+  settings_map->GetSettingsForOneType(
+      ContentSettingsType::CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER,
+      std::string(), &host_settings);
+  EXPECT_EQ(1u, host_settings.size());
+  EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
+}
+
+// Test the "smart" UI, aka the logic to hide the UI on subsequent same-domain
+// navigations, until a certain time threshold has been reached. This is an
+// android-only feature.
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       DoNotShowUIUntilThresholdReached) {
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
+  GURL a_url(embedded_test_server()->GetURL(
+      "a.com", "/subresource_filter/frame_with_included_script.html"));
+  GURL b_url(embedded_test_server()->GetURL(
+      "b.com", "/subresource_filter/frame_with_included_script.html"));
+  // Test utils only support one blacklisted site at a time.
+  // TODO(csharrison): Add support for more than one URL.
+  ConfigureAsPhishingURL(a_url);
+
+  // Cast is safe because this is the only type of client in non-unittest code.
+  ChromeSubresourceFilterClient* client =
+      static_cast<ChromeSubresourceFilterClient*>(
+          ContentSubresourceFilterDriverFactory::FromWebContents(web_contents())
+              ->client());
+  auto test_clock = base::MakeUnique<base::SimpleTestClock>();
+  base::SimpleTestClock* raw_clock = test_clock.get();
+  settings_manager()->set_clock_for_testing(std::move(test_clock));
+
+  base::HistogramTester histogram_tester;
+
+  // First load should trigger the UI.
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
+  EXPECT_TRUE(client->shown_for_navigation());
+
+  histogram_tester.ExpectBucketCount(kSubresourceFilterActionsHistogram,
+                                     kActionUISuppressed, 0);
+
+  // Second load should not trigger the UI, but should still filter content.
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
+
+  bool use_smart_ui = settings_manager()->should_use_smart_ui();
+  EXPECT_EQ(client->shown_for_navigation(), !use_smart_ui);
+
+  histogram_tester.ExpectBucketCount(kSubresourceFilterActionsHistogram,
+                                     kActionUISuppressed, use_smart_ui ? 1 : 0);
+
+  ConfigureAsPhishingURL(b_url);
+
+  // Load to another domain should trigger the UI.
+  ui_test_utils::NavigateToURL(browser(), b_url);
+  EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
+  EXPECT_TRUE(client->shown_for_navigation());
+
+  ConfigureAsPhishingURL(a_url);
+
+  // Fast forward the clock, and a_url should trigger the UI again.
+  raw_clock->Advance(
+      SubresourceFilterContentSettingsManager::kDelayBeforeShowingInfobarAgain);
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
+  EXPECT_TRUE(client->shown_for_navigation());
+
+  histogram_tester.ExpectBucketCount(kSubresourceFilterActionsHistogram,
+                                     kActionUISuppressed, use_smart_ui ? 1 : 0);
+}
+
 IN_PROC_BROWSER_TEST_P(SubresourceFilterWebSocketBrowserTest, BlockWebSocket) {
   GURL url(GetTestUrl(
       base::StringPrintf("subresource_filter/page_with_websocket.html?%s",
                          GetParam() == IN_WORKER ? "inWorker" : "")));
   GURL websocket_url(GetWebSocketUrl("echo-with-no-extension"));
   ConfigureAsPhishingURL(url);
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("echo-with-no-extension"));
   ui_test_utils::NavigateToURL(browser(), url);
   CreateWebSocketAndExpectResult(websocket_url,
@@ skipping 349 matching lines @@
                                    std::string(kSubresourceFilterOnlySuffix)),
               ::testing::IsEmpty());
 
   EXPECT_THAT(tester.GetAllSamples(std::string(kNavigationChainSize) +
                                    std::string(kSubresourceFilterOnlySuffix)),
               ::testing::IsEmpty());
 }
 #endif
 
 }  // namespace subresource_filter