Supervised Users: Create ResourceThrottle only if filtering is enabled

Before this CL, we were creating a SupervisedUserResourceThrottle for
each top-level navigation. If the current user isn't supervised, this is
pointless. One side-effect was that we were recording
ManagerUsers.Filter* histograms for all navigations, including for
non-supervised users.

This CL adds an "enabled" field on SupervisedUserURLFilter, which is set
to true iff the user is supervised. This allows us to not create the
resource throttle for regular users.

BUG=671472

Review-Url: https://codereview.chromium.org/2564043002
Cr-Commit-Position: refs/heads/master@{#437868}

chrome/browser/search/search_unittest.cc

@@ -443,6 +443,7 @@ TEST_F(SearchTest, UseLocalNTPIfNTPURLIsBlockedForSupervisedUser) {
   std::map<std::string, bool> hosts;
   hosts["foo.com"] = false;
   url_filter->SetManualHosts(&hosts);
+  url_filter->SetEnabled(true);
 
   EXPECT_EQ(GURL(chrome::kChromeSearchLocalNtpUrl),
             GetNewTabPageURL(profile()));

chrome/browser/supervised_user/supervised_user_resource_throttle.cc

@@ -117,7 +117,7 @@ SupervisedUserResourceThrottle::MaybeCreate(
     const SupervisedUserURLFilter* url_filter) {
   // Only treat main frame requests (ignoring subframes and subresources).
   bool is_main_frame = resource_type == content::RESOURCE_TYPE_MAIN_FRAME;
-  if (!is_main_frame)
+  if (!is_main_frame || !url_filter->enabled())
     return nullptr;
 
   // Can't use base::MakeUnique because the constructor is private.

chrome/browser/supervised_user/supervised_user_resource_throttle_browsertest.cc

@@ -136,3 +136,28 @@ IN_PROC_BROWSER_TEST_F(SupervisedUserResourceThrottleTest, DontBlockSubFrame) {
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(tab, "loaded2()", &loaded2));
   EXPECT_TRUE(loaded2);
 }
+
+class SupervisedUserResourceThrottleNotSupervisedTest
+    : public SupervisedUserResourceThrottleTest {
+ protected:
+  SupervisedUserResourceThrottleNotSupervisedTest() {}
+  ~SupervisedUserResourceThrottleNotSupervisedTest() override {}
+
+ private:
+  // Overridden to do nothing, so that the supervised user ID will be empty.
+  void SetUpCommandLine(base::CommandLine* command_line) override {}
+};
+
+IN_PROC_BROWSER_TEST_F(SupervisedUserResourceThrottleNotSupervisedTest,
+                       DontBlock) {
+  BlockHost(kExampleHost);
+
+  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+
+  GURL blocked_url = embedded_test_server()->GetURL(
+      kExampleHost, "/supervised_user/simple.html");
+  ui_test_utils::NavigateToURL(browser(), blocked_url);
+  // Even though the URL is marked as blocked, the load should go through, since
+  // the user isn't supervised.
+  EXPECT_FALSE(tab->ShowingInterstitialPage());
+}

chrome/browser/supervised_user/supervised_user_service.cc

@@ -441,6 +441,13 @@ SupervisedUserService::URLFilterContext::io_url_filter() const {
   return io_url_filter_.get();
 }
 
+void SupervisedUserService::URLFilterContext::SetEnabled(bool enabled) {
+  ui_url_filter_->SetEnabled(enabled);
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&SupervisedUserURLFilter::SetEnabled,
+                                     io_url_filter_, enabled));
+}
+
 void SupervisedUserService::URLFilterContext::SetDefaultFilteringBehavior(
     SupervisedUserURLFilter::FilteringBehavior behavior) {
   ui_url_filter_->SetDefaultFilteringBehavior(behavior);
@@ -672,6 +679,8 @@ void SupervisedUserService::SetActive(bool active) {
     BrowserList::RemoveObserver(this);
 #endif
   }
+
+  url_filter_context_.SetEnabled(active_);
 }
 
 #if !defined(OS_ANDROID)

chrome/browser/supervised_user/supervised_user_service.h

@@ -250,6 +250,7 @@ class SupervisedUserService : public KeyedService,
     SupervisedUserURLFilter* ui_url_filter() const;
     SupervisedUserURLFilter* io_url_filter() const;
 
