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

chrome/browser/managed_mode/managed_mode_resource_throttle.cc

Index: chrome/browser/managed_mode/managed_mode_resource_throttle.cc
diff --git a/chrome/browser/managed_mode/managed_mode_resource_throttle.cc b/chrome/browser/managed_mode/managed_mode_resource_throttle.cc
index 7e6b28e4c09c574cdb002f896663425db5e6bad1..bd6e80704d6d0cb844210965fd5fcc0f50c5a9c2 100644
--- a/chrome/browser/managed_mode/managed_mode_resource_throttle.cc
+++ b/chrome/browser/managed_mode/managed_mode_resource_throttle.cc
@@ -4,13 +4,27 @@
 
 #include "chrome/browser/managed_mode/managed_mode_resource_throttle.h"
 
+#include "base/lazy_instance.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_navigation_observer.h"

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

We don't need this, I think.

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

Done.

 #include "chrome/browser/managed_mode/managed_mode_url_filter.h"
-#include "chrome/browser/policy/url_blacklist_manager.h"
+#include "chrome/browser/tab_contents/tab_util.h"

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

Why do we need this?

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

I needed it, then deleted the call apparently. :(

 #include "content/public/browser/resource_controller.h"
 #include "net/url_request/url_request.h"
 
+namespace {
+
+// This map contains hostnames mapped to <render_process_host_id_,
+// render_view_id> pairs which identify individual tabs. If a hostname

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

The other way around.

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

Keys mapped to values, makes sense.

+// is present for a specific pair then the user clicked preview, is
+// navigating around and has not clicked one of the options on the infobar.
+typedef std::map<std::pair<int, int>, std::string > PreviewMap;

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

Nit: no space before >

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

Done.

+static base::LazyInstance<PreviewMap>
+    in_preview_mode = LAZY_INSTANCE_INITIALIZER;
+
+}
+
 ManagedModeResourceThrottle::ManagedModeResourceThrottle(
     const net::URLRequest* request,
     int render_process_host_id,
@@ -21,29 +35,68 @@ ManagedModeResourceThrottle::ManagedModeResourceThrottle(
       render_process_host_id_(render_process_host_id),
       render_view_id_(render_view_id),
       is_main_frame_(is_main_frame),
+      is_in_preview_mode_(false),
       url_filter_(ManagedMode::GetURLFilterForIOThread()) {}
 
 ManagedModeResourceThrottle::~ManagedModeResourceThrottle() {}
 
-void ManagedModeResourceThrottle::WillStartRequest(bool* defer) {
-  if (url_filter_->GetFilteringBehaviorForURL(request_->url()) !=
+// static
+void ManagedModeResourceThrottle::AddObserver(int render_process_host_id,
+                                              int render_view_id,
+                                              const std::string& host) {
+  in_preview_mode.Get().insert(
+      std::make_pair(std::make_pair(render_process_host_id, render_view_id),
+                     host));
+}
+
+// static
+void ManagedModeResourceThrottle::RemoveObserver(int render_process_host_id,
+                                                 int render_view_id) {
+  if (in_preview_mode.Pointer()) {

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

Why this check?

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

I was under the impression that in_preview_mode can become null when the browser
is closing for example but I think it was just web_contents() that can return
null.

+    in_preview_mode.Get().erase(std::make_pair(render_process_host_id,
+                                               render_view_id));
+  }
+}
+
+void ManagedModeResourceThrottle::CheckNeedToShowInterstitial(bool is_redirect,
+                                                              const GURL& url,
+                                                              bool* defer) {
+  // Only treat main frame requests for now (without subresources).
+  if (!is_main_frame_)
+    return;
+
+  if (url_filter_->GetFilteringBehaviorForURL(url) !=
       ManagedModeURLFilter::BLOCK)
     return;
 
-  if (is_main_frame_) {
-    *defer = true;
-    ManagedModeInterstitial::ShowInterstitial(
-        render_process_host_id_, render_view_id_, request_->url(),
-        base::Bind(&ManagedModeResourceThrottle::OnInterstitialResult,
-                   weak_ptr_factory_.GetWeakPtr()));
-  } else {
-    // NOTREACHED();
-    // XXX
-  }
+  // Do not show interstitial for redirects in preview mode and URLs which have
+  // the same hostname as the one on which the user clicked "Preview" on.
+  PreviewMap::iterator it = in_preview_mode.Get().find(

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

You could probably save the map pointer you get here in a local variable.

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

Done.

+      std::make_pair(render_process_host_id_, render_view_id_));
+  if ((is_redirect && is_in_preview_mode_) ||

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

Hm. If |is_in_preview_mode_| is true, we should see only redirects, so
|is_redirect| should also be true. Could you DCHECK that?

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

Done.

+      (it != in_preview_mode.Get().end() && url.host() == it->second))

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

You could split these up into separate if-statements for readability.

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

Done.

+    return;
+
+  *defer = true;
+  DLOG(ERROR) << "Showing interstitial";
+  ManagedModeInterstitial::ShowInterstitial(
+      render_process_host_id_, render_view_id_, url,
+      base::Bind(&ManagedModeResourceThrottle::OnInterstitialResult,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void ManagedModeResourceThrottle::WillStartRequest(bool* defer) {
+  CheckNeedToShowInterstitial(false, request_->url(), defer);
+}
+
+void ManagedModeResourceThrottle::WillRedirectRequest(const GURL& new_url,
+                                                      bool* defer) {
+  CheckNeedToShowInterstitial(true, new_url, defer);
 }
 
 void ManagedModeResourceThrottle::OnInterstitialResult(bool continue_request) {
   if (continue_request) {
+    is_in_preview_mode_ = true;
     controller()->Resume();
   } else {
     controller()->Cancel();