Support manual (white|black)list, previewing and allowing after interstitial

chrome/browser/managed_mode/managed_mode_navigation_observer.cc

Index: chrome/browser/managed_mode/managed_mode_navigation_observer.cc
diff --git a/chrome/browser/managed_mode/managed_mode_navigation_observer.cc b/chrome/browser/managed_mode/managed_mode_navigation_observer.cc
index 4abf30f2afe845a4e4205ab37a4c1acd28551dad..ac7aebbd4e16abacdb8e4e9e49c810fa5e5cf943 100644
--- a/chrome/browser/managed_mode/managed_mode_navigation_observer.cc
+++ b/chrome/browser/managed_mode/managed_mode_navigation_observer.cc
@@ -11,6 +11,7 @@
 #include "chrome/browser/infobars/infobar_tab_helper.h"
 #include "chrome/browser/managed_mode/managed_mode.h"
 #include "chrome/browser/managed_mode/managed_mode_interstitial.h"
+#include "chrome/browser/managed_mode/managed_mode_resource_throttle.h"
 #include "chrome/browser/managed_mode/managed_mode_url_filter.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ -18,17 +19,18 @@
 #include "chrome/common/jstemplate_builder.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/common/frame_navigate_params.h"
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "ui/base/l10n/l10n_util.h"
 
