Add a NavigationThrottle to the public content/ interface

content/browser/frame_host/navigation_handle_impl.h

 // Copyright 2015 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 CONTENT_BROWSER_FRAME_HOST_NAVIGATION_HANDLE_IMPL_H_
 #define CONTENT_BROWSER_FRAME_HOST_NAVIGATION_HANDLE_IMPL_H_
 
 #include "content/public/browser/navigation_handle.h"
 
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/navigation_throttle.h"
 #include "url/gurl.h"
 
 namespace content {
 
 class NavigatorDelegate;
 struct NavigationRequestInfo;
 
 // This class keeps track of a single navigation. It is created upon receipt of
 // a DidStartProvisionalLoad IPC in a RenderFrameHost. The RenderFrameHost owns
 // the newly created NavigationHandleImpl as long as the navigation is ongoing.
@@ skipping 22 matching lines @@
 // NavigationRequest. It is then owned by the NavigationRequest until the
 // navigation is ready to commit. The NavigationHandleImpl ownership is then
 // transferred to the RenderFrameHost in which the navigation will commit.
 //
 // When PlzNavigate is enabled, the NavigationHandleImpl will never be reset
 // following the receipt of a DidStartProvisionalLoad IPC. There are also no
 // transferring navigations. The other causes of NavigationHandleImpl reset in
 // the RenderFrameHost still apply.
 class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
  public:
-  static scoped_ptr<NavigationHandleImpl> Create(const GURL& url,
-                                                 const bool is_main_frame,
-                                                 NavigatorDelegate* delegate);
-
   ~NavigationHandleImpl() override;
 
   // NavigationHandle implementation:
-  const GURL& GetURL() const override;
-  net::Error GetNetErrorCode() const override;
-  bool IsInMainFrame() const override;
-  bool HasCommittedDocument() const override;
-  bool HasCommittedErrorPage() const override;
+  const GURL& GetURL() override;
+  WebContents* GetWebContents() override;
+  bool IsInMainFrame() override;
+  bool IsPost() override;
+  const Referrer& GetReferrer() override;
+  bool HasUserGesture() override;
+  ui::PageTransition GetPageTransition() override;
+  bool IsExternalProtocol() override;
+  net::Error GetNetErrorCode() override;
+  bool HasCommittedDocument() override;
+  bool HasCommittedErrorPage() override;
+  void RegisterThrottleForTesting(
+      scoped_ptr<NavigationThrottle> navigation_throttle) override;
 
   void set_net_error_code(net::Error net_error_code) {
     net_error_code_ = net_error_code;
   }
 
   // Returns whether the navigation is currently being transferred from one
   // RenderFrameHost to another. In particular, a DidStartProvisionalLoad IPC
   // for the navigation URL, received in the new RenderFrameHost, should not
   // indicate the start of a new navigation in that case.
   bool is_transferring() const { return is_transferring_; }
   void set_is_transferring(bool is_transferring) {
     is_transferring_ = is_transferring;
   }
 
+  void set_web_contents(WebContents* web_contents) {
+    web_contents_ = web_contents;
+  }
+
+  // Called when the URLRequest will start in the network stack.
+  NavigationThrottle::ThrottleCheckResult WillStartRequest(
+      bool is_post,
+      const Referrer& sanitized_referrer,
+      bool has_user_gesture,
+      ui::PageTransition transition,
+      bool is_external_protocol);
+
+  // Called when the URLRequest will be redirected in the network stack.
+  NavigationThrottle::ThrottleCheckResult WillRedirectRequest(
+      const GURL& new_url,
+      bool new_method_is_post,
+      const GURL& new_referrer_url,
+      bool new_is_external_protocol);
+
   // Called when the navigation was redirected. This will update the |url_| and
   // inform the delegate.
   void DidRedirectNavigation(const GURL& new_url);
 
   // Called when the navigation was committed. This will update the |state_|
   // and inform the delegate,
   void DidCommitNavigation();
 
  private:
+  friend class NavigationHandleFactory;
+  friend class TestNavigationHandle;
+
   // Used to track the state the navigation is currently in.
   enum State {
     DID_START = 0,
+    WILL_SEND_REQUEST,
     DID_COMMIT,
     DID_COMMIT_ERROR_PAGE,
   };
 
   NavigationHandleImpl(const GURL& url,
                        const bool is_main_frame,
                        NavigatorDelegate* delegate);
 
   // See NavigationHandle for a description of those member variables.
   GURL url_;
+  const bool is_main_frame_;
+  bool is_post_;
+  Referrer sanitized_referrer_;
+  bool has_user_gesture_;
+  ui::PageTransition transition_;
+  bool is_external_protocol_;
   net::Error net_error_code_;
+
+  // The state the navigation is in.
   State state_;
-  const bool is_main_frame_;
 
   // Whether the navigation is in the middle of a transfer. Set to false when
   // the DidStartProvisionalLoad is received from the new renderer.
   bool is_transferring_;
 
   // The delegate that should be notified about events related to this
   // navigation.
   NavigatorDelegate* delegate_;
 
+  // A pointer to the WebContents the navigation is taking place in.
+  //
+  // The NavigationHandle is either owned by a NavigationRequest or a
+  // RenderFrameHost, both of which are owned by a FrameTreeNode, which is
+  // ultimately owned by the WebContents. Therefore, the NavigationHandle
+  // cannot outlive the WebContents and it is safe to use a raw pointer.
+  //
+  // Note that it makes sense for the NavigationHandleImpl to keep a pointer to
+  // the WebContents because the NavigationHandle interface is closely tied to
+  // a WebContents (it is passed outside content/ through WebContentsObserver
+  // methods).

[nasko, 2015/09/16 20:33:39]

It feels like a bit of a layering violation, but I think it is better than
passing the two always together.

Charlie, what is your take?

[Charlie Reis, 2015/09/16 20:46:07]

It's definitely a layering violation.  :)

That said, Camille made a good point that NavigationHandle and
WebContentsObserver are tightly coupled, and I haven't yet been able to come up
with a case where the NavigationHandle doesn't have a WebContents.  I'm somewhat
ok with allowing it, and hence my request for this comment.

Just to explore the alternative-- it would be better from a layering perspective
for frame_host classes not to depend on WebContents.  If we went that route,
NavigationHandle wouldn't know its WebContents, and callers would have to make
that association on their own.  This is doable but a bit clumsy and presents a
risk of bugs if they get it wrong.

Neither approach seems great, so perhaps the minor layering violation is better
to avoid the potential bugs.  Worst case, some NavigationHandles won't have a
WebContents.

+  WebContents* web_contents_;
+
+  // A list of Throttles registered for this navigation.
+  ScopedVector<NavigationThrottle> throttles_;
+
   DISALLOW_COPY_AND_ASSIGN(NavigationHandleImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_NAVIGATION_HANDLE_IMPL_H_