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

chrome/browser/managed_mode/managed_mode_resource_throttle.h

Index: chrome/browser/managed_mode/managed_mode_resource_throttle.h
diff --git a/chrome/browser/managed_mode/managed_mode_resource_throttle.h b/chrome/browser/managed_mode/managed_mode_resource_throttle.h
index 886fb2a2d9b7930f1be01bbd6e6c74096991bdeb..b5652aba3ae6df43b06f9e3d2dc0febab6f9522d 100644
--- a/chrome/browser/managed_mode/managed_mode_resource_throttle.h
+++ b/chrome/browser/managed_mode/managed_mode_resource_throttle.h
@@ -15,10 +15,6 @@ namespace net {
 class URLRequest;
 }
 
-namespace policy {
-class URLBlacklist;
-}
-
 class ManagedModeResourceThrottle : public content::ResourceThrottle {
  public:
   ManagedModeResourceThrottle(const net::URLRequest* request,
@@ -27,9 +23,26 @@ class ManagedModeResourceThrottle : public content::ResourceThrottle {
                               bool is_main_frame);
   virtual ~ManagedModeResourceThrottle();
 
+  // Adds/removes the observer characterized by the render_process_host_id and

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

AddObserver makes it sound like the method adds an observer to this object,
which is not really what happens. What we do is (temporarily) whitelist all
requests for the given RenderView and host?

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

Renamed.

+  // render_view_id to the preview map. Adding saves the last approved
+  // hostname as well.
+  static void AddObserver(int render_process_host_id,
+                          int render_view_id,
+                          const std::string& host);
+  static void RemoveObserver(int render_process_host_id, int render_view_id);
+
+  // Checks whether the |url| needs to trigger an interstitial. If it
+  // |is_redirect| and the user already saw an interstitial which lead to this

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

Nit: the simple past of "lead" is "led".

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

Done.

+  // url then do not show another one.

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

Nit: URL in upper case if you use it as a noun, |url| in pipes if you're
referring to the variable.

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

Done.

+  void CheckNeedToShowInterstitial(bool is_redirect,

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

This actually *does* show the interstitial if necessary, right? You should name
the method to indicate that.

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

Done.

+                                   const GURL& url,
+                                   bool* defer);
+
   // content::ResourceThrottle implementation:
   virtual void WillStartRequest(bool* defer) OVERRIDE;
 
+  virtual void WillRedirectRequest(const GURL& new_url, bool* defer) OVERRIDE;
+
  private:
   void OnInterstitialResult(bool continue_request);
 
@@ -38,6 +51,9 @@ class ManagedModeResourceThrottle : public content::ResourceThrottle {
   int render_process_host_id_;
   int render_view_id_;
   bool is_main_frame_;
+  // A resource throttle is in preview mode if the user clicked "Preview" after
+  // an interstitial.

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

I think this class also shouldn't know about preview mode. Can we just talk
about this in terms of behavior (i.e. in some cases we show an interstitial, in
some cases we don't)?

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

Renamed it to after_interstitial_ which I think fits and relates to the
ResourceThrottle.

+  bool is_in_preview_mode_;
   const ManagedModeURLFilter* url_filter_;
 
   DISALLOW_COPY_AND_ASSIGN(ManagedModeResourceThrottle);