Fix managed mode allow/block flow

chrome/browser/managed_mode/managed_mode_navigation_observer.h

Index: chrome/browser/managed_mode/managed_mode_navigation_observer.h
diff --git a/chrome/browser/managed_mode/managed_mode_navigation_observer.h b/chrome/browser/managed_mode/managed_mode_navigation_observer.h
index 1ddd4953e9a79fb2549b6c5dd43ccf931b6df305..6490c8dc4c7ca612cad532dc7f90f066cc01efc4 100644
--- a/chrome/browser/managed_mode/managed_mode_navigation_observer.h
+++ b/chrome/browser/managed_mode/managed_mode_navigation_observer.h
@@ -69,8 +69,6 @@ class ManagedModeNavigationObserver
   // an interstitial for this RenderView. This allows the user to navigate
   // around on the website after clicking preview.
   void AddTemporaryException();
-  // Updates the ResourceThrottle with the latest user navigation status.
-  void UpdateExceptionNavigationStatus();
   void RemoveTemporaryException();
 
   void AddURLToPatternList(const GURL& url);
@@ -102,7 +100,6 @@ class ManagedModeNavigationObserver
       const GURL& url,
       content::PageTransition transition_type,
       content::RenderViewHost* render_view_host) OVERRIDE;
-  virtual void DidGetUserGesture() OVERRIDE;
 
   // Returns whether the user would stay in elevated state if he visits this
   // URL.
@@ -118,12 +115,6 @@ class ManagedModeNavigationObserver
   InfoBarDelegate* warn_infobar_delegate_;
   InfoBarDelegate* preview_infobar_delegate_;
 
-  // Whether we received a user gesture.
-  // The goal is to allow automatic redirects (in order not to break the flow
-  // or show too many interstitials) while not allowing the user to navigate
-  // to blocked pages. We consider a redirect to be automatic if we did
-  // not get a user gesture.
-  bool got_user_gesture_;
   ObserverState state_;
   std::set<GURL> navigated_urls_;
   GURL last_url_;
@@ -133,6 +124,15 @@ class ManagedModeNavigationObserver
   bool is_elevated_;
 
   int last_allowed_page_;
+  // There are two starting points for a new navigation:

[Bernhard Bauer, 2013/04/08 12:33:07]

Nit: newline before the comment please.

[Sergiu, 2013/04/08 14:11:09]

Done.

+  // 1. NavigateToPendingEntry when the omnibox is used to navigate to a URL or
+  //    the user goes back or forward.
+  // 2. ProvisionalChangeToMainFrameURL when the user clicks on a link.
+  // The main problem is that ProvisionalChangeToMainFrameURL is called for
+  // redirects as well and we need a way to distinguish between the two
+  // scenarios. |after_click_on_link_| helps us do that by tracking the cases
+  // when the user did not click on a URL.
+  bool after_click_on_link_;

[Bernhard Bauer, 2013/04/08 12:33:07]

It's still not clear to me from the comment how the flag tracks that. From the
implementation, it seems you're tracking the fact that we got a
DidNavigateMainFrame() call, which means we're done with redirects. Maybe it
would be better to call the flag |finished_redirects_| or something then?

Also (and perhaps more importantly), what about page-initiated redirects after
DidNavigateMainFrame()? Are we regarding these as user clicks?

[Sergiu, 2013/04/08 14:11:09]

Changed the variable name, it makes more sense indeed. Like I said in one of the
TODO's in the .cc file, yes, we are regarding those as they would come from the
user. As a result there is one scenario where I think this approach fails and
that is when we have a client-side redirect that leads to a different domain.
I'm still thinking of a way cover that scenario as well.

 
   DISALLOW_COPY_AND_ASSIGN(ManagedModeNavigationObserver);
 };