Add a SupervisedUserServiceObserver.

For now, this is used in the SupervisedUserInterstitial instead of listening to pref changes manually.
More clients will follow in later CLs.

BUG=

Review URL: https://codereview.chromium.org/597573002

Cr-Commit-Position: refs/heads/master@{#296412}

chrome/browser/supervised_user/supervised_user_interstitial.cc

@@ -111,11 +111,14 @@ SupervisedUserInterstitial::SupervisedUserInterstitial(
     const GURL& url,
     const base::Callback<void(bool)>& callback)
     : web_contents_(web_contents),
+      profile_(Profile::FromBrowserContext(web_contents->GetBrowserContext())),
       interstitial_page_(NULL),
       url_(url),
       callback_(callback) {}
 
-SupervisedUserInterstitial::~SupervisedUserInterstitial() {}
+SupervisedUserInterstitial::~SupervisedUserInterstitial() {
+  DCHECK(!web_contents_);
+}
 
 bool SupervisedUserInterstitial::Init() {
   if (ShouldProceed()) {
@@ -152,24 +155,9 @@ bool SupervisedUserInterstitial::Init() {
     }
   }
 
-  // TODO(bauerb): Extract an observer callback on SupervisedUserService for
-  // this.
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents_->GetBrowserContext());
-  PrefService* prefs = profile->GetPrefs();
-  pref_change_registrar_.Init(prefs);
-  pref_change_registrar_.Add(
-      prefs::kDefaultSupervisedUserFilteringBehavior,
-      base::Bind(&SupervisedUserInterstitial::OnFilteringPrefsChanged,
-                 base::Unretained(this)));
-  pref_change_registrar_.Add(
-      prefs::kSupervisedUserManualHosts,
-      base::Bind(&SupervisedUserInterstitial::OnFilteringPrefsChanged,
-                 base::Unretained(this)));
-  pref_change_registrar_.Add(
-      prefs::kSupervisedUserManualURLs,
-      base::Bind(&SupervisedUserInterstitial::OnFilteringPrefsChanged,
-                 base::Unretained(this)));
+  SupervisedUserService* supervised_user_service =
+      SupervisedUserServiceFactory::GetForProfile(profile_);
+  supervised_user_service->AddObserver(this);
 
   interstitial_page_ =
       content::InterstitialPage::Create(web_contents_, true, url_, this);
@@ -183,22 +171,20 @@ std::string SupervisedUserInterstitial::GetHTMLContents() {
   strings.SetString("blockPageTitle",
                     l10n_util::GetStringUTF16(IDS_BLOCK_INTERSTITIAL_TITLE));
 
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents_->GetBrowserContext());
   SupervisedUserService* supervised_user_service =
-      SupervisedUserServiceFactory::GetForProfile(profile);
+      SupervisedUserServiceFactory::GetForProfile(profile_);
 
   bool allow_access_requests = supervised_user_service->AccessRequestsEnabled();
   strings.SetBoolean("allowAccessRequests", allow_access_requests);
 
-  std::string profile_image_url = profile->GetPrefs()->GetString(
+  std::string profile_image_url = profile_->GetPrefs()->GetString(
       prefs::kSupervisedUserCustodianProfileImageURL);
   strings.SetString("avatarURL1x", BuildAvatarImageUrl(profile_image_url,
                                                        kAvatarSize1x));
   strings.SetString("avatarURL2x", BuildAvatarImageUrl(profile_image_url,
                                                        kAvatarSize2x));
 
-  std::string profile_image_url2 = profile->GetPrefs()->GetString(
+  std::string profile_image_url2 = profile_->GetPrefs()->GetString(
       prefs::kSupervisedUserSecondCustodianProfileImageURL);
   strings.SetString("secondAvatarURL1x", BuildAvatarImageUrl(profile_image_url2,
                                                              kAvatarSize1x));
@@ -263,10 +249,8 @@ void SupervisedUserInterstitial::CommandReceived(const std::string& command) {
                               ACCESS_REQUEST,
                               HISTOGRAM_BOUNDING_VALUE);
 
-    Profile* profile =
-        Profile::FromBrowserContext(web_contents_->GetBrowserContext());
     SupervisedUserService* supervised_user_service =
-        SupervisedUserServiceFactory::GetForProfile(profile);
+        SupervisedUserServiceFactory::GetForProfile(profile_);
     supervised_user_service->AddAccessRequest(url_);
     DVLOG(1) << "Sent access request for " << url_.spec();
 
@@ -287,24 +271,30 @@ void SupervisedUserInterstitial::OnDontProceed() {
   DispatchContinueRequest(false);
 }
 
+void SupervisedUserInterstitial::OnURLFilterChanged() {
+  if (ShouldProceed())
+    interstitial_page_->Proceed();
+}
+
 bool SupervisedUserInterstitial::ShouldProceed() {
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents_->GetBrowserContext());
   SupervisedUserService* supervised_user_service =
-      SupervisedUserServiceFactory::GetForProfile(profile);
+      SupervisedUserServiceFactory::GetForProfile(profile_);
   SupervisedUserURLFilter* url_filter =
       supervised_user_service->GetURLFilterForUIThread();
   return url_filter->GetFilteringBehaviorForURL(url_) !=
          SupervisedUserURLFilter::BLOCK;
 }
 
-void SupervisedUserInterstitial::OnFilteringPrefsChanged() {
-  if (ShouldProceed())
-    interstitial_page_->Proceed();
-}
-
 void SupervisedUserInterstitial::DispatchContinueRequest(
     bool continue_request) {
+  SupervisedUserService* supervised_user_service =
+      SupervisedUserServiceFactory::GetForProfile(profile_);
+  supervised_user_service->RemoveObserver(this);
+
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE, base::Bind(callback_, continue_request));
+
+  // After this, the WebContents may be destroyed. Make sure we don't try to use
+  // it again.
+  web_contents_ = NULL;
 }

chrome/browser/supervised_user/supervised_user_interstitial.h

@@ -8,7 +8,7 @@
 #include <string>
 
 #include "base/callback.h"
-#include "base/prefs/pref_change_registrar.h"
+#include "chrome/browser/supervised_user/supervised_user_service_observer.h"
 #include "content/public/browser/interstitial_page_delegate.h"
 #include "url/gurl.h"
 
@@ -17,10 +17,13 @@ class InterstitialPage;
 class WebContents;
 }
 
