Fix how captive portals track which page is loading.

chrome/browser/captive_portal/captive_portal_tab_helper.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_CAPTIVE_PORTAL_CAPTIVE_PORTAL_TAB_HELPER_H_
 #define CHROME_BROWSER_CAPTIVE_PORTAL_CAPTIVE_PORTAL_TAB_HELPER_H_
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "chrome/browser/captive_portal/captive_portal_service.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "webkit/glue/resource_type.h"
 
 class GURL;
 class Profile;
 
 namespace captive_portal {
 
 class CaptivePortalLoginDetector;
 class CaptivePortalTabReloader;
 
 // Along with the classes it owns, responsible for detecting page loads broken
 // by a captive portal, triggering captive portal checks on navigation events
 // that may indicate a captive portal is present, or has been removed / logged
 // in to, and taking any correcting actions.
 //
 // It acts as a WebContentsObserver for its CaptivePortalLoginDetector and
 // CaptivePortalTabReloader.  It filters out non-main-frame resource loads, and
 // treats the commit of an error page as a single event, rather than as 3
 // (ProvisionalLoadFail, DidStartProvisionalLoad, DidCommit), which simplifies
 // the CaptivePortalTabReloader.  It is also needed by CaptivePortalTabReloaders
 // to inform the tab's CaptivePortalLoginDetector when the tab is at a captive
 // portal's login page.
 //
-// TODO(mmenke): Support redirects.  Needed for HSTS, which simulates redirects
-//               at the network layer.  Also may reduce the number of
-//               unnecessary captive portal checks on high latency connections.
+// The TabHelper assumes that a WebContents can only have one RenderViewHost
+// with a provisional load at a time, and tracks only that navigation.  While
+// this may not be perfectly accurate in all cases, it should be good enough.

[cbentzel, 2012/08/15 17:00:38]

Can you be clearer in this comment about a case where there would be more than
one RVH with a provisional load?

[mmenke, 2012/08/16 14:30:03]

Done.

 //
 // For the design doc, see:
 // https://docs.google.com/document/d/1k-gP2sswzYNvryu9NcgN7q5XrsMlUdlUdoW9WRaEmfM/edit
 class CaptivePortalTabHelper : public content::WebContentsObserver,
                                public content::NotificationObserver,
                                public base::NonThreadSafe {
  public:
   CaptivePortalTabHelper(Profile* profile,
                          content::WebContents* web_contents);
   virtual ~CaptivePortalTabHelper();
@@ skipping 27 matching lines @@
   // content::NotificationObserver:
   virtual void Observe(
       int type,
       const content::NotificationSource& source,
       const content::NotificationDetails& details) OVERRIDE;
 
   // A "Login Tab" is a tab that was originally at a captive portal login
   // page.  This is set to false when a captive portal is no longer detected.
   bool IsLoginTab() const;
 
- private:
-  friend class CaptivePortalBrowserTest;
-  friend class CaptivePortalTabHelperTest;
-
+ protected:

[cbentzel, 2012/08/15 17:00:38]

Why are these protected instead of private?

[mmenke, 2012/08/16 14:30:03]

Since I had to override GetProvisionalChildID for unit tests anyways, I had the
subclass friend the test fixture, rather than the CaptivePortalTabHelper itself.
 I was thinking this would make it slightly clearer what functions were used by
the tests.

Since this apparently wasn't true, and the tests no longer need to use a
subclass, I've gone ahead and removed the protected section.

   // Called by Observe in response to the corresponding event.
-  void OnRedirect(int64 frame_id, const GURL& new_url);
-
-  // Called by Observe in response to the corresponding event.
-  void OnCaptivePortalResults(Result previous_result, Result result);
+  void OnRedirect(int child_id,
+                  ResourceType::Type resource_type,
+                  const GURL& new_url);
 
   // Called to indicate a tab is at, or is navigating to, the captive portal
   // login page.
   void SetIsLoginTab();
 
   // |this| takes ownership of |tab_reloader|.
   void SetTabReloaderForTest(CaptivePortalTabReloader* tab_reloader);
 
+  const content::RenderViewHost* provisional_render_view_host() const {
+    return provisional_render_view_host_;
+  }
+
+ private:
+  friend class CaptivePortalBrowserTest;
+
+  // Called by Observe in response to the corresponding event.
+  void OnCaptivePortalResults(Result previous_result, Result result);
+
   CaptivePortalTabReloader* GetTabReloaderForTest();
 
   // Opens a login tab if the profile's active window doesn't have one already.
   void OpenLoginTab();
 
+  // Returns the child ID of the provisional RenderViewHost's Renderer process.
+  // Returns -1 if there's no such RenderViewHost.
+  virtual int GetProvisionalChildID() const;
+
   // Neither of these will ever be NULL.
   scoped_ptr<CaptivePortalTabReloader> tab_reloader_;
   scoped_ptr<CaptivePortalLoginDetector> login_detector_;
 
   Profile* profile_;
 
   // If a provisional load has failed, and the tab is loading an error page, the
   // error code associated with the error page we're loading.
   // net::OK, otherwise.
   int pending_error_code_;
 
-  // The ID of the main frame that's currently provisionally loaded, if there is
-  // one.  -1 (unknown/invalid) when there is no such frame, or when an id of
-  // -1 is passed to DidStartProvisionalLoadForFrame.
-  int64 provisional_main_frame_id_;
+  // The RenderViewHost with a provisional load, if any.  Can either be
+  // the currently displayed RenderViewHost or a pending RenderViewHost for
+  // cross-process navitations.  NULL when there's currently no provisional
+  // load.
+  content::RenderViewHost* provisional_render_view_host_;
 
   content::NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(CaptivePortalTabHelper);
 };
 
 }  // namespace captive_portal
 
 #endif  // CHROME_BROWSER_CAPTIVE_PORTAL_CAPTIVE_PORTAL_TAB_HELPER_H_