Skip to content

Commit

Permalink
Change the NavigationHandle interface to use a RenderFrameHost.
Browse files Browse the repository at this point in the history
This is a spin-off of https://codereview.chromium.org/1294243004/, this
changes the ctor in NavigationHandleImpl to use a FrameTreeNode and the
public test interface to use a RanderFrameHost.

Review URL: https://codereview.chromium.org/1390453002

Cr-Commit-Position: refs/heads/master@{#352353}
  • Loading branch information
Steelskin authored and Commit bot committed Oct 5, 2015
1 parent 4022a3c commit c0ad5d1
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class InterceptNavigationThrottleTest
SimulateWillStart(const GURL& url, const GURL& sanitized_url, bool is_post) {
scoped_ptr<content::NavigationHandle> test_handle =
content::NavigationHandle::CreateNavigationHandleForTesting(
url, true, web_contents());
url, main_rfh());
test_handle->RegisterThrottleForTesting(
scoped_ptr<NavigationThrottle>(
new InterceptNavigationThrottle(
Expand All @@ -76,7 +76,7 @@ class InterceptNavigationThrottleTest
NavigationThrottle::ThrottleCheckResult Simulate302() {
scoped_ptr<content::NavigationHandle> test_handle =
content::NavigationHandle::CreateNavigationHandleForTesting(
GURL(kTestUrl), true, web_contents());
GURL(kTestUrl), main_rfh());
test_handle->RegisterThrottleForTesting(
scoped_ptr<NavigationThrottle>(
new InterceptNavigationThrottle(
Expand Down
27 changes: 15 additions & 12 deletions content/browser/frame_host/navigation_handle_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "content/browser/frame_host/navigation_handle_impl.h"

#include "content/browser/frame_host/frame_tree_node.h"
#include "content/browser/frame_host/navigator.h"
#include "content/browser/frame_host/navigator_delegate.h"
#include "content/public/browser/content_browser_client.h"
#include "content/public/common/content_client.h"
Expand All @@ -14,17 +16,14 @@ namespace content {
// static
scoped_ptr<NavigationHandleImpl> NavigationHandleImpl::Create(
const GURL& url,
bool is_main_frame,
NavigatorDelegate* delegate) {
FrameTreeNode* frame_tree_node) {
return scoped_ptr<NavigationHandleImpl>(
new NavigationHandleImpl(url, is_main_frame, delegate));
new NavigationHandleImpl(url, frame_tree_node));
}

NavigationHandleImpl::NavigationHandleImpl(const GURL& url,
const bool is_main_frame,
NavigatorDelegate* delegate)
FrameTreeNode* frame_tree_node)
: url_(url),
is_main_frame_(is_main_frame),
is_post_(false),
has_user_gesture_(false),
transition_(ui::PAGE_TRANSITION_LINK),
Expand All @@ -34,20 +33,24 @@ NavigationHandleImpl::NavigationHandleImpl(const GURL& url,
is_same_page_(false),
state_(INITIAL),
is_transferring_(false),
delegate_(delegate) {
delegate_->DidStartNavigation(this);
frame_tree_node_(frame_tree_node) {
GetDelegate()->DidStartNavigation(this);
}

NavigationHandleImpl::~NavigationHandleImpl() {
delegate_->DidFinishNavigation(this);
GetDelegate()->DidFinishNavigation(this);
}

NavigatorDelegate* NavigationHandleImpl::GetDelegate() const {
return frame_tree_node_->navigator()->GetDelegate();
}

const GURL& NavigationHandleImpl::GetURL() {
return url_;
}

bool NavigationHandleImpl::IsInMainFrame() {
return is_main_frame_;
return frame_tree_node_->IsMainFrame();
}

bool NavigationHandleImpl::IsPost() {
Expand Down Expand Up @@ -193,15 +196,15 @@ NavigationHandleImpl::WillRedirectRequest(const GURL& new_url,

void NavigationHandleImpl::DidRedirectNavigation(const GURL& new_url) {
url_ = new_url;
delegate_->DidRedirectNavigation(this);
GetDelegate()->DidRedirectNavigation(this);
}

void NavigationHandleImpl::ReadyToCommitNavigation(
RenderFrameHostImpl* render_frame_host) {
CHECK(!render_frame_host_);
render_frame_host_ = render_frame_host;
state_ = READY_TO_COMMIT;
delegate_->ReadyToCommitNavigation(this);
GetDelegate()->ReadyToCommitNavigation(this);
}

void NavigationHandleImpl::DidCommitNavigation(
Expand Down
17 changes: 7 additions & 10 deletions content/browser/frame_host/navigation_handle_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ struct NavigationRequestInfo;
// the RenderFrameHost still apply.
class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
public:
static scoped_ptr<NavigationHandleImpl> Create(const GURL& url,
bool is_main_frame,
NavigatorDelegate* delegate);
static scoped_ptr<NavigationHandleImpl> Create(
const GURL& url,
FrameTreeNode* frame_tree_node);
~NavigationHandleImpl() override;

// NavigationHandle implementation:
Expand Down Expand Up @@ -87,7 +87,7 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
const GURL& new_referrer_url,
bool new_is_external_protocol) override;

NavigatorDelegate* delegate() const { return delegate_; }
NavigatorDelegate* GetDelegate() const;

void set_net_error_code(net::Error net_error_code) {
net_error_code_ = net_error_code;
Expand Down Expand Up @@ -142,12 +142,10 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
};

NavigationHandleImpl(const GURL& url,
const bool is_main_frame,
NavigatorDelegate* delegate);
FrameTreeNode* frame_tree_node);

// 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_;
Expand All @@ -164,9 +162,8 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
// 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_;
// The FrameTreeNode this navigation is happening in.
FrameTreeNode* frame_tree_node_;

// A list of Throttles registered for this navigation.
ScopedVector<NavigationThrottle> throttles_;
Expand Down
4 changes: 2 additions & 2 deletions content/browser/frame_host/navigation_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ bool NavigationRequest::BeginNavigation() {
// DidStartProvisionalLoadForFrame for the navigation.
}

void NavigationRequest::CreateNavigationHandle(NavigatorDelegate* delegate) {
void NavigationRequest::CreateNavigationHandle() {
navigation_handle_ = NavigationHandleImpl::Create(
common_params_.url, frame_tree_node_->IsMainFrame(), delegate);
common_params_.url, frame_tree_node_);
}

void NavigationRequest::TransferNavigationHandleOwnership(
Expand Down
2 changes: 1 addition & 1 deletion content/browser/frame_host/navigation_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate {

// Creates a NavigationHandle. This should be called after any previous
// NavigationRequest for the FrameTreeNode has been destroyed.
void CreateNavigationHandle(NavigatorDelegate* delegate);
void CreateNavigationHandle();

// Transfers the ownership of the NavigationHandle to |render_frame_host|.
// This should be called when the navigation is ready to commit, because the
Expand Down
8 changes: 4 additions & 4 deletions content/browser/frame_host/navigator_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ void NavigatorImpl::DidStartProvisionalLoad(
render_frame_host->SetNavigationHandle(scoped_ptr<NavigationHandleImpl>());
}

render_frame_host->SetNavigationHandle(
NavigationHandleImpl::Create(validated_url, is_main_frame, delegate_));
render_frame_host->SetNavigationHandle(NavigationHandleImpl::Create(
validated_url, render_frame_host->frame_tree_node()));
}

void NavigatorImpl::DidFailProvisionalLoadWithError(
Expand Down Expand Up @@ -686,7 +686,7 @@ void NavigatorImpl::OnBeginNavigation(
controller_->GetLastCommittedEntryIndex(),
controller_->GetEntryCount()));
NavigationRequest* navigation_request = frame_tree_node->navigation_request();
navigation_request->CreateNavigationHandle(delegate_);
navigation_request->CreateNavigationHandle();

if (frame_tree_node->IsMainFrame()) {
// Renderer-initiated main-frame navigations that need to swap processes
Expand Down Expand Up @@ -855,7 +855,7 @@ void NavigatorImpl::RequestNavigation(
navigation_type, is_same_document_history_load, navigation_start,
controller_));
NavigationRequest* navigation_request = frame_tree_node->navigation_request();
navigation_request->CreateNavigationHandle(delegate_);
navigation_request->CreateNavigationHandle();

// Have the current renderer execute its beforeunload event if needed. If it
// is not needed (when beforeunload dispatch is not needed or this navigation
Expand Down
3 changes: 1 addition & 2 deletions content/browser/frame_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -915,8 +915,7 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
// message.
if (!navigation_handle_) {
navigation_handle_ = NavigationHandleImpl::Create(
validated_params.url, frame_tree_node_->IsMainFrame(),
frame_tree_node_->navigator()->GetDelegate());
validated_params.url, frame_tree_node_);
}

accessibility_reset_count_ = 0;
Expand Down
9 changes: 5 additions & 4 deletions content/public/browser/navigation_handle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "content/browser/frame_host/navigation_handle_impl.h"
#include "content/browser/frame_host/navigator.h"
#include "content/browser/frame_host/render_frame_host_impl.h"
#include "content/browser/web_contents/web_contents_impl.h"

namespace content {
Expand All @@ -14,16 +15,16 @@ WebContents* NavigationHandle::GetWebContents() {
// The NavigationHandleImpl cannot access the WebContentsImpl as it would be
// a layering violation, hence the cast here.
return static_cast<WebContentsImpl*>(
static_cast<NavigationHandleImpl*>(this)->delegate());
static_cast<NavigationHandleImpl*>(this)->GetDelegate());
}

// static
scoped_ptr<NavigationHandle> NavigationHandle::CreateNavigationHandleForTesting(
const GURL& url,
bool is_main_frame,
WebContents* web_contents) {
RenderFrameHost* render_frame_host) {
scoped_ptr<NavigationHandleImpl> handle_impl = NavigationHandleImpl::Create(
url, is_main_frame, static_cast<WebContentsImpl*>(web_contents));
url,
static_cast<RenderFrameHostImpl*>(render_frame_host)->frame_tree_node());
return scoped_ptr<NavigationHandle>(handle_impl.Pass());
}

Expand Down
3 changes: 1 addition & 2 deletions content/public/browser/navigation_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ class CONTENT_EXPORT NavigationHandle {

static scoped_ptr<NavigationHandle> CreateNavigationHandleForTesting(
const GURL& url,
bool is_main_frame,
WebContents* web_contents);
RenderFrameHost* render_frame_host);

// Registers a NavigationThrottle for tests. The throttle can
// modify the request, pause the request or cancel the request. This will
Expand Down

0 comments on commit c0ad5d1

Please sign in to comment.