+class Profile;
+
 // Delegate for an interstitial page when a page is blocked for a supervised
 // user because it is on a blacklist (in "allow everything" mode) or not on any
 // whitelist (in "allow only specified sites" mode).
-class SupervisedUserInterstitial : public content::InterstitialPageDelegate {
+class SupervisedUserInterstitial : public content::InterstitialPageDelegate,
+                                   public SupervisedUserServiceObserver {
  public:
   static void Show(content::WebContents* web_contents,
                    const GURL& url,
@@ -40,20 +43,22 @@ class SupervisedUserInterstitial : public content::InterstitialPageDelegate {
   virtual void OnProceed() OVERRIDE;
   virtual void OnDontProceed() OVERRIDE;
 
+  // SupervisedUserServiceObserver implementation.
+  virtual void OnURLFilterChanged() OVERRIDE;
+
   // Returns whether the blocked URL is now allowed. Called initially before the
   // interstitial is shown (to catch race conditions), or when the URL filtering
   // prefs change.
   bool ShouldProceed();
 
-  void OnFilteringPrefsChanged();
   void DispatchContinueRequest(bool continue_request);
 
   // Owns the interstitial, which owns us.
   content::WebContents* web_contents_;
 
-  content::InterstitialPage* interstitial_page_;  // Owns us.
+  Profile* profile_;
 
-  PrefChangeRegistrar pref_change_registrar_;
+  content::InterstitialPage* interstitial_page_;  // Owns us.
 
   GURL url_;
 

chrome/browser/supervised_user/supervised_user_service.cc

@@ -24,6 +24,7 @@
 #include "chrome/browser/supervised_user/supervised_user_pref_mapping_service.h"
 #include "chrome/browser/supervised_user/supervised_user_pref_mapping_service_factory.h"
 #include "chrome/browser/supervised_user/supervised_user_registration_utility.h"
+#include "chrome/browser/supervised_user/supervised_user_service_observer.h"
 #include "chrome/browser/supervised_user/supervised_user_settings_service.h"
 #include "chrome/browser/supervised_user/supervised_user_settings_service_factory.h"
 #include "chrome/browser/supervised_user/supervised_user_shared_settings_service_factory.h"
@@ -313,6 +314,16 @@ void SupervisedUserService::DidBlockNavigation(
   }
 }
 
+void SupervisedUserService::AddObserver(
+    SupervisedUserServiceObserver* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void SupervisedUserService::RemoveObserver(
+    SupervisedUserServiceObserver* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
 #if defined(ENABLE_EXTENSIONS)
 std::string SupervisedUserService::GetDebugPolicyProviderName() const {
   // Save the string space in official builds.
@@ -532,16 +543,25 @@ void SupervisedUserService::OnDefaultFilteringBehaviorChanged() {
   SupervisedUserURLFilter::FilteringBehavior behavior =
       SupervisedUserURLFilter::BehaviorFromInt(behavior_value);
   url_filter_context_.SetDefaultFilteringBehavior(behavior);
+
+  FOR_EACH_OBSERVER(
+      SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 }
 
 void SupervisedUserService::UpdateSiteLists() {
 #if defined(ENABLE_EXTENSIONS)
   url_filter_context_.LoadWhitelists(GetActiveSiteLists());
+
+  FOR_EACH_OBSERVER(
+      SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 #endif
 }
 
 void SupervisedUserService::LoadBlacklist(const base::FilePath& path) {
   url_filter_context_.LoadBlacklist(path);
+
+  FOR_EACH_OBSERVER(
+      SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 }
 
 bool SupervisedUserService::AccessRequestsEnabled() {
@@ -814,6 +834,9 @@ void SupervisedUserService::UpdateManualHosts() {
     (*host_map)[it.key()] = allow;
   }
   url_filter_context_.SetManualHosts(host_map.Pass());
+
+  FOR_EACH_OBSERVER(
+      SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 }
 
 void SupervisedUserService::UpdateManualURLs() {
@@ -827,6 +850,9 @@ void SupervisedUserService::UpdateManualURLs() {
     (*url_map)[GURL(it.key())] = allow;
   }
   url_filter_context_.SetManualURLs(url_map.Pass());
+
+  FOR_EACH_OBSERVER(
+      SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 }
 
 void SupervisedUserService::OnBrowserSetLastActive(Browser* browser) {

chrome/browser/supervised_user/supervised_user_service.h

@@ -13,6 +13,7 @@
 #include "base/callback.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/observer_list.h"
 #include "base/prefs/pref_change_registrar.h"
 #include "base/scoped_observer.h"
 #include "base/strings/string16.h"
@@ -35,6 +36,7 @@ class GoogleServiceAuthError;
 class PermissionRequestCreator;
 class Profile;
 class SupervisedUserRegistrationUtility;
+class SupervisedUserServiceObserver;
 class SupervisedUserSettingsService;
 class SupervisedUserSiteList;
 class SupervisedUserURLFilter;
@@ -166,6 +168,9 @@ class SupervisedUserService : public KeyedService,
   void AddNavigationBlockedCallback(const NavigationBlockedCallback& callback);
   void DidBlockNavigation(content::WebContents* web_contents);
 
+  void AddObserver(SupervisedUserServiceObserver* observer);
+  void RemoveObserver(SupervisedUserServiceObserver* observer);
+
 #if defined(ENABLE_EXTENSIONS)
   // extensions::ManagementPolicy::Provider implementation:
   virtual std::string GetDebugPolicyProviderName() const OVERRIDE;
@@ -326,6 +331,8 @@ class SupervisedUserService : public KeyedService,
   // Used to create permission requests.
   scoped_ptr<PermissionRequestCreator> permissions_creator_;
 
+  ObserverList<SupervisedUserServiceObserver> observer_list_;
+
   base::WeakPtrFactory<SupervisedUserService> weak_ptr_factory_;
 };
 

chrome/browser/supervised_user/supervised_user_service_observer.h

+// Copyright 2014 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 CHROME_BROWSER_SUPERVISED_USER_SUPERVISED_USER_SERVICE_OBSERVER_H_
+#define CHROME_BROWSER_SUPERVISED_USER_SUPERVISED_USER_SERVICE_OBSERVER_H_
+
+class SupervisedUserServiceObserver {
+ public:
+  // Called whenever the URL filter is updated, e.g. a manual exception or a
+  // content pack is added, or the default fallback behavior is changed.
+  virtual void OnURLFilterChanged() {}
+
+ protected:
+  virtual ~SupervisedUserServiceObserver() {}
+};
+
+#endif  // CHROME_BROWSER_SUPERVISED_USER_SUPERVISED_USER_SERVICE_OBSERVER_H_

chrome/chrome_browser.gypi

@@ -2285,6 +2285,7 @@
       'browser/supervised_user/supervised_user_service.h',
       'browser/supervised_user/supervised_user_service_factory.cc',
       'browser/supervised_user/supervised_user_service_factory.h',
+      'browser/supervised_user/supervised_user_service_observer.h',
       'browser/supervised_user/supervised_user_settings_service.cc',
       'browser/supervised_user/supervised_user_settings_service.h',
       'browser/supervised_user/supervised_user_settings_service_factory.cc',