Watch for filtering pref changes in ManagedModeInterstitial and unblock requests if they become…

chrome/browser/managed_mode/managed_mode_interstitial.cc

Index: chrome/browser/managed_mode/managed_mode_interstitial.cc
diff --git a/chrome/browser/managed_mode/managed_mode_interstitial.cc b/chrome/browser/managed_mode/managed_mode_interstitial.cc
index b3daecc6e2eefdb11b5fffdc6082455324af2fb2..cc858c308e9644831d7d672a2eee53f2275477e0 100644
--- a/chrome/browser/managed_mode/managed_mode_interstitial.cc
+++ b/chrome/browser/managed_mode/managed_mode_interstitial.cc
@@ -17,7 +17,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/web_contents.h"
-#include "content/public/browser/web_contents_delegate.h"
 #include "content/public/browser/web_ui.h"
 #include "grit/browser_resources.h"
 #include "grit/generated_resources.h"
@@ -34,13 +33,37 @@ ManagedModeInterstitial::ManagedModeInterstitial(
     const GURL& url,
     const base::Callback<void(bool)>& callback)
     : web_contents_(web_contents),
+      interstitial_page_(NULL),
       url_(url),
-      weak_ptr_factory_(this),
       callback_(callback) {
+  if (ShouldProceed()) {
+    // It can happen that the site was only allowed very recently and the URL
+    // filter on the IO thread had not been updated yet. Proceed with the
+    // request without showing the interstitial

[Pam (message me for reviews), 2013/09/04 09:56:59]

missing .

[Bernhard Bauer, 2013/09/04 11:48:35]

Done.

+    DispatchContinueRequest(true);
+    delete this;
+    return;
+  }
+
+  // TODO(bauerb): Extract an observer callback on ManagedUserService for this.
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  languages_ = profile->GetPrefs()->GetString(prefs::kAcceptLanguages);
-
+  PrefService* prefs = profile->GetPrefs();
+  pref_change_registrar_.Init(prefs);
+  pref_change_registrar_.Add(
+      prefs::kDefaultManagedModeFilteringBehavior,
+      base::Bind(&ManagedModeInterstitial::OnFilteringPrefsChanged,
+                 base::Unretained(this)));
+  pref_change_registrar_.Add(
+      prefs::kManagedModeManualHosts,
+      base::Bind(&ManagedModeInterstitial::OnFilteringPrefsChanged,
+                 base::Unretained(this)));
+  pref_change_registrar_.Add(
+      prefs::kManagedModeManualURLs,
+      base::Bind(&ManagedModeInterstitial::OnFilteringPrefsChanged,
+                 base::Unretained(this)));
+
+  languages_ = prefs->GetString(prefs::kAcceptLanguages);
   interstitial_page_ =
       content::InterstitialPage::Create(web_contents, true, url_, this);
   interstitial_page_->Show();
@@ -125,12 +148,31 @@ void ManagedModeInterstitial::CommandReceived(const std::string& command) {
   NOTREACHED();
 }
 
-void ManagedModeInterstitial::OnProceed() { NOTREACHED(); }
+void ManagedModeInterstitial::OnProceed() {
+  DCHECK(ShouldProceed());

[Pam (message me for reviews), 2013/09/04 09:56:59]

Should we just crash? I could imagine some situations where showing a blocked
page would be worse than crashing.

[Bernhard Bauer, 2013/09/04 11:48:35]

Well, this is really one of these "this cannot happen" situations. I feel like
purposely crashing would be trying to handle it, which you're not supposed to
do. 

[Pam (message me for reviews), 2013/09/05 10:13:52]

Keeping going somehow even though something unexpected has happened is
"handling" it. Crashing would be "not handling". I'm proposing changing the
DCHECK to a CHECK, on the grounds that showing a blocked site is kinda-sorta a
"security" problem.

[Bernhard Bauer, 2013/09/05 12:49:10]

What I meant by handling was that a CHECK results in production code that checks
this condition.
Fair enough. I changed it to a CHECK, and added a comment.

+  DispatchContinueRequest(true);
+}
 
 void ManagedModeInterstitial::OnDontProceed() {
   DispatchContinueRequest(false);
 }
 
+bool ManagedModeInterstitial::ShouldProceed() {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents_->GetBrowserContext());
+  ManagedUserService* managed_user_service =
+      ManagedUserServiceFactory::GetForProfile(profile);
+  ManagedModeURLFilter* url_filter =
+      managed_user_service->GetURLFilterForUIThread();
+  return url_filter->GetFilteringBehaviorForURL(url_) !=
+         ManagedModeURLFilter::BLOCK;
+}
+
+void ManagedModeInterstitial::OnFilteringPrefsChanged() {
+  if (ShouldProceed())
+    interstitial_page_->Proceed();
+}
+
 void ManagedModeInterstitial::DispatchContinueRequest(bool continue_request) {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE, base::Bind(callback_, continue_request));