+    void SetEnabled(bool enabled);
     void SetDefaultFilteringBehavior(
         SupervisedUserURLFilter::FilteringBehavior behavior);
     void LoadWhitelists(

chrome/browser/supervised_user/supervised_user_url_filter.cc

@@ -231,7 +231,8 @@ GURL BuildURL(bool is_https, const std::string& host_and_path) {
 }  // namespace
 
 SupervisedUserURLFilter::SupervisedUserURLFilter()
-    : default_behavior_(ALLOW),
+    : enabled_(false),
+      default_behavior_(ALLOW),
       contents_(new Contents()),
       blacklist_(nullptr),
       amp_cache_path_regex_(kAmpCachePathPattern),
@@ -323,6 +324,16 @@ bool SupervisedUserURLFilter::HostMatchesPattern(
   return trimmed_host == trimmed_pattern;
 }
 
+void SupervisedUserURLFilter::SetEnabled(bool enabled) {
+  if (enabled_ == enabled)
+    return;
+
+  enabled_ = enabled;
+
+  for (Observer& observer : observers_)
+    observer.OnSiteListUpdated();
+}
+
 SupervisedUserURLFilter::FilteringBehavior
 SupervisedUserURLFilter::GetFilteringBehaviorForURL(const GURL& url) const {
   supervised_user_error_page::FilteringBehaviorReason reason;
@@ -343,6 +354,11 @@ SupervisedUserURLFilter::GetFilteringBehaviorForURL(
     supervised_user_error_page::FilteringBehaviorReason* reason) const {
   DCHECK(CalledOnValidThread());
 
+  if (!enabled_) {
+    *reason = supervised_user_error_page::DEFAULT;
+    return ALLOW;
+  }
+
   GURL effective_url = GetEmbeddedURL(url);
   if (!effective_url.is_valid())
     effective_url = url;
@@ -645,8 +661,10 @@ GURL SupervisedUserURLFilter::GetEmbeddedURL(const GURL& url) const {
 void SupervisedUserURLFilter::SetContents(std::unique_ptr<Contents> contents) {
   DCHECK(CalledOnValidThread());
   contents_ = std::move(contents);
-  for (Observer& observer : observers_)
-    observer.OnSiteListUpdated();
+  if (enabled_) {
+    for (Observer& observer : observers_)
+      observer.OnSiteListUpdated();
+  }
 }
 
 void SupervisedUserURLFilter::CheckCallback(

chrome/browser/supervised_user/supervised_user_url_filter.h

@@ -61,7 +61,11 @@ class SupervisedUserURLFilter
 
   class Observer {
    public:
+    // Called whenever the filter changes.
+    // TODO(treib,bauerb): Rename to OnURLFilterUpdated.
     virtual void OnSiteListUpdated() = 0;
+    // Called whenever a check started via
+    // GetFilteringBehaviorForURLWithAsyncChecks completes.
     virtual void OnURLChecked(
         const GURL& url,
         FilteringBehavior behavior,
@@ -106,6 +110,13 @@ class SupervisedUserURLFilter
   static bool HostMatchesPattern(const std::string& canonical_host,
                                  const std::string& pattern);
 
+  // Returns whether the filter is enabled. If this is false, all URL checks
+  // will return ALLOW.
+  bool enabled() const { return enabled_; }
+
+  // Enables or disables the filter. Notifies observers if the state changed.
+  void SetEnabled(bool enabled);
+
   // Returns the filtering behavior for a given URL, based on the default
   // behavior and whether it is on a site list.
   FilteringBehavior GetFilteringBehaviorForURL(const GURL& url) const;
@@ -198,6 +209,10 @@ class SupervisedUserURLFilter
   // This is mutable to allow notification in const member functions.
   mutable base::ObserverList<Observer> observers_;
 
+  // Whether this filter is enabled. True for supervised user profiles, false
+  // otherwise.
+  bool enabled_;
+
   FilteringBehavior default_behavior_;
   std::unique_ptr<Contents> contents_;
 

chrome/browser/supervised_user/supervised_user_url_filter_unittest.cc

@@ -22,6 +22,7 @@ class SupervisedUserURLFilterTest : public ::testing::Test,
  public:
   SupervisedUserURLFilterTest() : filter_(new SupervisedUserURLFilter) {
     filter_->SetDefaultFilteringBehavior(SupervisedUserURLFilter::BLOCK);
+    filter_->SetEnabled(true);
     filter_->AddObserver(this);
   }
 
@@ -68,6 +69,11 @@ TEST_F(SupervisedUserURLFilterTest, Basic) {
   EXPECT_TRUE(IsURLWhitelisted("chrome://extensions/"));
   EXPECT_TRUE(IsURLWhitelisted("chrome-extension://foo/main.html"));
   EXPECT_TRUE(IsURLWhitelisted("file:///home/chronos/user/Downloads/img.jpg"));
+
+  // If the filter is disabled, everything should be allowed.
+  filter_->SetEnabled(false);
+  EXPECT_TRUE(IsURLWhitelisted("http://google.com"));
+  EXPECT_TRUE(IsURLWhitelisted("http://notgoogle.com/"));
 }
 
 TEST_F(SupervisedUserURLFilterTest, EffectiveURL) {