diff options
6 files changed, 13 insertions, 279 deletions
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc index 2716893..a8d32e9 100644 --- a/content/browser/frame_host/navigation_handle_impl.cc +++ b/content/browser/frame_host/navigation_handle_impl.cc @@ -147,23 +147,14 @@ bool NavigationHandleImpl::IsErrorPage() { } void NavigationHandleImpl::Resume() { - if (state_ != DEFERRING_START && state_ != DEFERRING_REDIRECT && - state_ != DEFERRING_RESPONSE) { + if (state_ != DEFERRING_START && state_ != DEFERRING_REDIRECT) return; - } NavigationThrottle::ThrottleCheckResult result = NavigationThrottle::DEFER; if (state_ == DEFERRING_START) { result = CheckWillStartRequest(); - } else if (state_ == DEFERRING_REDIRECT) { - result = CheckWillRedirectRequest(); } else { - result = CheckWillProcessResponse(); - - // If the navigation is about to proceed after processing the response, then - // it's ready to commit. - if (result == NavigationThrottle::PROCEED) - ReadyToCommitNavigation(render_frame_host_, response_headers_); + result = CheckWillRedirectRequest(); } if (result != NavigationThrottle::DEFER) @@ -288,28 +279,6 @@ void NavigationHandleImpl::WillRedirectRequest( RunCompleteCallback(result); } -void NavigationHandleImpl::WillProcessResponse( - RenderFrameHostImpl* render_frame_host, - scoped_refptr<net::HttpResponseHeaders> response_headers, - const ThrottleChecksFinishedCallback& callback) { - DCHECK(!render_frame_host_ || render_frame_host_ == render_frame_host); - render_frame_host_ = render_frame_host; - response_headers_ = response_headers; - state_ = WILL_PROCESS_RESPONSE; - complete_callback_ = callback; - - // Notify each throttle of the response. - NavigationThrottle::ThrottleCheckResult result = CheckWillProcessResponse(); - - // If the navigation is about to proceed, then it's ready to commit. - if (result == NavigationThrottle::PROCEED) - ReadyToCommitNavigation(render_frame_host, response_headers); - - // If the navigation is not deferred, run the callback. - if (result != NavigationThrottle::DEFER) - RunCompleteCallback(result); -} - void NavigationHandleImpl::DidRedirectNavigation(const GURL& new_url) { url_ = new_url; GetDelegate()->DidRedirectNavigation(this); @@ -400,46 +369,13 @@ NavigationHandleImpl::CheckWillRedirectRequest() { return NavigationThrottle::PROCEED; } -NavigationThrottle::ThrottleCheckResult -NavigationHandleImpl::CheckWillProcessResponse() { - DCHECK(state_ == WILL_PROCESS_RESPONSE || state_ == DEFERRING_RESPONSE); - DCHECK(state_ != WILL_PROCESS_RESPONSE || next_index_ == 0); - DCHECK(state_ != DEFERRING_RESPONSE || next_index_ != 0); - for (size_t i = next_index_; i < throttles_.size(); ++i) { - NavigationThrottle::ThrottleCheckResult result = - throttles_[i]->WillProcessResponse(); - switch (result) { - case NavigationThrottle::PROCEED: - continue; - - case NavigationThrottle::CANCEL: - case NavigationThrottle::CANCEL_AND_IGNORE: - state_ = CANCELING; - return result; - - case NavigationThrottle::DEFER: - state_ = DEFERRING_RESPONSE; - next_index_ = i + 1; - return result; - } - } - next_index_ = 0; - state_ = WILL_PROCESS_RESPONSE; - return NavigationThrottle::PROCEED; -} - void NavigationHandleImpl::RunCompleteCallback( NavigationThrottle::ThrottleCheckResult result) { DCHECK(result != NavigationThrottle::DEFER); + if (!complete_callback_.is_null()) + complete_callback_.Run(result); - ThrottleChecksFinishedCallback callback = complete_callback_; complete_callback_.Reset(); - - if (!callback.is_null()) - callback.Run(result); - - // No code after running the callback, as it might have resulted in our - // destruction. } } // namespace content diff --git a/content/browser/frame_host/navigation_handle_impl.h b/content/browser/frame_host/navigation_handle_impl.h index 626465f..192cdab 100644 --- a/content/browser/frame_host/navigation_handle_impl.h +++ b/content/browser/frame_host/navigation_handle_impl.h @@ -160,18 +160,6 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle { scoped_refptr<net::HttpResponseHeaders> response_headers, const ThrottleChecksFinishedCallback& callback); - // Called when the URLRequest has delivered response headers and metadata. - // |callback| will be called when all throttle checks have completed, - // allowing the caller to cancel the navigation or let it proceed. - // NavigationHandle will not call |callback| with a result of DEFER. - // If the result is PROCEED, then 'ReadyToCommitNavigation' will be called - // with |render_frame_host| and |response_headers| just before calling - // |callback|. - void WillProcessResponse( - RenderFrameHostImpl* render_frame_host, - scoped_refptr<net::HttpResponseHeaders> response_headers, - const ThrottleChecksFinishedCallback& callback); - // Returns the FrameTreeNode this navigation is happening in. FrameTreeNode* frame_tree_node() { return frame_tree_node_; } @@ -202,8 +190,6 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle { WILL_REDIRECT_REQUEST, DEFERRING_REDIRECT, CANCELING, - WILL_PROCESS_RESPONSE, - DEFERRING_RESPONSE, READY_TO_COMMIT, DID_COMMIT, DID_COMMIT_ERROR_PAGE, @@ -215,7 +201,6 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle { NavigationThrottle::ThrottleCheckResult CheckWillStartRequest(); NavigationThrottle::ThrottleCheckResult CheckWillRedirectRequest(); - NavigationThrottle::ThrottleCheckResult CheckWillProcessResponse(); // Helper function to run and reset the |complete_callback_|. This marks the // end of a round of NavigationThrottleChecks. diff --git a/content/browser/frame_host/navigation_handle_impl_unittest.cc b/content/browser/frame_host/navigation_handle_impl_unittest.cc index 950ee99..ee999cb 100644 --- a/content/browser/frame_host/navigation_handle_impl_unittest.cc +++ b/content/browser/frame_host/navigation_handle_impl_unittest.cc @@ -9,10 +9,9 @@ namespace content { -// Test version of a NavigationThrottle. It will always return the current +// Test version of a NavigationThrottle. It will always return the same // NavigationThrottle::ThrottleCheckResult |result_|, It also monitors the -// number of times WillStartRequest, WillRedirectRequest, and -// WillProcessResponse were called. +// number of times WillStartRequest and WillRedirectRequest were called. class TestNavigationThrottle : public NavigationThrottle { public: TestNavigationThrottle(NavigationHandle* handle, @@ -20,8 +19,7 @@ class TestNavigationThrottle : public NavigationThrottle { : NavigationThrottle(handle), result_(result), will_start_calls_(0), - will_redirect_calls_(0), - will_process_response_calls_(0) {} + will_redirect_calls_(0) {} ~TestNavigationThrottle() override {} @@ -35,25 +33,16 @@ class TestNavigationThrottle : public NavigationThrottle { return result_; } - NavigationThrottle::ThrottleCheckResult WillProcessResponse() override { - ++will_process_response_calls_; - return result_; - } - int will_start_calls() const { return will_start_calls_; } int will_redirect_calls() const { return will_redirect_calls_; } - int will_process_response_calls() const { - return will_process_response_calls_; - } private: // The result returned by the TestNavigationThrottle. NavigationThrottle::ThrottleCheckResult result_; - // The number of times each handler was called. + // The number of times WillStartRequest and WillRedirectRequest were called. int will_start_calls_; int will_redirect_calls_; - int will_process_response_calls_; }; class NavigationHandleImplTest : public RenderViewHostImplTestHarness { @@ -83,10 +72,6 @@ class NavigationHandleImplTest : public RenderViewHostImplTestHarness { return test_handle_->state() == NavigationHandleImpl::DEFERRING_REDIRECT; } - bool IsDeferringResponse() { - return test_handle_->state() == NavigationHandleImpl::DEFERRING_RESPONSE; - } - bool IsCanceling() { return test_handle_->state() == NavigationHandleImpl::CANCELING; } @@ -123,24 +108,6 @@ class NavigationHandleImplTest : public RenderViewHostImplTestHarness { base::Unretained(this))); } - // Helper function to call WillProcessResponse on |handle|. If this function - // returns DEFER, |callback_result_| will be set to the actual result of the - // throttle checks when they are finished. - // TODO(clamy): this should also simulate that WillStartRequest was called if - // it has not been called before. - void SimulateWillProcessResponse() { - was_callback_called_ = false; - callback_result_ = NavigationThrottle::DEFER; - - // It's safe to use base::Unretained since the NavigationHandle is owned by - // the NavigationHandleImplTest. - test_handle_->WillProcessResponse( - main_test_rfh(), - scoped_refptr<net::HttpResponseHeaders>(), - base::Bind(&NavigationHandleImplTest::UpdateThrottleCheckResult, - base::Unretained(this))); - } - // Returns the handle used in tests. NavigationHandleImpl* test_handle() const { return test_handle_.get(); } @@ -183,80 +150,46 @@ TEST_F(NavigationHandleImplTest, ResumeDeferred) { CreateTestNavigationThrottle(NavigationThrottle::DEFER); EXPECT_FALSE(IsDeferringStart()); EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_FALSE(IsDeferringResponse()); EXPECT_EQ(0, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Simulate WillStartRequest. The request should be deferred. The callback // should not have been called. SimulateWillStartRequest(); EXPECT_TRUE(IsDeferringStart()); EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_FALSE(IsDeferringResponse()); EXPECT_FALSE(was_callback_called()); EXPECT_EQ(1, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Resume the request. It should no longer be deferred and the callback // should have been called. test_handle()->Resume(); EXPECT_FALSE(IsDeferringStart()); EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_FALSE(IsDeferringResponse()); EXPECT_TRUE(was_callback_called()); EXPECT_EQ(NavigationThrottle::PROCEED, callback_result()); EXPECT_EQ(1, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Simulate WillRedirectRequest. The request should be deferred. The callback // should not have been called. SimulateWillRedirectRequest(); EXPECT_FALSE(IsDeferringStart()); EXPECT_TRUE(IsDeferringRedirect()); - EXPECT_FALSE(IsDeferringResponse()); EXPECT_FALSE(was_callback_called()); EXPECT_EQ(1, test_throttle->will_start_calls()); EXPECT_EQ(1, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Resume the request. It should no longer be deferred and the callback // should have been called. test_handle()->Resume(); EXPECT_FALSE(IsDeferringStart()); EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_FALSE(IsDeferringResponse()); EXPECT_TRUE(was_callback_called()); EXPECT_EQ(NavigationThrottle::PROCEED, callback_result()); EXPECT_EQ(1, test_throttle->will_start_calls()); EXPECT_EQ(1, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); - - // Simulate WillProcessResponse. It will be deferred. The callback should not - // have been called. - SimulateWillProcessResponse(); - EXPECT_FALSE(IsDeferringStart()); - EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_TRUE(IsDeferringResponse()); - EXPECT_FALSE(was_callback_called()); - EXPECT_EQ(1, test_throttle->will_start_calls()); - EXPECT_EQ(1, test_throttle->will_redirect_calls()); - EXPECT_EQ(1, test_throttle->will_process_response_calls()); - - // Resume the request. It should no longer be deferred and the callback should - // have been called. - test_handle()->Resume(); - EXPECT_FALSE(IsDeferringStart()); - EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_FALSE(IsDeferringResponse()); - EXPECT_TRUE(was_callback_called()); - EXPECT_EQ(NavigationThrottle::PROCEED, callback_result()); - EXPECT_EQ(1, test_throttle->will_start_calls()); - EXPECT_EQ(1, test_throttle->will_redirect_calls()); - EXPECT_EQ(1, test_throttle->will_process_response_calls()); - EXPECT_TRUE(test_handle()->GetRenderFrameHost()); } // Checks that a navigation deferred during WillStartRequest can be properly @@ -268,7 +201,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredWillStart) { EXPECT_FALSE(IsDeferringRedirect()); EXPECT_EQ(0, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Simulate WillStartRequest. The request should be deferred. The callback // should not have been called. @@ -278,7 +210,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredWillStart) { EXPECT_FALSE(was_callback_called()); EXPECT_EQ(1, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Cancel the request. The callback should have been called. test_handle()->CancelDeferredNavigation( @@ -290,7 +221,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredWillStart) { EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result()); EXPECT_EQ(1, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); } // Checks that a navigation deferred during WillRedirectRequest can be properly @@ -302,7 +232,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredWillRedirect) { EXPECT_FALSE(IsDeferringRedirect()); EXPECT_EQ(0, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Simulate WillRedirectRequest. The request should be deferred. The callback // should not have been called. @@ -312,7 +241,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredWillRedirect) { EXPECT_FALSE(was_callback_called()); EXPECT_EQ(0, test_throttle->will_start_calls()); EXPECT_EQ(1, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Cancel the request. The callback should have been called. test_handle()->CancelDeferredNavigation( @@ -324,7 +252,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredWillRedirect) { EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result()); EXPECT_EQ(0, test_throttle->will_start_calls()); EXPECT_EQ(1, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); } // Checks that a navigation deferred can be canceled and not ignored. @@ -335,7 +262,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredNoIgnore) { EXPECT_FALSE(IsDeferringRedirect()); EXPECT_EQ(0, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Simulate WillRedirectRequest. The request should be deferred. The callback // should not have been called. @@ -345,7 +271,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredNoIgnore) { EXPECT_FALSE(was_callback_called()); EXPECT_EQ(1, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); // Cancel the request. The callback should have been called with CANCEL, and // not CANCEL_AND_IGNORE. @@ -357,7 +282,6 @@ TEST_F(NavigationHandleImplTest, CancelDeferredNoIgnore) { EXPECT_EQ(NavigationThrottle::CANCEL, callback_result()); EXPECT_EQ(1, test_throttle->will_start_calls()); EXPECT_EQ(0, test_throttle->will_redirect_calls()); - EXPECT_EQ(0, test_throttle->will_process_response_calls()); } // Checks that a NavigationThrottle asking to defer followed by a @@ -371,10 +295,8 @@ TEST_F(NavigationHandleImplTest, DeferThenProceed) { EXPECT_FALSE(IsDeferringRedirect()); EXPECT_EQ(0, defer_throttle->will_start_calls()); EXPECT_EQ(0, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(0, proceed_throttle->will_start_calls()); EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); - EXPECT_EQ(0, proceed_throttle->will_process_response_calls()); // Simulate WillStartRequest. The request should be deferred. The callback // should not have been called. The second throttle should not have been @@ -385,7 +307,6 @@ TEST_F(NavigationHandleImplTest, DeferThenProceed) { EXPECT_FALSE(was_callback_called()); EXPECT_EQ(1, defer_throttle->will_start_calls()); EXPECT_EQ(0, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(0, proceed_throttle->will_start_calls()); EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); @@ -398,7 +319,6 @@ TEST_F(NavigationHandleImplTest, DeferThenProceed) { EXPECT_EQ(NavigationThrottle::PROCEED, callback_result()); EXPECT_EQ(1, defer_throttle->will_start_calls()); EXPECT_EQ(0, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(1, proceed_throttle->will_start_calls()); EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); @@ -411,7 +331,6 @@ TEST_F(NavigationHandleImplTest, DeferThenProceed) { EXPECT_FALSE(was_callback_called()); EXPECT_EQ(1, defer_throttle->will_start_calls()); EXPECT_EQ(1, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(1, proceed_throttle->will_start_calls()); EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); @@ -424,7 +343,6 @@ TEST_F(NavigationHandleImplTest, DeferThenProceed) { EXPECT_EQ(NavigationThrottle::PROCEED, callback_result()); EXPECT_EQ(1, defer_throttle->will_start_calls()); EXPECT_EQ(1, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(1, proceed_throttle->will_start_calls()); EXPECT_EQ(1, proceed_throttle->will_redirect_calls()); } @@ -440,7 +358,6 @@ TEST_F(NavigationHandleImplTest, DeferThenCancelWillStartRequest) { EXPECT_FALSE(IsDeferringRedirect()); EXPECT_EQ(0, defer_throttle->will_start_calls()); EXPECT_EQ(0, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(0, cancel_throttle->will_start_calls()); EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); @@ -453,7 +370,6 @@ TEST_F(NavigationHandleImplTest, DeferThenCancelWillStartRequest) { EXPECT_FALSE(was_callback_called()); EXPECT_EQ(1, defer_throttle->will_start_calls()); EXPECT_EQ(0, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(0, cancel_throttle->will_start_calls()); EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); @@ -467,7 +383,6 @@ TEST_F(NavigationHandleImplTest, DeferThenCancelWillStartRequest) { EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result()); EXPECT_EQ(1, defer_throttle->will_start_calls()); EXPECT_EQ(0, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(1, cancel_throttle->will_start_calls()); EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); } @@ -483,7 +398,6 @@ TEST_F(NavigationHandleImplTest, DeferThenCancelWillRedirectRequest) { EXPECT_FALSE(IsDeferringRedirect()); EXPECT_EQ(0, defer_throttle->will_start_calls()); EXPECT_EQ(0, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(0, cancel_throttle->will_start_calls()); EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); @@ -496,7 +410,6 @@ TEST_F(NavigationHandleImplTest, DeferThenCancelWillRedirectRequest) { EXPECT_FALSE(was_callback_called()); EXPECT_EQ(0, defer_throttle->will_start_calls()); EXPECT_EQ(1, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(0, cancel_throttle->will_start_calls()); EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); @@ -510,7 +423,6 @@ TEST_F(NavigationHandleImplTest, DeferThenCancelWillRedirectRequest) { EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result()); EXPECT_EQ(0, defer_throttle->will_start_calls()); EXPECT_EQ(1, defer_throttle->will_redirect_calls()); - EXPECT_EQ(0, defer_throttle->will_process_response_calls()); EXPECT_EQ(0, cancel_throttle->will_start_calls()); EXPECT_EQ(1, cancel_throttle->will_redirect_calls()); } @@ -528,7 +440,6 @@ TEST_F(NavigationHandleImplTest, CancelThenProceedWillStartRequest) { EXPECT_FALSE(IsDeferringRedirect()); EXPECT_EQ(0, cancel_throttle->will_start_calls()); EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); - EXPECT_EQ(0, cancel_throttle->will_process_response_calls()); EXPECT_EQ(0, proceed_throttle->will_start_calls()); EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); @@ -542,7 +453,6 @@ TEST_F(NavigationHandleImplTest, CancelThenProceedWillStartRequest) { EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result()); EXPECT_EQ(1, cancel_throttle->will_start_calls()); EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); - EXPECT_EQ(0, cancel_throttle->will_process_response_calls()); EXPECT_EQ(0, proceed_throttle->will_start_calls()); EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); } @@ -560,7 +470,6 @@ TEST_F(NavigationHandleImplTest, CancelThenProceedWillRedirectRequest) { EXPECT_FALSE(IsDeferringRedirect()); EXPECT_EQ(0, cancel_throttle->will_start_calls()); EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); - EXPECT_EQ(0, cancel_throttle->will_process_response_calls()); EXPECT_EQ(0, proceed_throttle->will_start_calls()); EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); @@ -574,74 +483,8 @@ TEST_F(NavigationHandleImplTest, CancelThenProceedWillRedirectRequest) { EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result()); EXPECT_EQ(0, cancel_throttle->will_start_calls()); EXPECT_EQ(1, cancel_throttle->will_redirect_calls()); - EXPECT_EQ(0, cancel_throttle->will_process_response_calls()); - EXPECT_EQ(0, proceed_throttle->will_start_calls()); - EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); -} - -// Checks that a NavigationThrottle asking to proceed followed by a -// NavigationThrottle asking to cancel behave correctly in WillProcessResponse. -// Both throttles will be called, and the request will be cancelled. -TEST_F(NavigationHandleImplTest, ProceedThenCancelWillProcessResponse) { - TestNavigationThrottle* proceed_throttle = - CreateTestNavigationThrottle(NavigationThrottle::PROCEED); - TestNavigationThrottle* cancel_throttle = - CreateTestNavigationThrottle(NavigationThrottle::CANCEL_AND_IGNORE); - EXPECT_FALSE(IsDeferringStart()); - EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_EQ(0, cancel_throttle->will_start_calls()); - EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); - EXPECT_EQ(0, cancel_throttle->will_process_response_calls()); - EXPECT_EQ(0, proceed_throttle->will_start_calls()); - EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); - EXPECT_EQ(0, proceed_throttle->will_process_response_calls()); - - // Simulate WillRedirectRequest. The request should not be deferred. The - // callback should have been called. - SimulateWillProcessResponse(); - EXPECT_FALSE(IsDeferringStart()); - EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_TRUE(was_callback_called()); - EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result()); - EXPECT_EQ(0, cancel_throttle->will_start_calls()); - EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); - EXPECT_EQ(1, cancel_throttle->will_process_response_calls()); - EXPECT_EQ(0, proceed_throttle->will_start_calls()); - EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); - EXPECT_EQ(1, proceed_throttle->will_process_response_calls()); -} - -// Checks that a NavigationThrottle asking to cancel followed by a -// NavigationThrottle asking to proceed behave correctly in WillProcessResponse. -// The navigation will be canceled directly, and the second throttle will not -// be called. -TEST_F(NavigationHandleImplTest, CancelThenProceedWillProcessResponse) { - TestNavigationThrottle* cancel_throttle = - CreateTestNavigationThrottle(NavigationThrottle::CANCEL_AND_IGNORE); - TestNavigationThrottle* proceed_throttle = - CreateTestNavigationThrottle(NavigationThrottle::PROCEED); - EXPECT_FALSE(IsDeferringStart()); - EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_EQ(0, cancel_throttle->will_start_calls()); - EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); - EXPECT_EQ(0, cancel_throttle->will_process_response_calls()); - EXPECT_EQ(0, proceed_throttle->will_start_calls()); - EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); - - // Simulate WillRedirectRequest. The request should not be deferred. The - // callback should have been called. The second throttle should not have - // been notified. - SimulateWillProcessResponse(); - EXPECT_FALSE(IsDeferringStart()); - EXPECT_FALSE(IsDeferringRedirect()); - EXPECT_TRUE(was_callback_called()); - EXPECT_EQ(NavigationThrottle::CANCEL_AND_IGNORE, callback_result()); - EXPECT_EQ(0, cancel_throttle->will_start_calls()); - EXPECT_EQ(0, cancel_throttle->will_redirect_calls()); - EXPECT_EQ(1, cancel_throttle->will_process_response_calls()); EXPECT_EQ(0, proceed_throttle->will_start_calls()); EXPECT_EQ(0, proceed_throttle->will_redirect_calls()); - EXPECT_EQ(0, proceed_throttle->will_process_response_calls()); } } // namespace content diff --git a/content/browser/loader/navigation_resource_throttle.cc b/content/browser/loader/navigation_resource_throttle.cc index 6321062..44fd0c1 100644 --- a/content/browser/loader/navigation_resource_throttle.cc +++ b/content/browser/loader/navigation_resource_throttle.cc @@ -4,13 +4,9 @@ #include "content/browser/loader/navigation_resource_throttle.h" -#include "base/bind.h" #include "base/callback.h" -#include "base/location.h" -#include "base/logging.h" #include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/render_frame_host_impl.h" -#include "content/public/browser/browser_thread.h" #include "content/public/browser/resource_context.h" #include "content/public/browser/resource_controller.h" #include "content/public/browser/resource_request_info.h" @@ -30,7 +26,7 @@ typedef base::Callback<void(NavigationThrottle::ThrottleCheckResult)> void SendCheckResultToIOThread(UIChecksPerformedCallback callback, NavigationThrottle::ThrottleCheckResult result) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK_NE(result, NavigationThrottle::DEFER); + CHECK(result != NavigationThrottle::DEFER); BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(callback, result)); } @@ -97,28 +93,21 @@ void CheckWillRedirectRequestOnUIThread( } void WillProcessResponseOnUIThread( - UIChecksPerformedCallback callback, int render_process_id, int render_frame_host_id, scoped_refptr<net::HttpResponseHeaders> headers) { DCHECK_CURRENTLY_ON(BrowserThread::UI); RenderFrameHostImpl* render_frame_host = RenderFrameHostImpl::FromID(render_process_id, render_frame_host_id); - if (!render_frame_host) { - SendCheckResultToIOThread(callback, NavigationThrottle::PROCEED); + if (!render_frame_host) return; - } NavigationHandleImpl* navigation_handle = render_frame_host->navigation_handle(); - if (!navigation_handle) { - SendCheckResultToIOThread(callback, NavigationThrottle::PROCEED); + if (!navigation_handle) return; - } - navigation_handle->WillProcessResponse( - render_frame_host, headers, - base::Bind(&SendCheckResultToIOThread, callback)); + navigation_handle->ReadyToCommitNavigation(render_frame_host, headers); } } // namespace @@ -215,15 +204,10 @@ void NavigationResourceThrottle::WillProcessResponse(bool* defer) { request_->response_headers()->raw_headers()); } - UIChecksPerformedCallback callback = - base::Bind(&NavigationResourceThrottle::OnUIChecksPerformed, - weak_ptr_factory_.GetWeakPtr()); - BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, - base::Bind(&WillProcessResponseOnUIThread, callback, render_process_id, + base::Bind(&WillProcessResponseOnUIThread, render_process_id, render_frame_id, response_headers)); - *defer = true; } const char* NavigationResourceThrottle::GetNameForLogging() const { diff --git a/content/public/browser/navigation_throttle.cc b/content/public/browser/navigation_throttle.cc index 5c795d6..ac3b3aa 100644 --- a/content/public/browser/navigation_throttle.cc +++ b/content/public/browser/navigation_throttle.cc @@ -20,9 +20,4 @@ NavigationThrottle::WillRedirectRequest() { return NavigationThrottle::PROCEED; } -NavigationThrottle::ThrottleCheckResult -NavigationThrottle::WillProcessResponse() { - return NavigationThrottle::PROCEED; -} - } // namespace content diff --git a/content/public/browser/navigation_throttle.h b/content/public/browser/navigation_throttle.h index 5876343..fff64b1 100644 --- a/content/public/browser/navigation_throttle.h +++ b/content/public/browser/navigation_throttle.h @@ -55,15 +55,6 @@ class CONTENT_EXPORT NavigationThrottle { // CANCEL_AND_IGNORE or DEFER and perform the destruction asynchronously. virtual ThrottleCheckResult WillRedirectRequest(); - // Called when a response's headers and metadata are available. - // - // The implementer is responsible for ensuring that the WebContents this - // throttle is associated with remain alive during the duration of this - // method. Failing to do so will result in use-after-free bugs. Should the - // implementer need to destroy the WebContents, it should return CANCEL, - // CANCEL_AND_IGNORE and perform the destruction asynchronously. - virtual ThrottleCheckResult WillProcessResponse(); - // The NavigationHandle that is tracking the information related to this // navigation. NavigationHandle* navigation_handle() const { return navigation_handle_; } |