Revert of Supervised Users: Create ResourceThrottle only if filtering is...

Revert of Supervised Users: Create ResourceThrottle only if filtering is enabled (patchset #3 id:40001 of https://codereview.chromium.org/2564043002/ )

Reason for revert:
This is believed to have increased the rate of failure for SupervisedUserContentProviderTest#testWithSupervisedUser on linux_android_rel_ng.

See http://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=chrome_public_test_apk%20%28with%20patch%29&builder=tryserver.chromium.android%3Alinux_android_rel_ng for dramatic increase in failure rate after this CL landed.

Example failure: https://chromium-swarm.appspot.com/task?id=330d0f66aec28810

Original issue's description:
> 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
>
> Committed: https://crrev.com/54cf4ab900ab78c713bb4d53489f0571a433df24
> Cr-Commit-Position: refs/heads/master@{#437868}

TBR=bauerb@chromium.org,treib@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=671472

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

chrome/browser/search/search_unittest.cc

@@ -443,7 +443,6 @@ 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 || !url_filter->enabled())
+  if (!is_main_frame)
     return nullptr;
 
   // Can't use base::MakeUnique because the constructor is private.

chrome/browser/supervised_user/supervised_user_resource_throttle_browsertest.cc

@@ -136,28 +136,3 @@ 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,13 +441,6 @@ 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);
@@ -679,8 +672,6 @@ 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,7 +250,6 @@ 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,8 +231,7 @@ GURL BuildURL(bool is_https, const std::string& host_and_path) {
 }  // namespace
 
 SupervisedUserURLFilter::SupervisedUserURLFilter()
-    : enabled_(false),
-      default_behavior_(ALLOW),
+    : default_behavior_(ALLOW),
       contents_(new Contents()),
       blacklist_(nullptr),
       amp_cache_path_regex_(kAmpCachePathPattern),
@@ -324,16 +323,6 @@ 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;
@@ -354,11 +343,6 @@ 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;
@@ -661,10 +645,8 @@ GURL SupervisedUserURLFilter::GetEmbeddedURL(const GURL& url) const {
 void SupervisedUserURLFilter::SetContents(std::unique_ptr<Contents> contents) {
   DCHECK(CalledOnValidThread());
   contents_ = std::move(contents);
-  if (enabled_) {
-    for (Observer& observer : observers_)
-      observer.OnSiteListUpdated();
-  }
+  for (Observer& observer : observers_)
+    observer.OnSiteListUpdated();
 }
 
 void SupervisedUserURLFilter::CheckCallback(

chrome/browser/supervised_user/supervised_user_url_filter.h

@@ -61,11 +61,7 @@ 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,
@@ -110,13 +106,6 @@ 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;
@@ -209,10 +198,6 @@ 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,7 +22,6 @@ class SupervisedUserURLFilterTest : public ::testing::Test,
  public:
   SupervisedUserURLFilterTest() : filter_(new SupervisedUserURLFilter) {
     filter_->SetDefaultFilteringBehavior(SupervisedUserURLFilter::BLOCK);
-    filter_->SetEnabled(true);
     filter_->AddObserver(this);
   }
 
@@ -69,11 +68,6 @@ 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) {