-namespace {
+using content::BrowserThread;
 
-bool IsInList(const ListValue *list, const std::string& url_to_add) {
-  return list->Find(*Value::CreateStringValue(url_to_add)) != list->end();
-}
+namespace {
 
 class ManagedModeWarningInfobarDelegate : public ConfirmInfoBarDelegate {
  public:
@@ -157,22 +159,26 @@ string16 ManagedModePreviewInfobarDelegate::GetButtonLabel(
     InfoBarButton button) const {
   return l10n_util::GetStringUTF16(
       (button == BUTTON_OK) ? IDS_MANAGED_MODE_PREVIEW_ACCEPT
-                            : IDS_MANAGED_MODE_PREVIEW_CANCEL);
+                            : IDS_MANAGED_MODE_GO_BACK_ACTION);
 }
 
 bool ManagedModePreviewInfobarDelegate::Accept() {
   ManagedModeNavigationObserver* observer =
       ManagedModeNavigationObserver::FromWebContents(
           owner()->GetWebContents());
-  observer->AddURLList();
-  // Clear the pointer as the infobar was closed.
-  observer->PreviewInfobarDismissed();
+  observer->AddSavedURLsToWhitelist();
+  // Notify the navigation observer that the infobar was dismissed.
+  observer->ClearInterstitialState();
 
   return true;
 }
 
 bool ManagedModePreviewInfobarDelegate::Cancel() {
   // TODO(bauerb): Go back to the last page.
+  ManagedModeNavigationObserver* observer =
+      ManagedModeNavigationObserver::FromWebContents(
+          owner()->GetWebContents());
+  observer->ClearInterstitialState();
   return false;
 }
 
@@ -193,7 +199,9 @@ void ManagedModePreviewInfobarDelegate::InfoBarDismissed() {
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(ManagedModeNavigationObserver)
 
-ManagedModeNavigationObserver::~ManagedModeNavigationObserver() {}
+ManagedModeNavigationObserver::~ManagedModeNavigationObserver() {
+  RemoveObserverOnIOThread();
+}
 
 ManagedModeNavigationObserver::ManagedModeNavigationObserver(
     content::WebContents* web_contents)
@@ -202,7 +210,34 @@ ManagedModeNavigationObserver::ManagedModeNavigationObserver(
       warn_infobar_delegate_(NULL),
       preview_infobar_delegate_(NULL),
       after_interstitial_(false),
-      last_allowed_page_(-1) {}
+      redirects_completed_(false),
+      last_allowed_page_(-1) {
+  DLOG(ERROR) << "--- New navigation observer";

[Bernhard Bauer, 2012/11/26 16:06:30]

Please remove these log statements before committing.

[Sergiu, 2012/11/27 15:37:34]

Done.

+}
+
+void ManagedModeNavigationObserver::AddObserverOnIOThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[Bernhard Bauer, 2012/11/26 16:06:30]

"...OnIOThread()" methods are called on the IO thread. I think you don't need to
expose the fact that this method posts a task *to* the IO thread in its name.

[Sergiu, 2012/11/27 15:37:34]

Renamed.

+  if (web_contents()) {

[Bernhard Bauer, 2012/11/26 16:06:30]

You could early-return if web_contents() is NULL.

Also, when does that happen?

[Sergiu, 2012/11/27 15:37:34]

It can happen in the RemoveObserverFunction when the browser shuts down
apparently. I don't think that it should happen when we add the observer so I
put a DCHECK in to make sure.

+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&ManagedModeResourceThrottle::AddObserver,
+                   web_contents()->GetRenderProcessHost()->GetID(),
+                   web_contents()->GetRenderViewHost()->GetRoutingID(),
+                   navigated_urls_.back().host()));
+  }
+}
+
+void ManagedModeNavigationObserver::RemoveObserverOnIOThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (web_contents())
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&ManagedModeResourceThrottle::RemoveObserver,
+                   web_contents()->GetRenderProcessHost()->GetID(),
+                   web_contents()->GetRenderViewHost()->GetRoutingID()));
+}
 
 void ManagedModeNavigationObserver::WarnInfobarDismissed() {
   DCHECK(warn_infobar_delegate_);
@@ -214,70 +249,110 @@ void ManagedModeNavigationObserver::PreviewInfobarDismissed() {
   preview_infobar_delegate_ = NULL;
 }
 
-void ManagedModeNavigationObserver::AddNavigatedURL(const GURL& url) {
-  if (std::find(navigated_urls_.begin(), navigated_urls_.end(), url) !=
+void ManagedModeNavigationObserver::DismissInfobar(
+      InfoBarDelegate** dismissed_infobar_delegate) {

[Bernhard Bauer, 2012/11/26 16:06:30]

I'm not sure if the saved lines are worth adding an interface to dismiss and
NULL out _arbitrary_ InfoBarDelegate pointers.

[Sergiu, 2012/11/27 15:37:34]

Removed the function.

+  if (!*dismissed_infobar_delegate)
+    return;
+
+  InfoBarTabHelper* infobar_tab_helper =
+      InfoBarTabHelper::FromWebContents(web_contents());
+  infobar_tab_helper->RemoveInfoBar(*dismissed_infobar_delegate);
+  *dismissed_infobar_delegate= NULL;

[Bernhard Bauer, 2012/11/26 16:06:30]

Nit: Space before equals sign.

[Sergiu, 2012/11/27 15:37:34]

Done.

+}
+
+void ManagedModeNavigationObserver::SaveNavigatedURL(const GURL& url) {
+  if (redirects_completed_)
+    return;
+
+  if (std::find(navigated_urls_.begin(), navigated_urls_.end(), url) ==
       navigated_urls_.end()) {
     navigated_urls_.push_back(url);
   }
 }
 
-void ManagedModeNavigationObserver::AddURLList() {
-  // Get a copy of the whitelist since it can't be edited in place.
-  // |whitelist| is the preference list while AddStringToManualWhitelist adds
-  // the navigated urls to the URL filter.
-  scoped_ptr<base::ListValue> whitelist(
-      ManagedMode::GetWhitelist()->DeepCopy());
-  std::string url_to_add;
-  int added_url_count = 0;
-
-  for (std::vector<GURL>::const_iterator it = navigated_urls_.begin();
-       it+1 != navigated_urls_.end(); ++it) {
-    url_to_add = it->spec();
-    if (!IsInList(ManagedMode::GetWhitelist().get(), url_to_add)) {
-      DLOG(ERROR) << "Adding (exact):" << url_to_add;
-      ManagedMode::AddStringToManualWhitelist(url_to_add);
-      whitelist->Append(Value::CreateStringValue(url_to_add));
-      ++added_url_count;
+void ManagedModeNavigationObserver::AddSavedURLsToWhitelist() {
+  // |whitelist| is used to add the URLs to the preference list while
+  // AddURLPatternToManualWhitelist adds the navigated urls to the URL filter.
+  base::ListValue whitelist;
+  std::string pattern_to_add;
+  bool added_urls = false;
+
+  if (!navigated_urls_.empty()) {
+    for (std::vector<GURL>::const_iterator it = navigated_urls_.begin();
+         it+1 != navigated_urls_.end(); ++it) {
+      GURL current_url = *it;
+      GURL::Replacements replaced_components;
+      // Add the . to match the exact host.
+      replaced_components.SetHostStr("."+it->host());

[Bernhard Bauer, 2012/11/26 16:06:30]

Nit: space around plus sign.

[Sergiu, 2012/11/27 15:37:34]

Done.

+      replaced_components.SetRefStr("");
+      replaced_components.SetSchemeStr("");
+      current_url.ReplaceComponents(replaced_components);
+      pattern_to_add = current_url.spec();
+      DLOG(ERROR) << "Adding (exact):" << pattern_to_add;
+      whitelist.AppendString(pattern_to_add);
     }
-  }
 
-  // If the URL uses https add the protocol as well instead of just the
-  // hostname.
-  if (navigated_urls_.back().SchemeIs("https")) {
-    url_to_add = navigated_urls_.back().GetOrigin().spec();
-  } else {
-    url_to_add = navigated_urls_.back().host();
+    // If the URL uses https add the protocol as well. Also since this is the

[Bernhard Bauer, 2012/11/26 16:06:30]

Nit: HTTPS in upper case please, and commas would be nice.

[Sergiu, 2012/11/27 15:37:34]

Done.

+    // final destination add the whole subdomain.
+    // hostname.

[Bernhard Bauer, 2012/11/26 16:06:30]

?

[Sergiu, 2012/11/27 15:37:34]

Done.

+    if (navigated_urls_.back().SchemeIs("https")) {
+      pattern_to_add = "https://" + navigated_urls_.back().host();
+    } else {
+      pattern_to_add = navigated_urls_.back().host();
+    }
+
+    DLOG(ERROR) << "Adding (hostname): " << pattern_to_add;
+    whitelist.AppendString(pattern_to_add);
+
+    added_urls = ManagedMode::AddToManualWhitelist(whitelist);
   }
 
-  // Use the local whitelist to see if this last URL is already there.
-  if (!IsInList(ManagedMode::GetWhitelist().get(), url_to_add)) {
-    DLOG(ERROR) << "Adding (hostname): " << url_to_add;
-    ManagedMode::AddStringToManualWhitelist(url_to_add);
-    whitelist->Append(Value::CreateStringValue(url_to_add));
-    ++added_url_count;
-  } else {
+  if ((navigated_urls_.empty() || !added_urls) && after_interstitial_) {
     // Tell the user that the site was already present in the whitelist.
     InfoBarTabHelper* infobar_tab_helper =
         InfoBarTabHelper::FromWebContents(web_contents());
     infobar_tab_helper->AddInfoBar(new SimpleAlertInfoBarDelegate(
         infobar_tab_helper,
         NULL,
-        l10n_util::GetStringFUTF16(IDS_MANAGED_MODE_ALREADY_ADDED_MESSAGE,
-                                   base::IntToString16(added_url_count)),
+        l10n_util::GetStringUTF16(IDS_MANAGED_MODE_ALREADY_ADDED_MESSAGE),
         true));
   }
 
-  ManagedMode::SetWhitelist(whitelist.get());
+}
+
+void ManagedModeNavigationObserver::MarkShownInterstitial() {
+  after_interstitial_ = true;
+}
+
+void ManagedModeNavigationObserver::ClearShownInterstitial() {
+  after_interstitial_ = false;
+}
+
+void ManagedModeNavigationObserver::ClearInterstitialState() {
+  DLOG(ERROR) << "Clearing interstitial state";
+  if (after_interstitial_) {
+    DismissInfobar(&preview_infobar_delegate_);
+  }
+  navigated_urls_.clear();
+  after_interstitial_ = false;
+  redirects_completed_ = false;
+  RemoveObserverOnIOThread();
 }
 
 void ManagedModeNavigationObserver::NavigateToPendingEntry(
     const GURL& url,
     content::NavigationController::ReloadType reload_type) {
   DLOG(ERROR) << "NavigateToPendingEntry: " << url;
-  // This means that a new navigation was instantiated and the data related to
-  // the list of URLs needs to be cleared.
-  navigated_urls_.clear();
-  after_interstitial_ = false;
+
+  // This method gets called first when a user navigates to a (new) URL.
+  // This means that the data related to the list of URLs needs to be cleared
+  // in certain circumstances.
+  if (web_contents()->GetController().GetCurrentEntryIndex() <
+      last_allowed_page_ ||
+      navigated_urls_.empty() ||
+      navigated_urls_.back().host() != url.host()) {
+    ClearInterstitialState();
+  }
 }
 
 void ManagedModeNavigationObserver::DidNavigateMainFrame(
@@ -288,14 +363,18 @@ void ManagedModeNavigationObserver::DidNavigateMainFrame(
   ManagedModeURLFilter::FilteringBehavior behavior =
       url_filter_->GetFilteringBehaviorForURL(params.url);
 
-  if (behavior != ManagedModeURLFilter::ALLOW)
-    AddNavigatedURL(params.url);
+  if (!redirects_completed_)
+    SaveNavigatedURL(params.url);
 
   if (behavior == ManagedModeURLFilter::ALLOW && after_interstitial_) {
     // The initial page that triggered the interstitial was blocked but the
     // final page is already in the whitelist so add the series of URLs
     // which lead to the final page to the whitelist as well.
-    AddURLList();
+    AddSavedURLsToWhitelist();
+  }
+  if (after_interstitial_ && !redirects_completed_) {
+    redirects_completed_ = true;
+    AddObserverOnIOThread();
   }
 }
 
@@ -317,14 +396,15 @@ void ManagedModeNavigationObserver::ProvisionalChangeToMainFrameUrl(
     const GURL& opener_url,
     content::RenderViewHost* render_view_host) {
   DLOG(ERROR) << "ProvisionalChangeToMainFrameUrl: " << url;
-  // Mark the fact that an interstitial will be triggered here if the URL
-  // must be blocked.
+  // This function is the last one to be called before the resource throttle
+  // shows the interstitial if the URL must be blocked.
   ManagedModeURLFilter::FilteringBehavior behavior =
       url_filter_->GetFilteringBehaviorForURL(url);
-  if (behavior == ManagedModeURLFilter::BLOCK)
-    after_interstitial_ = true;
-  if (behavior != ManagedModeURLFilter::ALLOW)
-    AddNavigatedURL(url);
+  if (!navigated_urls_.empty() && redirects_completed_ &&
+      navigated_urls_.back().host() != url.host())
+    ClearInterstitialState();

[Bernhard Bauer, 2012/11/26 16:06:30]

Nit: newline please. Also, I thought we wanted to trigger this from the
ResourceThrottle?

[Sergiu, 2012/11/27 15:37:34]

We set the Observer state from the interstitial and act according to it here. Or
do you suggest we do it another way?

+  if (behavior != ManagedModeURLFilter::ALLOW && !redirects_completed_)
+    SaveNavigatedURL(url);
 }
 
 void ManagedModeNavigationObserver::DidCommitProvisionalLoadForFrame(
@@ -340,7 +420,6 @@ void ManagedModeNavigationObserver::DidCommitProvisionalLoadForFrame(
   ManagedModeURLFilter::FilteringBehavior behavior =
       url_filter_->GetFilteringBehaviorForURL(url);
 
-  DLOG(ERROR) << "Current behavior: " << behavior;
   if (behavior == ManagedModeURLFilter::WARN) {
     if (!warn_infobar_delegate_) {
       InfoBarTabHelper* infobar_tab_helper =
@@ -352,15 +431,11 @@ void ManagedModeNavigationObserver::DidCommitProvisionalLoadForFrame(
     }
   } else {
     if (warn_infobar_delegate_) {
-      InfoBarTabHelper* infobar_tab_helper =
-          InfoBarTabHelper::FromWebContents(web_contents());
-      infobar_tab_helper->RemoveInfoBar(warn_infobar_delegate_);
-      warn_infobar_delegate_= NULL;
+      DismissInfobar(&warn_infobar_delegate_);
     }
-    last_allowed_page_ = web_contents()->GetController().GetCurrentEntryIndex();
   }
 
-  if (behavior == ManagedModeURLFilter::BLOCK) {
+  if (after_interstitial_ && behavior == ManagedModeURLFilter::BLOCK) {
     if (!preview_infobar_delegate_) {
       InfoBarTabHelper* infobar_tab_helper =
           InfoBarTabHelper::FromWebContents(web_contents());
@@ -370,10 +445,11 @@ void ManagedModeNavigationObserver::DidCommitProvisionalLoadForFrame(
     }
   } else {
     if (preview_infobar_delegate_) {
-      InfoBarTabHelper* infobar_tab_helper =
-          InfoBarTabHelper::FromWebContents(web_contents());
-      infobar_tab_helper->RemoveInfoBar(preview_infobar_delegate_);
-      preview_infobar_delegate_= NULL;
+      DismissInfobar(&preview_infobar_delegate_);
     }
   }
+
+  if (behavior == ManagedModeURLFilter::ALLOW) {
+    last_allowed_page_ = web_contents()->GetController().GetCurrentEntryIndex();
+  }
 }