diff options
32 files changed, 1067 insertions, 553 deletions
diff --git a/chrome/browser/captive_portal/captive_portal_tab_helper.cc b/chrome/browser/captive_portal/captive_portal_tab_helper.cc index 271491c..1e76e41 100644 --- a/chrome/browser/captive_portal/captive_portal_tab_helper.cc +++ b/chrome/browser/captive_portal/captive_portal_tab_helper.cc @@ -14,6 +14,7 @@ #include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_tabstrip.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "content/public/browser/navigation_handle.h" #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_source.h" @@ -34,122 +35,80 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(CaptivePortalTabHelper); CaptivePortalTabHelper::CaptivePortalTabHelper( content::WebContents* web_contents) : content::WebContentsObserver(web_contents), - // web_contents is NULL in unit tests. - profile_(web_contents ? Profile::FromBrowserContext( - web_contents->GetBrowserContext()) - : NULL), - tab_reloader_( - new CaptivePortalTabReloader( - profile_, - web_contents, - base::Bind(&CaptivePortalTabHelper::OpenLoginTabForWebContents, - web_contents, false))), - login_detector_(new CaptivePortalLoginDetector(profile_)), - pending_error_code_(net::OK), - provisional_render_view_host_(NULL) { + profile_(Profile::FromBrowserContext(web_contents->GetBrowserContext())), + navigation_handle_(nullptr), + tab_reloader_(new CaptivePortalTabReloader( + profile_, + web_contents, + base::Bind(&CaptivePortalTabHelper::OpenLoginTabForWebContents, + web_contents, + false))), + login_detector_(new CaptivePortalLoginDetector(profile_)) { registrar_.Add(this, chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, content::Source<Profile>(profile_)); - registrar_.Add(this, - content::NOTIFICATION_RESOURCE_RECEIVED_REDIRECT, - content::Source<content::WebContents>(web_contents)); } CaptivePortalTabHelper::~CaptivePortalTabHelper() { } -void CaptivePortalTabHelper::RenderViewDeleted( - content::RenderViewHost* render_view_host) { - // This can happen when a cross-process navigation is aborted, either by - // pressing stop or by starting a new cross-process navigation that can't - // re-use |provisional_render_view_host_|. May also happen on a crash. - if (render_view_host == provisional_render_view_host_) - OnLoadAborted(); -} - -void CaptivePortalTabHelper::DidStartProvisionalLoadForFrame( - content::RenderFrameHost* render_frame_host, - const GURL& validated_url, - bool is_error_page, - bool is_iframe_srcdoc) { +void CaptivePortalTabHelper::DidStartNavigation( + content::NavigationHandle* navigation_handle) { DCHECK(CalledOnValidThread()); - - // Ignore subframes. - if (render_frame_host->GetParent()) + if (!navigation_handle->IsInMainFrame()) return; - content::RenderViewHost* render_view_host = - render_frame_host->GetRenderViewHost(); - if (provisional_render_view_host_) { - // If loading an error page for a previous failure, treat this as part of - // the previous load. Link Doctor pages act like two error page loads in a - // row. The second time, provisional_render_view_host_ will be NULL. - if (is_error_page && provisional_render_view_host_ == render_view_host) - return; - // Otherwise, abort the old load. - OnLoadAborted(); + // Always track the latest navigation. If a navigation was already tracked, + // and it committed (either the navigation proper or an error page), it is + // safe to start tracking the new navigation. Otherwise simulate an abort + // before reporting the start of the new navigation. + if (navigation_handle_ && !navigation_handle_->HasCommittedDocument() && + !navigation_handle_->HasCommittedErrorPage()) { + tab_reloader_->OnAbort(); } - provisional_render_view_host_ = render_view_host; - pending_error_code_ = net::OK; - - tab_reloader_->OnLoadStart(validated_url.SchemeIsCryptographic()); + navigation_handle_ = navigation_handle; + tab_reloader_->OnLoadStart( + navigation_handle->GetURL().SchemeIsCryptographic()); } -void CaptivePortalTabHelper::DidCommitProvisionalLoadForFrame( - content::RenderFrameHost* render_frame_host, - const GURL& url, - ui::PageTransition transition_type) { +void CaptivePortalTabHelper::DidRedirectNavigation( + content::NavigationHandle* navigation_handle) { DCHECK(CalledOnValidThread()); - - // Ignore subframes. - if (render_frame_host->GetParent()) + if (navigation_handle != navigation_handle_) return; - - if (provisional_render_view_host_ == render_frame_host->GetRenderViewHost()) { - tab_reloader_->OnLoadCommitted(pending_error_code_); - } else { - // This may happen if the active RenderView commits a page before a cross - // process navigation cancels the old load. In this case, the commit of the - // old navigation will cancel the newer one. - OnLoadAborted(); - - // Send information about the new load. - tab_reloader_->OnLoadStart(url.SchemeIsCryptographic()); - tab_reloader_->OnLoadCommitted(net::OK); - } - - provisional_render_view_host_ = NULL; - pending_error_code_ = net::OK; + DCHECK(navigation_handle->IsInMainFrame()); + tab_reloader_->OnRedirect( + navigation_handle->GetURL().SchemeIsCryptographic()); } -void CaptivePortalTabHelper::DidFailProvisionalLoad( - content::RenderFrameHost* render_frame_host, - const GURL& validated_url, - int error_code, - const base::string16& error_description, - bool was_ignored_by_handler) { +void CaptivePortalTabHelper::DidCommitNavigation( + content::NavigationHandle* navigation_handle) { DCHECK(CalledOnValidThread()); - - // Ignore subframes and unexpected RenderViewHosts. - if (render_frame_host->GetParent() || - render_frame_host->GetRenderViewHost() != provisional_render_view_host_) + if (!navigation_handle->IsInMainFrame()) return; - // Aborts generally aren't followed by loading an error page, so go ahead and - // reset the state now, to prevent any captive portal checks from triggering. - if (error_code == net::ERR_ABORTED) { - OnLoadAborted(); - return; - } + if (navigation_handle_ != navigation_handle) + DidStartNavigation(navigation_handle); - pending_error_code_ = error_code; + tab_reloader_->OnLoadCommitted(navigation_handle->GetNetErrorCode()); } -void CaptivePortalTabHelper::DidStopLoading() { +void CaptivePortalTabHelper::DidFinishNavigation( + content::NavigationHandle* navigation_handle) { DCHECK(CalledOnValidThread()); + if (navigation_handle != navigation_handle_) + return; + DCHECK(navigation_handle->IsInMainFrame()); + + if (!navigation_handle->HasCommittedDocument() && + !navigation_handle->HasCommittedErrorPage()) { + tab_reloader_->OnAbort(); + } login_detector_->OnStoppedLoading(); + + navigation_handle_ = nullptr; } void CaptivePortalTabHelper::Observe( @@ -157,31 +116,13 @@ void CaptivePortalTabHelper::Observe( const content::NotificationSource& source, const content::NotificationDetails& details) { DCHECK(CalledOnValidThread()); - switch (type) { - case content::NOTIFICATION_RESOURCE_RECEIVED_REDIRECT: { - DCHECK_EQ(web_contents(), - content::Source<content::WebContents>(source).ptr()); - - const content::ResourceRedirectDetails* redirect_details = - content::Details<content::ResourceRedirectDetails>(details).ptr(); - - OnRedirect(redirect_details->origin_child_id, - redirect_details->resource_type, - redirect_details->new_url); - break; - } - case chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT: { - DCHECK_EQ(profile_, content::Source<Profile>(source).ptr()); + DCHECK_EQ(chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, type); + DCHECK_EQ(profile_, content::Source<Profile>(source).ptr()); - const CaptivePortalService::Results* results = - content::Details<CaptivePortalService::Results>(details).ptr(); + const CaptivePortalService::Results* results = + content::Details<CaptivePortalService::Results>(details).ptr(); - OnCaptivePortalResults(results->previous_result, results->result); - break; - } - default: - NOTREACHED(); - } + OnCaptivePortalResults(results->previous_result, results->result); } void CaptivePortalTabHelper::OnSSLCertError(const net::SSLInfo& ssl_info) { @@ -230,19 +171,6 @@ void CaptivePortalTabHelper::OpenLoginTabForWebContents( captive_portal_tab_helper->SetIsLoginTab(); } -void CaptivePortalTabHelper::OnRedirect(int child_id, - ResourceType resource_type, - const GURL& new_url) { - // Only main frame redirects for the provisional RenderViewHost matter. - if (resource_type != content::RESOURCE_TYPE_MAIN_FRAME || - !provisional_render_view_host_ || - provisional_render_view_host_->GetProcess()->GetID() != child_id) { - return; - } - - tab_reloader_->OnRedirect(new_url.SchemeIsCryptographic()); -} - void CaptivePortalTabHelper::OnCaptivePortalResults( CaptivePortalResult previous_result, CaptivePortalResult result) { @@ -250,15 +178,6 @@ void CaptivePortalTabHelper::OnCaptivePortalResults( login_detector_->OnCaptivePortalResults(previous_result, result); } -void CaptivePortalTabHelper::OnLoadAborted() { - // No further messages for the cancelled navigation will occur. - provisional_render_view_host_ = NULL; - // May have been aborting the load of an error page. - pending_error_code_ = net::OK; - - tab_reloader_->OnAbort(); -} - void CaptivePortalTabHelper::SetIsLoginTab() { login_detector_->SetIsLoginTab(); } diff --git a/chrome/browser/captive_portal/captive_portal_tab_helper.h b/chrome/browser/captive_portal/captive_portal_tab_helper.h index 630ce09..965c994 100644 --- a/chrome/browser/captive_portal/captive_portal_tab_helper.h +++ b/chrome/browser/captive_portal/captive_portal_tab_helper.h @@ -20,6 +20,7 @@ class GURL; class Profile; namespace content { +class NavigationHandle; class WebContents; } @@ -36,21 +37,19 @@ class CaptivePortalTabReloader; // 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. +// CaptivePortalTabReloader. It filters out non-main-frame navigations. It is +// also needed by CaptivePortalTabReloaders to inform the tab's +// CaptivePortalLoginDetector when the tab is at a captive portal's login page. // -// The TabHelper assumes that a WebContents can only have one RenderViewHost -// with a provisional load at a time, and tracks only that navigation. This -// assumption can be violated in rare cases, for example, a same-site -// navigation interrupted by a cross-process navigation started from the -// omnibox, may commit before it can be cancelled. In these cases, this class -// may pass incorrect messages to the TabReloader, which will, at worst, result -// in not opening up a login tab until a second load fails or not automatically -// reloading a tab after logging in. +// The TabHelper assumes that a WebContents can only have one main frame +// navigation at a time. This assumption can be violated in rare cases, for +// example, a same-site navigation interrupted by a cross-process navigation +// started from the omnibox, may commit before it can be cancelled. In these +// cases, this class may pass incorrect messages to the TabReloader, which +// will, at worst, result in not opening up a login tab until a second load +// fails or not automatically reloading a tab after logging in. +// TODO(clamy): See if this class can be made to handle these edge-cases +// following the refactor of navigation signaling to WebContentsObservers. // // For the design doc, see: // https://docs.google.com/document/d/1k-gP2sswzYNvryu9NcgN7q5XrsMlUdlUdoW9WRaEmfM/edit @@ -63,26 +62,14 @@ class CaptivePortalTabHelper ~CaptivePortalTabHelper() override; // content::WebContentsObserver: - void RenderViewDeleted(content::RenderViewHost* render_view_host) override; - - void DidStartProvisionalLoadForFrame( - content::RenderFrameHost* render_frame_host, - const GURL& validated_url, - bool is_error_page, - bool is_iframe_srcdoc) override; - - void DidCommitProvisionalLoadForFrame( - content::RenderFrameHost* render_frame_host, - const GURL& url, - ui::PageTransition transition_type) override; - - void DidFailProvisionalLoad(content::RenderFrameHost* render_frame_host, - const GURL& validated_url, - int error_code, - const base::string16& error_description, - bool was_ignored_by_handler) override; - - void DidStopLoading() override; + void DidStartNavigation( + content::NavigationHandle* navigation_handle) override; + void DidRedirectNavigation( + content::NavigationHandle* navigation_handle) override; + void DidCommitNavigation( + content::NavigationHandle* navigation_handle) override; + void DidFinishNavigation( + content::NavigationHandle* navigation_handle) override; // content::NotificationObserver: void Observe(int type, @@ -108,17 +95,10 @@ class CaptivePortalTabHelper explicit CaptivePortalTabHelper(content::WebContents* web_contents); // Called by Observe in response to the corresponding event. - void OnRedirect(int child_id, - content::ResourceType resource_type, - const GURL& new_url); - - // Called by Observe in response to the corresponding event. void OnCaptivePortalResults( captive_portal::CaptivePortalResult previous_result, captive_portal::CaptivePortalResult result); - void OnLoadAborted(); - // Called to indicate a tab is at, or is navigating to, the captive portal // login page. void SetIsLoginTab(); @@ -126,29 +106,19 @@ class CaptivePortalTabHelper // |this| takes ownership of |tab_reloader|. void SetTabReloaderForTest(CaptivePortalTabReloader* tab_reloader); - const content::RenderViewHost* provisional_render_view_host() const { - return provisional_render_view_host_; - } - CaptivePortalTabReloader* GetTabReloaderForTest(); Profile* profile_; + // The current main frame navigation happening for the WebContents, or + // nullptr if there is none. If there are two main frame navigations + // happening at once, it's the one that started most recently. + content::NavigationHandle* navigation_handle_; + // Neither of these will ever be NULL. scoped_ptr<CaptivePortalTabReloader> tab_reloader_; scoped_ptr<CaptivePortalLoginDetector> login_detector_; - // 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 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); diff --git a/chrome/browser/captive_portal/captive_portal_tab_helper_unittest.cc b/chrome/browser/captive_portal/captive_portal_tab_helper_unittest.cc index 88e511f..1580224 100644 --- a/chrome/browser/captive_portal/captive_portal_tab_helper_unittest.cc +++ b/chrome/browser/captive_portal/captive_portal_tab_helper_unittest.cc @@ -18,6 +18,7 @@ #include "content/public/browser/render_process_host.h" #include "content/public/browser/web_contents.h" #include "content/public/test/test_renderer_host.h" +#include "content/public/test/web_contents_tester.h" #include "net/base/net_errors.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -34,9 +35,6 @@ const char* const kHttpsUrl = "https://whatever.com/"; // is different from kHttpsUrl, but best to keep things consistent. const char* const kHttpsUrl2 = "https://cross_process.com/"; -// Error pages use a "data:" URL. Shouldn't actually matter what this is. -const char* const kErrorPageUrl = "data:blah"; - // Some navigations behave differently depending on if they're cross-process // or not. enum NavigationType { @@ -67,94 +65,75 @@ class MockCaptivePortalTabReloader : public CaptivePortalTabReloader { class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness { public: CaptivePortalTabHelperTest() - : tab_helper_(NULL), - mock_reloader_(new testing::StrictMock<MockCaptivePortalTabReloader>) { - tab_helper_.SetTabReloaderForTest(mock_reloader_); - } + : mock_reloader_(new testing::StrictMock<MockCaptivePortalTabReloader>) {} ~CaptivePortalTabHelperTest() override {} void SetUp() override { ChromeRenderViewHostTestHarness::SetUp(); - web_contents1_.reset(CreateTestWebContents()); - web_contents2_.reset(CreateTestWebContents()); - - // This will simulate the initialization of the RenderFrame in the renderer - // process. This is needed because WebContents does not initialize a - // RenderFrame on construction, and the tests expect one to exist. - content::RenderFrameHostTester::For(main_render_frame1()) - ->InitializeRenderFrameIfNeeded(); - content::RenderFrameHostTester::For(main_render_frame2()) - ->InitializeRenderFrameIfNeeded(); - } - void TearDown() override { - web_contents2_.reset(NULL); - web_contents1_.reset(NULL); - ChromeRenderViewHostTestHarness::TearDown(); + // Load kHttpUrl. This ensures that any subsequent navigation to kHttpsUrl2 + // will be properly registered as cross-process. + content::WebContentsTester* web_contents_tester = + content::WebContentsTester::For(web_contents()); + web_contents_tester->NavigateAndCommit(GURL(kHttpUrl)); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + rfh_tester->SimulateNavigationStop(); + + tab_helper_.reset(new CaptivePortalTabHelper(web_contents())); + tab_helper_->profile_ = nullptr; + tab_helper_->SetTabReloaderForTest(mock_reloader_); } // Simulates a successful load of |url|. - void SimulateSuccess(const GURL& url, - content::RenderViewHost* render_view_host) { + void SimulateSuccess(const GURL& url) { EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic())) .Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - render_view_host->GetMainFrame(), url, false, false); + content::WebContentsTester* web_contents_tester = + content::WebContentsTester::For(web_contents()); + web_contents_tester->StartNavigation(url); EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::OK)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame( - render_view_host->GetMainFrame(), - url, - ui::PAGE_TRANSITION_LINK); + content::RenderFrameHost* rfh = + pending_main_rfh() ? pending_main_rfh() : main_rfh(); + content::RenderFrameHostTester::For(rfh)->SimulateNavigationCommit(url); } // Simulates a connection timeout while requesting |url|. - void SimulateTimeout(const GURL& url, - content::RenderViewHost* render_view_host) { + void SimulateTimeout(const GURL& url) { EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic())) .Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - render_view_host->GetMainFrame(), url, false, false); + content::WebContentsTester* web_contents_tester = + content::WebContentsTester::For(web_contents()); + web_contents_tester->StartNavigation(url); + content::RenderFrameHost* rfh = + pending_main_rfh() ? pending_main_rfh() : main_rfh(); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(rfh); - tab_helper().DidFailProvisionalLoad(render_view_host->GetMainFrame(), - url, - net::ERR_TIMED_OUT, - base::string16(), - false); - - // Provisional load starts for the error page. - tab_helper().DidStartProvisionalLoadForFrame( - render_view_host->GetMainFrame(), GURL(kErrorPageUrl), true, false); + rfh_tester->SimulateNavigationError(url, net::ERR_TIMED_OUT); EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::ERR_TIMED_OUT)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame( - render_view_host->GetMainFrame(), - GURL(kErrorPageUrl), - ui::PAGE_TRANSITION_LINK); + rfh_tester->SimulateNavigationErrorPageCommit(); } // Simulates an abort while requesting |url|. void SimulateAbort(const GURL& url, - content::RenderViewHost* render_view_host, NavigationType navigation_type) { EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic())) .Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - render_view_host->GetMainFrame(), url, false, false); + content::WebContentsTester* web_contents_tester = + content::WebContentsTester::For(web_contents()); + web_contents_tester->StartNavigation(url); + DCHECK_IMPLIES(navigation_type == kSameProcess, !pending_main_rfh()); EXPECT_CALL(mock_reloader(), OnAbort()).Times(1); - if (navigation_type == kSameProcess) { - tab_helper().DidFailProvisionalLoad(render_view_host->GetMainFrame(), - url, - net::ERR_ABORTED, - base::string16(), - false); - } else { - // For interrupted provisional cross-process navigations, the - // RenderViewHost is destroyed without sending a DidFailProvisionalLoad - // notification. - tab_helper().RenderViewDeleted(render_view_host); - } + content::RenderFrameHost* rfh = + navigation_type == kSameProcess ? main_rfh() : pending_main_rfh(); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(rfh); + rfh_tester->SimulateNavigationError(url, net::ERR_ABORTED); + rfh_tester->SimulateNavigationStop(); // Make sure that above call resulted in abort, for tests that continue // after the abort. @@ -163,45 +142,28 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness { // Simulates an abort while loading an error page. void SimulateAbortTimeout(const GURL& url, - content::RenderViewHost* render_view_host, NavigationType navigation_type) { EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic())) .Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - render_view_host->GetMainFrame(), url, false, false); - - tab_helper().DidFailProvisionalLoad(render_view_host->GetMainFrame(), - url, - net::ERR_TIMED_OUT, - base::string16(), - false); - - // Start event for the error page. - tab_helper().DidStartProvisionalLoadForFrame( - render_view_host->GetMainFrame(), url, true, false); + content::WebContentsTester* web_contents_tester = + content::WebContentsTester::For(web_contents()); + web_contents_tester->StartNavigation(url); + DCHECK_IMPLIES(navigation_type == kSameProcess, !pending_main_rfh()); EXPECT_CALL(mock_reloader(), OnAbort()).Times(1); - if (navigation_type == kSameProcess) { - tab_helper().DidFailProvisionalLoad(render_view_host->GetMainFrame(), - url, - net::ERR_ABORTED, - base::string16(), - false); - } else { - // For interrupted provisional cross-process navigations, the - // RenderViewHost is destroyed without sending a DidFailProvisionalLoad - // notification. - tab_helper().RenderViewDeleted(render_view_host); - } + content::RenderFrameHost* rfh = + navigation_type == kSameProcess ? main_rfh() : pending_main_rfh(); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(rfh); + rfh_tester->SimulateNavigationError(url, net::ERR_TIMED_OUT); + rfh_tester->SimulateNavigationStop(); // Make sure that above call resulted in abort, for tests that continue // after the abort. EXPECT_CALL(mock_reloader(), OnAbort()).Times(0); } - CaptivePortalTabHelper& tab_helper() { - return tab_helper_; - } + CaptivePortalTabHelper* tab_helper() { return tab_helper_.get(); } // Simulates a captive portal redirect by calling the Observe method. void ObservePortalResult(CaptivePortalResult previous_result, @@ -215,44 +177,16 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness { EXPECT_CALL(mock_reloader(), OnCaptivePortalResults(previous_result, result)).Times(1); - tab_helper().Observe(chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, - source_profile, - details_results); - } - - // Simulates a redirect. Uses OnRedirect rather than Observe, for simplicity. - void OnRedirect(ResourceType type, const GURL& new_url, int child_id) { - tab_helper().OnRedirect(child_id, type, new_url); + tab_helper()->Observe(chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, + source_profile, details_results); } MockCaptivePortalTabReloader& mock_reloader() { return *mock_reloader_; } - void SetIsLoginTab() { - tab_helper().SetIsLoginTab(); - } - - content::RenderViewHost* render_view_host1() { - return web_contents1_->GetRenderViewHost(); - } - - content::RenderViewHost* render_view_host2() { - return web_contents2_->GetRenderViewHost(); - } - - content::RenderFrameHost* main_render_frame1() { - return web_contents1_->GetMainFrame(); - } - - content::RenderFrameHost* main_render_frame2() { - return web_contents2_->GetMainFrame(); - } + void SetIsLoginTab() { tab_helper()->SetIsLoginTab(); } private: - // Only the RenderViewHosts are used. - scoped_ptr<content::WebContents> web_contents1_; - scoped_ptr<content::WebContents> web_contents2_; - - CaptivePortalTabHelper tab_helper_; + scoped_ptr<CaptivePortalTabHelper> tab_helper_; // Owned by |tab_helper_|. testing::StrictMock<MockCaptivePortalTabReloader>* mock_reloader_; @@ -261,86 +195,70 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness { }; TEST_F(CaptivePortalTabHelperTest, HttpSuccess) { - SimulateSuccess(GURL(kHttpUrl), render_view_host1()); - tab_helper().DidStopLoading(); + SimulateSuccess(GURL(kHttpUrl)); + content::RenderFrameHostTester::For(main_rfh())->SimulateNavigationStop(); } TEST_F(CaptivePortalTabHelperTest, HttpTimeout) { - SimulateTimeout(GURL(kHttpUrl), render_view_host1()); - tab_helper().DidStopLoading(); -} - -// Same as above, but simulates what happens when the Link Doctor is enabled, -// which adds another provisional load/commit for the error page, after the -// first two. -TEST_F(CaptivePortalTabHelperTest, HttpTimeoutLinkDoctor) { - SimulateTimeout(GURL(kHttpUrl), render_view_host1()); - - EXPECT_CALL(mock_reloader(), OnLoadStart(false)).Times(1); - // Provisional load starts for the error page. - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), GURL(kErrorPageUrl), true, false); - - EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::OK)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame(main_render_frame1(), - GURL(kErrorPageUrl), - ui::PAGE_TRANSITION_LINK); - tab_helper().DidStopLoading(); + SimulateTimeout(GURL(kHttpUrl)); + content::RenderFrameHostTester::For(main_rfh())->SimulateNavigationStop(); } TEST_F(CaptivePortalTabHelperTest, HttpsSuccess) { - SimulateSuccess(GURL(kHttpsUrl), render_view_host1()); - tab_helper().DidStopLoading(); - EXPECT_FALSE(tab_helper().IsLoginTab()); + SimulateSuccess(GURL(kHttpsUrl)); + content::RenderFrameHostTester::For(main_rfh())->SimulateNavigationStop(); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } TEST_F(CaptivePortalTabHelperTest, HttpsTimeout) { - SimulateTimeout(GURL(kHttpsUrl), render_view_host1()); + SimulateTimeout(GURL(kHttpsUrl)); // Make sure no state was carried over from the timeout. - SimulateSuccess(GURL(kHttpsUrl), render_view_host1()); - EXPECT_FALSE(tab_helper().IsLoginTab()); + SimulateSuccess(GURL(kHttpsUrl)); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } TEST_F(CaptivePortalTabHelperTest, HttpsAbort) { - SimulateAbort(GURL(kHttpsUrl), render_view_host1(), kSameProcess); + SimulateAbort(GURL(kHttpsUrl), kCrossProcess); // Make sure no state was carried over from the abort. - SimulateSuccess(GURL(kHttpsUrl), render_view_host1()); - EXPECT_FALSE(tab_helper().IsLoginTab()); + SimulateSuccess(GURL(kHttpsUrl)); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } // A cross-process navigation is aborted by a same-site navigation. TEST_F(CaptivePortalTabHelperTest, AbortCrossProcess) { - SimulateAbort(GURL(kHttpsUrl2), render_view_host2(), kCrossProcess); + SimulateAbort(GURL(kHttpsUrl2), kCrossProcess); // Make sure no state was carried over from the abort. - SimulateSuccess(GURL(kHttpUrl), render_view_host1()); - EXPECT_FALSE(tab_helper().IsLoginTab()); + SimulateSuccess(GURL(kHttpUrl)); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } // Abort while there's a provisional timeout error page loading. TEST_F(CaptivePortalTabHelperTest, HttpsAbortTimeout) { - SimulateAbortTimeout(GURL(kHttpsUrl), render_view_host1(), kSameProcess); + SimulateAbortTimeout(GURL(kHttpsUrl), kCrossProcess); // Make sure no state was carried over from the timeout or the abort. - SimulateSuccess(GURL(kHttpsUrl), render_view_host1()); - EXPECT_FALSE(tab_helper().IsLoginTab()); + SimulateSuccess(GURL(kHttpsUrl)); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } // Abort a cross-process navigation while there's a provisional timeout error // page loading. TEST_F(CaptivePortalTabHelperTest, AbortTimeoutCrossProcess) { - SimulateAbortTimeout(GURL(kHttpsUrl2), render_view_host2(), - kCrossProcess); + SimulateAbortTimeout(GURL(kHttpsUrl2), kCrossProcess); // Make sure no state was carried over from the timeout or the abort. - SimulateSuccess(GURL(kHttpsUrl), render_view_host1()); - EXPECT_FALSE(tab_helper().IsLoginTab()); + SimulateSuccess(GURL(kHttpsUrl)); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } // Opposite case from above - a same-process error page is aborted in favor of // a cross-process one. TEST_F(CaptivePortalTabHelperTest, HttpsAbortTimeoutForCrossProcess) { - SimulateAbortTimeout(GURL(kHttpsUrl), render_view_host1(), kSameProcess); + SimulateSuccess(GURL(kHttpsUrl)); + content::RenderFrameHostTester::For(main_rfh())->SimulateNavigationStop(); + + SimulateAbortTimeout(GURL(kHttpsUrl), kSameProcess); // Make sure no state was carried over from the timeout or the abort. - SimulateSuccess(GURL(kHttpsUrl2), render_view_host2()); - EXPECT_FALSE(tab_helper().IsLoginTab()); + SimulateSuccess(GURL(kHttpsUrl2)); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } // A provisional same-site navigation is interrupted by a cross-process @@ -352,41 +270,29 @@ TEST_F(CaptivePortalTabHelperTest, UnexpectedProvisionalLoad) { // A same-site load for the original RenderViewHost starts. EXPECT_CALL(mock_reloader(), OnLoadStart(same_site_url.SchemeIsCryptographic())).Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), same_site_url, false, false); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + rfh_tester->SimulateNavigationStart(same_site_url); // It's unexpectedly interrupted by a cross-process navigation, which starts - // navigating before the old navigation cancels. We generate an abort message - // for the old navigation. + // navigating before the old navigation cancels. EXPECT_CALL(mock_reloader(), OnAbort()).Times(1); EXPECT_CALL(mock_reloader(), OnLoadStart(cross_process_url.SchemeIsCryptographic())).Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame2(), cross_process_url, false, false); + content::WebContentsTester::For(web_contents()) + ->StartNavigation(cross_process_url); // The cross-process navigation fails. - tab_helper().DidFailProvisionalLoad(main_render_frame2(), - cross_process_url, - net::ERR_FAILED, - base::string16(), - false); + content::RenderFrameHostTester* pending_rfh_tester = + content::RenderFrameHostTester::For(pending_main_rfh()); + pending_rfh_tester->SimulateNavigationError(cross_process_url, + net::ERR_FAILED); // The same-site navigation finally is aborted. - tab_helper().DidFailProvisionalLoad(main_render_frame1(), - same_site_url, - net::ERR_ABORTED, - base::string16(), - false); - - // The provisional load starts for the error page for the cross-process - // navigation. - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame2(), GURL(kErrorPageUrl), true, false); + rfh_tester->SimulateNavigationStop(); EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::ERR_FAILED)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame(main_render_frame2(), - GURL(kErrorPageUrl), - ui::PAGE_TRANSITION_TYPED); + pending_rfh_tester->SimulateNavigationErrorPageCommit(); } // Similar to the above test, except the original RenderViewHost manages to @@ -398,32 +304,29 @@ TEST_F(CaptivePortalTabHelperTest, UnexpectedCommit) { // A same-site load for the original RenderViewHost starts. EXPECT_CALL(mock_reloader(), OnLoadStart(same_site_url.SchemeIsCryptographic())).Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), same_site_url, false, false); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + rfh_tester->SimulateNavigationStart(same_site_url); // It's unexpectedly interrupted by a cross-process navigation, which starts - // navigating before the old navigation cancels. We generate an abort message - // for the old navigation. + // navigating before the old navigation cancels. EXPECT_CALL(mock_reloader(), OnAbort()).Times(1); EXPECT_CALL(mock_reloader(), OnLoadStart(cross_process_url.SchemeIsCryptographic())).Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame2(), cross_process_url, false, false); + content::WebContentsTester::For(web_contents()) + ->StartNavigation(cross_process_url); // The cross-process navigation fails. - tab_helper().DidFailProvisionalLoad(main_render_frame2(), - cross_process_url, - net::ERR_FAILED, - base::string16(), - false); + content::RenderFrameHostTester::For(pending_main_rfh()) + ->SimulateNavigationError(cross_process_url, net::ERR_FAILED); // The same-site navigation succeeds. EXPECT_CALL(mock_reloader(), OnAbort()).Times(1); EXPECT_CALL(mock_reloader(), - OnLoadStart(same_site_url.SchemeIsCryptographic())).Times(1); + OnLoadStart(same_site_url.SchemeIsCryptographic())) + .Times(1); EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::OK)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame( - main_render_frame1(), same_site_url, ui::PAGE_TRANSITION_LINK); + rfh_tester->SimulateNavigationCommit(same_site_url); } // Simulates navigations for a number of subframes, and makes sure no @@ -431,32 +334,32 @@ TEST_F(CaptivePortalTabHelperTest, UnexpectedCommit) { TEST_F(CaptivePortalTabHelperTest, HttpsSubframe) { GURL url = GURL(kHttpsUrl); - content::RenderFrameHostTester* render_frame_host_tester = - content::RenderFrameHostTester::For(main_render_frame1()); - content::RenderFrameHost* subframe1 = - render_frame_host_tester->AppendChild("subframe1"); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + content::RenderFrameHost* subframe1 = rfh_tester->AppendChild("subframe1"); // Normal load. - tab_helper().DidStartProvisionalLoadForFrame(subframe1, url, false, false); - tab_helper().DidCommitProvisionalLoadForFrame( - subframe1, url, ui::PAGE_TRANSITION_LINK); + content::RenderFrameHostTester* subframe_tester1 = + content::RenderFrameHostTester::For(subframe1); + subframe_tester1->SimulateNavigationStart(url); + subframe_tester1->SimulateNavigationCommit(url); + subframe_tester1->SimulateNavigationStop(); // Timeout. - content::RenderFrameHost* subframe2 = - render_frame_host_tester->AppendChild("subframe2"); - tab_helper().DidStartProvisionalLoadForFrame(subframe2, url, false, false); - tab_helper().DidFailProvisionalLoad( - subframe2, url, net::ERR_TIMED_OUT, base::string16(), false); - tab_helper().DidStartProvisionalLoadForFrame(subframe2, url, true, false); - tab_helper().DidFailProvisionalLoad( - subframe2, url, net::ERR_ABORTED, base::string16(), false); + content::RenderFrameHost* subframe2 = rfh_tester->AppendChild("subframe2"); + content::RenderFrameHostTester* subframe_tester2 = + content::RenderFrameHostTester::For(subframe2); + subframe_tester2->SimulateNavigationStart(url); + subframe_tester2->SimulateNavigationError(url, net::ERR_TIMED_OUT); + subframe_tester2->SimulateNavigationStop(); // Abort. - content::RenderFrameHost* subframe3 = - render_frame_host_tester->AppendChild("subframe3"); - tab_helper().DidStartProvisionalLoadForFrame(subframe3, url, false, false); - tab_helper().DidFailProvisionalLoad( - subframe3, url, net::ERR_ABORTED, base::string16(), false); + content::RenderFrameHost* subframe3 = rfh_tester->AppendChild("subframe3"); + content::RenderFrameHostTester* subframe_tester3 = + content::RenderFrameHostTester::For(subframe3); + subframe_tester3->SimulateNavigationStart(url); + subframe_tester3->SimulateNavigationError(url, net::ERR_ABORTED); + subframe_tester3->SimulateNavigationStop(); } // Simulates a subframe erroring out at the same time as a provisional load, @@ -465,63 +368,44 @@ TEST_F(CaptivePortalTabHelperTest, HttpsSubframe) { TEST_F(CaptivePortalTabHelperTest, HttpsSubframeParallelError) { // URL used by both frames. GURL url = GURL(kHttpsUrl); - content::RenderFrameHost* subframe = - content::RenderFrameHostTester::For(main_render_frame1()) - ->AppendChild("subframe"); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + content::RenderFrameHost* subframe = rfh_tester->AppendChild("subframe"); + content::RenderFrameHostTester* subframe_tester = + content::RenderFrameHostTester::For(subframe); // Loads start. EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic())) .Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), url, false, false); - tab_helper().DidStartProvisionalLoadForFrame(subframe, url, false, false); + rfh_tester->SimulateNavigationStart(url); + subframe_tester->SimulateNavigationStart(url); // Loads return errors. - tab_helper().DidFailProvisionalLoad( - main_render_frame1(), url, net::ERR_UNEXPECTED, base::string16(), false); - tab_helper().DidFailProvisionalLoad( - subframe, url, net::ERR_TIMED_OUT, base::string16(), false); - - // Provisional load starts for the error pages. - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), url, true, false); - tab_helper().DidStartProvisionalLoadForFrame(subframe, url, true, false); + rfh_tester->SimulateNavigationError(url, net::ERR_UNEXPECTED); + subframe_tester->SimulateNavigationError(url, net::ERR_TIMED_OUT); // Error page load finishes. - tab_helper().DidCommitProvisionalLoadForFrame( - subframe, url, ui::PAGE_TRANSITION_AUTO_SUBFRAME); + subframe_tester->SimulateNavigationErrorPageCommit(); EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::ERR_UNEXPECTED)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame( - main_render_frame1(), url, ui::PAGE_TRANSITION_LINK); + rfh_tester->SimulateNavigationErrorPageCommit(); } // Simulates an HTTP to HTTPS redirect, which then times out. TEST_F(CaptivePortalTabHelperTest, HttpToHttpsRedirectTimeout) { GURL http_url(kHttpUrl); EXPECT_CALL(mock_reloader(), OnLoadStart(false)).Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), http_url, false, false); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + rfh_tester->SimulateNavigationStart(http_url); GURL https_url(kHttpsUrl); EXPECT_CALL(mock_reloader(), OnRedirect(true)).Times(1); - OnRedirect(content::RESOURCE_TYPE_MAIN_FRAME, - https_url, - render_view_host1()->GetProcess()->GetID()); + rfh_tester->SimulateRedirect(https_url); - tab_helper().DidFailProvisionalLoad(main_render_frame1(), - https_url, - net::ERR_TIMED_OUT, - base::string16(), - false); - - // Provisional load starts for the error page. - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), GURL(kErrorPageUrl), true, false); + rfh_tester->SimulateNavigationError(https_url, net::ERR_TIMED_OUT); EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::ERR_TIMED_OUT)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame(main_render_frame1(), - GURL(kErrorPageUrl), - ui::PAGE_TRANSITION_LINK); + rfh_tester->SimulateNavigationErrorPageCommit(); } // Simulates an HTTPS to HTTP redirect. @@ -529,138 +413,109 @@ TEST_F(CaptivePortalTabHelperTest, HttpsToHttpRedirect) { GURL https_url(kHttpsUrl); EXPECT_CALL(mock_reloader(), OnLoadStart(https_url.SchemeIsCryptographic())) .Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), https_url, false, false); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + rfh_tester->SimulateNavigationStart(https_url); GURL http_url(kHttpUrl); EXPECT_CALL(mock_reloader(), OnRedirect(http_url.SchemeIsCryptographic())) .Times(1); - OnRedirect(content::RESOURCE_TYPE_MAIN_FRAME, http_url, - render_view_host1()->GetProcess()->GetID()); + rfh_tester->SimulateRedirect(http_url); EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::OK)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame( - main_render_frame1(), http_url, ui::PAGE_TRANSITION_LINK); + rfh_tester->SimulateNavigationCommit(http_url); } -// Simulates an HTTPS to HTTPS redirect. +// Simulates an HTTP to HTTP redirect. TEST_F(CaptivePortalTabHelperTest, HttpToHttpRedirect) { GURL http_url(kHttpUrl); EXPECT_CALL(mock_reloader(), OnLoadStart(http_url.SchemeIsCryptographic())) .Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), http_url, false, false); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + rfh_tester->SimulateNavigationStart(http_url); EXPECT_CALL(mock_reloader(), OnRedirect(http_url.SchemeIsCryptographic())) .Times(1); - OnRedirect(content::RESOURCE_TYPE_MAIN_FRAME, http_url, - render_view_host1()->GetProcess()->GetID()); + rfh_tester->SimulateRedirect(http_url); EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::OK)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame( - main_render_frame1(), http_url, ui::PAGE_TRANSITION_LINK); + rfh_tester->SimulateNavigationCommit(http_url); } // Tests that a subframe redirect doesn't reset the timer to kick off a captive // portal probe for the main frame if the main frame request is taking too long. TEST_F(CaptivePortalTabHelperTest, SubframeRedirect) { GURL http_url(kHttpUrl); - EXPECT_CALL(mock_reloader(), OnLoadStart(false)).Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), http_url, false, false); + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + content::RenderFrameHost* subframe = rfh_tester->AppendChild("subframe"); + content::RenderFrameHostTester* subframe_tester = + content::RenderFrameHostTester::For(subframe); - GURL https_url(kHttpsUrl); - OnRedirect(content::RESOURCE_TYPE_SUB_FRAME, - https_url, - render_view_host1()->GetProcess()->GetID()); - - EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::OK)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame( - main_render_frame1(), GURL(kErrorPageUrl), ui::PAGE_TRANSITION_LINK); -} - -// Simulates a redirect, for another RenderViewHost. -TEST_F(CaptivePortalTabHelperTest, OtherRenderViewHostRedirect) { - GURL http_url(kHttpUrl); EXPECT_CALL(mock_reloader(), OnLoadStart(false)).Times(1); - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), http_url, false, false); + rfh_tester->SimulateNavigationStart(http_url); + subframe_tester->SimulateNavigationStart(http_url); - // Another RenderViewHost sees a redirect. None of the reloader's functions - // should be called. GURL https_url(kHttpsUrl); - OnRedirect(content::RESOURCE_TYPE_MAIN_FRAME, - https_url, - render_view_host2()->GetProcess()->GetID()); - - tab_helper().DidFailProvisionalLoad(main_render_frame1(), - https_url, - net::ERR_TIMED_OUT, - base::string16(), - false); - - // Provisional load starts for the error page. - tab_helper().DidStartProvisionalLoadForFrame( - main_render_frame1(), GURL(kErrorPageUrl), true, false); + subframe_tester->SimulateRedirect(https_url); - EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::ERR_TIMED_OUT)).Times(1); - tab_helper().DidCommitProvisionalLoadForFrame(main_render_frame1(), - GURL(kErrorPageUrl), - ui::PAGE_TRANSITION_LINK); + EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::OK)).Times(1); + rfh_tester->SimulateNavigationCommit(http_url); } TEST_F(CaptivePortalTabHelperTest, LoginTabLogin) { - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); SetIsLoginTab(); - EXPECT_TRUE(tab_helper().IsLoginTab()); + EXPECT_TRUE(tab_helper()->IsLoginTab()); ObservePortalResult(captive_portal::RESULT_INTERNET_CONNECTED, captive_portal::RESULT_INTERNET_CONNECTED); - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } TEST_F(CaptivePortalTabHelperTest, LoginTabError) { - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); SetIsLoginTab(); - EXPECT_TRUE(tab_helper().IsLoginTab()); + EXPECT_TRUE(tab_helper()->IsLoginTab()); ObservePortalResult(captive_portal::RESULT_INTERNET_CONNECTED, captive_portal::RESULT_NO_RESPONSE); - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } TEST_F(CaptivePortalTabHelperTest, LoginTabMultipleResultsBeforeLogin) { - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); SetIsLoginTab(); - EXPECT_TRUE(tab_helper().IsLoginTab()); + EXPECT_TRUE(tab_helper()->IsLoginTab()); ObservePortalResult(captive_portal::RESULT_INTERNET_CONNECTED, captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL); - EXPECT_TRUE(tab_helper().IsLoginTab()); + EXPECT_TRUE(tab_helper()->IsLoginTab()); ObservePortalResult(captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL, captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL); - EXPECT_TRUE(tab_helper().IsLoginTab()); + EXPECT_TRUE(tab_helper()->IsLoginTab()); ObservePortalResult(captive_portal::RESULT_NO_RESPONSE, captive_portal::RESULT_INTERNET_CONNECTED); - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } TEST_F(CaptivePortalTabHelperTest, NoLoginTab) { - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); ObservePortalResult(captive_portal::RESULT_INTERNET_CONNECTED, captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL); - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); ObservePortalResult(captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL, captive_portal::RESULT_NO_RESPONSE); - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); ObservePortalResult(captive_portal::RESULT_NO_RESPONSE, captive_portal::RESULT_INTERNET_CONNECTED); - EXPECT_FALSE(tab_helper().IsLoginTab()); + EXPECT_FALSE(tab_helper()->IsLoginTab()); } diff --git a/content/browser/devtools/devtools_manager_unittest.cc b/content/browser/devtools/devtools_manager_unittest.cc index 9ace6fa..e2870c8 100644 --- a/content/browser/devtools/devtools_manager_unittest.cc +++ b/content/browser/devtools/devtools_manager_unittest.cc @@ -172,6 +172,7 @@ TEST_F(DevToolsManagerTest, ReattachOnCancelPendingNavigation) { contents()->GetMainFrame()->PrepareForCommit(); contents()->TestDidNavigate(contents()->GetMainFrame(), 1, pending_id, true, url, ui::PAGE_TRANSITION_TYPED); + contents()->GetMainFrame()->SimulateNavigationStop(); EXPECT_FALSE(contents()->CrossProcessNavigationPending()); TestDevToolsClientHost client_host; diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc index 0023835..058b508 100644 --- a/content/browser/frame_host/navigation_controller_impl_unittest.cc +++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc @@ -552,9 +552,13 @@ void CheckNavigationEntryMatchLoadParams( } TEST_F(NavigationControllerTest, LoadURLWithParams) { + // Start a navigation in order to have enough state to fake a transfer. + contents()->NavigateAndCommit(GURL("http://foo")); + contents()->StartNavigation(GURL("http://bar")); + NavigationControllerImpl& controller = controller_impl(); - NavigationController::LoadURLParams load_params(GURL("http://foo")); + NavigationController::LoadURLParams load_params(GURL("http://foo/2")); load_params.referrer = Referrer(GURL("http://referrer"), blink::WebReferrerPolicyDefault); load_params.transition_type = ui::PAGE_TRANSITION_GENERATED; @@ -1104,21 +1108,19 @@ TEST_F(NavigationControllerTest, LoadURL_RedirectAbortDoesntShowPendingURL) { // Now make a pending new navigation, initiated by the renderer. const GURL kNewURL("http://foo/bee"); - NavigationController::LoadURLParams load_url_params(kNewURL); - load_url_params.transition_type = ui::PAGE_TRANSITION_TYPED; - load_url_params.is_renderer_initiated = true; - controller.LoadURLWithParams(load_url_params); + main_test_rfh()->SimulateNavigationStart(kNewURL); EXPECT_EQ(0U, notifications.size()); EXPECT_EQ(-1, controller.GetPendingEntryIndex()); EXPECT_TRUE(controller.GetPendingEntry()); EXPECT_EQ(0, controller.GetLastCommittedEntryIndex()); - EXPECT_EQ(0, delegate->navigation_state_change_count()); + EXPECT_EQ(1, delegate->navigation_state_change_count()); // The visible entry should be the last committed URL, not the pending one. EXPECT_EQ(kExistingURL, controller.GetVisibleEntry()->GetURL()); - // Now the navigation redirects. (There is no corresponding message here.) + // Now the navigation redirects. const GURL kRedirectURL("http://foo/see"); + main_test_rfh()->SimulateRedirect(kRedirectURL); // We don't want to change the NavigationEntry's url, in case it cancels. // Prevents regression of http://crbug.com/77786. @@ -1126,21 +1128,14 @@ TEST_F(NavigationControllerTest, LoadURL_RedirectAbortDoesntShowPendingURL) { // It may abort before committing, if it's a download or due to a stop or // a new navigation from the user. - FrameHostMsg_DidFailProvisionalLoadWithError_Params params; - params.error_code = net::ERR_ABORTED; - params.error_description = base::string16(); - params.url = kRedirectURL; - params.showing_repost_interstitial = false; - main_test_rfh()->OnMessageReceived( - FrameHostMsg_DidFailProvisionalLoadWithError(0, // routing_id - params)); + main_test_rfh()->SimulateNavigationError(kRedirectURL, net::ERR_ABORTED); // Because the pending entry is renderer initiated and not visible, we // clear it when it fails. EXPECT_EQ(-1, controller.GetPendingEntryIndex()); EXPECT_FALSE(controller.GetPendingEntry()); EXPECT_EQ(0, controller.GetLastCommittedEntryIndex()); - EXPECT_EQ(1, delegate->navigation_state_change_count()); + EXPECT_EQ(2, delegate->navigation_state_change_count()); // The visible entry should be the last committed URL, not the pending one, // so that no spoof is possible. diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc new file mode 100644 index 0000000..f249990 --- /dev/null +++ b/content/browser/frame_host/navigation_handle_impl.cc @@ -0,0 +1,67 @@ +// 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. + +#include "content/browser/frame_host/navigation_handle_impl.h" + +#include "content/browser/frame_host/navigator_delegate.h" +#include "net/url_request/redirect_info.h" + +namespace content { + +// static +scoped_ptr<NavigationHandleImpl> NavigationHandleImpl::Create( + const GURL& url, + const bool is_main_frame, + NavigatorDelegate* delegate) { + return scoped_ptr<NavigationHandleImpl>( + new NavigationHandleImpl(url, is_main_frame, delegate)); +} + +NavigationHandleImpl::NavigationHandleImpl(const GURL& url, + const bool is_main_frame, + NavigatorDelegate* delegate) + : url_(url), + net_error_code_(net::OK), + state_(DID_START), + is_main_frame_(is_main_frame), + is_transferring_(false), + delegate_(delegate) { + delegate_->DidStartNavigation(this); +} + +NavigationHandleImpl::~NavigationHandleImpl() { + delegate_->DidFinishNavigation(this); +} + +const GURL& NavigationHandleImpl::GetURL() const { + return url_; +} + +net::Error NavigationHandleImpl::GetNetErrorCode() const { + return net_error_code_; +} + +bool NavigationHandleImpl::IsInMainFrame() const { + return is_main_frame_; +} + +bool NavigationHandleImpl::HasCommittedDocument() const { + return state_ == DID_COMMIT; +} + +bool NavigationHandleImpl::HasCommittedErrorPage() const { + return state_ == DID_COMMIT_ERROR_PAGE; +} + +void NavigationHandleImpl::DidRedirectNavigation(const GURL& new_url) { + url_ = new_url; + delegate_->DidRedirectNavigation(this); +} + +void NavigationHandleImpl::DidCommitNavigation() { + state_ = net_error_code_ == net::OK ? DID_COMMIT : DID_COMMIT_ERROR_PAGE; + delegate_->DidCommitNavigation(this); +} + +} // namespace content diff --git a/content/browser/frame_host/navigation_handle_impl.h b/content/browser/frame_host/navigation_handle_impl.h new file mode 100644 index 0000000..33e51dd --- /dev/null +++ b/content/browser/frame_host/navigation_handle_impl.h @@ -0,0 +1,120 @@ +// 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 "content/common/content_export.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. +// The NavigationHandleImpl in the RenderFrameHost will be reset when the +// navigation stops, that is if one of the following events happen: +// - The RenderFrameHost receives a DidStartProvisionalLoad IPC for a new +// navigation (see below for special cases where the DidStartProvisionalLoad +// message does not indicate the start of a new navigation). +// - The RenderFrameHost stops loading. +// - The RenderFrameHost receives a DidDropNavigation IPC. +// +// When the navigation encounters an error, the DidStartProvisionalLoad marking +// the start of the load of the error page will not be considered as marking a +// new navigation. It will not reset the NavigationHandleImpl in the +// RenderFrameHost. +// +// If the navigation needs a cross-site transfer, then the NavigationHandleImpl +// will briefly be held by the RenderFrameHostManager, until a suitable +// RenderFrameHost for the navigation has been found. The ownership of the +// NavigationHandleImpl will then be transferred to the new RenderFrameHost. +// The DidStartProvisionalLoad received by the new RenderFrameHost for the +// transferring navigation will not reset the NavigationHandleImpl, as it does +// not mark the start of a new navigation. +// +// PlzNavigate: the NavigationHandleImpl is created just after creating a new +// 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; + + 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; + } + + // 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: + // Used to track the state the navigation is currently in. + enum State { + DID_START = 0, + 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_; + net::Error net_error_code_; + 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_; + + DISALLOW_COPY_AND_ASSIGN(NavigationHandleImpl); +}; + +} // namespace content + +#endif // CONTENT_BROWSER_FRAME_HOST_NAVIGATION_HANDLE_IMPL_H_ diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc index 6844c43..a99074f 100644 --- a/content/browser/frame_host/navigation_request.cc +++ b/content/browser/frame_host/navigation_request.cc @@ -7,6 +7,7 @@ #include "content/browser/frame_host/frame_tree.h" #include "content/browser/frame_host/frame_tree_node.h" #include "content/browser/frame_host/navigation_controller_impl.h" +#include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/navigation_request_info.h" #include "content/browser/frame_host/navigator.h" #include "content/browser/loader/navigation_url_loader.h" @@ -195,6 +196,16 @@ bool NavigationRequest::BeginNavigation() { // DidStartProvisionalLoadForFrame for the navigation. } +void NavigationRequest::CreateNavigationHandle(NavigatorDelegate* delegate) { + navigation_handle_ = NavigationHandleImpl::Create( + common_params_.url, frame_tree_node_->IsMainFrame(), delegate); +} + +void NavigationRequest::TransferNavigationHandleOwnership( + RenderFrameHostImpl* render_frame_host) { + render_frame_host->SetNavigationHandle(navigation_handle_.Pass()); +} + void NavigationRequest::OnRequestRedirected( const net::RedirectInfo& redirect_info, const scoped_refptr<ResourceResponse>& response) { @@ -205,6 +216,8 @@ void NavigationRequest::OnRequestRedirected( // TODO(davidben): This where prerender and navigation_interceptor should be // integrated. For now, just always follow all redirects. loader_->FollowRedirect(); + + navigation_handle_->DidRedirectNavigation(redirect_info.new_url); } void NavigationRequest::OnResponseStarted( @@ -220,6 +233,7 @@ void NavigationRequest::OnRequestFailed(bool has_stale_copy_in_cache, int net_error) { DCHECK(state_ == STARTED); state_ = FAILED; + navigation_handle_->set_net_error_code(static_cast<net::Error>(net_error)); frame_tree_node_->navigator()->FailedNavigation( frame_tree_node_, has_stale_copy_in_cache, net_error); } diff --git a/content/browser/frame_host/navigation_request.h b/content/browser/frame_host/navigation_request.h index 5d2aa03..a1530b4 100644 --- a/content/browser/frame_host/navigation_request.h +++ b/content/browser/frame_host/navigation_request.h @@ -19,7 +19,9 @@ namespace content { class FrameNavigationEntry; class FrameTreeNode; class NavigationControllerImpl; +class NavigationHandleImpl; class NavigationURLLoader; +class NavigatorDelegate; class ResourceRequestBody; class SiteInstanceImpl; struct NavigationRequestInfo; @@ -120,6 +122,22 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate { state_ = WAITING_FOR_RENDERER_RESPONSE; } + NavigationHandleImpl* navigation_handle() const { + return navigation_handle_.get(); + } + + // Creates a NavigationHandle. This should be called after any previous + // NavigationRequest for the FrameTreeNode has been destroyed. + void CreateNavigationHandle(NavigatorDelegate* delegate); + + // Transfers the ownership of the NavigationHandle to |render_frame_host|. + // This should be called when the navigation is ready to commit, because the + // NavigationHandle outlives the NavigationRequest. The NavigationHandle's + // lifetime is the entire navigation, while the NavigationRequest is + // destroyed when a navigation is ready for commit. + void TransferNavigationHandleOwnership( + RenderFrameHostImpl* render_frame_host); + private: NavigationRequest(FrameTreeNode* frame_tree_node, const CommonNavigationParams& common_params, @@ -169,6 +187,8 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate { bool is_view_source_; int bindings_; + scoped_ptr<NavigationHandleImpl> navigation_handle_; + DISALLOW_COPY_AND_ASSIGN(NavigationRequest); }; diff --git a/content/browser/frame_host/navigator_delegate.h b/content/browser/frame_host/navigator_delegate.h index 7779edb..ac8402c9 100644 --- a/content/browser/frame_host/navigator_delegate.h +++ b/content/browser/frame_host/navigator_delegate.h @@ -18,6 +18,7 @@ struct FrameHostMsg_DidFailProvisionalLoadWithError_Params; namespace content { class FrameTreeNode; +class NavigationHandle; class RenderFrameHostImpl; struct LoadCommittedDetails; struct OpenURLParams; @@ -26,6 +27,23 @@ struct OpenURLParams; // related events. class CONTENT_EXPORT NavigatorDelegate { public: + // Called when a navigation started. The same NavigationHandle will be + // provided for events related to the same navigation. + virtual void DidStartNavigation(NavigationHandle* navigation_handle) {} + + // Called when a navigation was redirected. + virtual void DidRedirectNavigation(NavigationHandle* navigation_handle) {} + + // Called when a navigation committed. + virtual void DidCommitNavigation(NavigationHandle* navigation_handle) {} + + // Called when a document load resulting from the navigation stopped. Note + // that |navigation_handle| will be destroyed at the end of this call. + virtual void DidFinishNavigation(NavigationHandle* navigation_handle) {} + + // TODO(clamy): all methods below that are related to navigation + // events should go away in favor of the ones above. + // The RenderFrameHost started a provisional load for the frame // represented by |render_frame_host|. virtual void DidStartProvisionalLoad( diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc index 878c9b1..b899825 100644 --- a/content/browser/frame_host/navigator_impl.cc +++ b/content/browser/frame_host/navigator_impl.cc @@ -11,6 +11,7 @@ #include "content/browser/frame_host/frame_tree_node.h" #include "content/browser/frame_host/navigation_controller_impl.h" #include "content/browser/frame_host/navigation_entry_impl.h" +#include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/navigation_request.h" #include "content/browser/frame_host/navigation_request_info.h" #include "content/browser/frame_host/navigator_delegate.h" @@ -118,24 +119,48 @@ NavigationController* NavigatorImpl::GetController() { void NavigatorImpl::DidStartProvisionalLoad( RenderFrameHostImpl* render_frame_host, const GURL& url) { + bool is_main_frame = render_frame_host->frame_tree_node()->IsMainFrame(); bool is_error_page = (url.spec() == kUnreachableWebDataURL); bool is_iframe_srcdoc = (url.spec() == kAboutSrcDocURL); GURL validated_url(url); RenderProcessHost* render_process_host = render_frame_host->GetProcess(); render_process_host->FilterURL(false, &validated_url); - bool is_main_frame = render_frame_host->frame_tree_node()->IsMainFrame(); - if (is_main_frame && !is_error_page) + if (is_main_frame && !is_error_page) { DidStartMainFrameNavigation(validated_url, render_frame_host->GetSiteInstance()); + } if (delegate_) { // Notify the observer about the start of the provisional load. - delegate_->DidStartProvisionalLoad( - render_frame_host, validated_url, is_error_page, is_iframe_srcdoc); + delegate_->DidStartProvisionalLoad(render_frame_host, validated_url, + is_error_page, is_iframe_srcdoc); } -} + if (is_error_page || + base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation)) { + return; + } + + if (render_frame_host->navigation_handle()) { + if (render_frame_host->navigation_handle()->is_transferring()) { + // If the navigation is completing a transfer, this + // DidStartProvisionalLoad should not correspond to a new navigation. + DCHECK_EQ(url, render_frame_host->navigation_handle()->GetURL()); + render_frame_host->navigation_handle()->set_is_transferring(false); + return; + } + + // This ensures that notifications about the end of the previous + // navigation are sent before notifications about the start of the + // new navigation. + render_frame_host->SetNavigationHandle(scoped_ptr<NavigationHandleImpl>()); + } + + render_frame_host->SetNavigationHandle( + NavigationHandleImpl::Create(url, is_main_frame, delegate_)); +} void NavigatorImpl::DidFailProvisionalLoadWithError( RenderFrameHostImpl* render_frame_host, @@ -461,6 +486,7 @@ void NavigatorImpl::DidNavigate( delegate_->DidCommitProvisionalLoad(render_frame_host, params.url, transition_type); + render_frame_host->navigation_handle()->DidCommitNavigation(); } if (!did_navigate) @@ -652,6 +678,7 @@ void NavigatorImpl::OnBeginNavigation( controller_->GetLastCommittedEntryIndex(), controller_->GetEntryCount())); NavigationRequest* navigation_request = frame_tree_node->navigation_request(); + navigation_request->CreateNavigationHandle(delegate_); if (frame_tree_node->IsMainFrame()) { // Renderer-initiated main-frame navigations that need to swap processes @@ -711,6 +738,7 @@ void NavigatorImpl::CommitNavigation(FrameTreeNode* frame_tree_node, CheckWebUIRendererDoesNotDisplayNormalURL( render_frame_host, navigation_request->common_params().url); + navigation_request->TransferNavigationHandleOwnership(render_frame_host); render_frame_host->CommitNavigation(response, body.Pass(), navigation_request->common_params(), navigation_request->request_params()); @@ -740,6 +768,7 @@ void NavigatorImpl::FailedNavigation(FrameTreeNode* frame_tree_node, CheckWebUIRendererDoesNotDisplayNormalURL( render_frame_host, navigation_request->common_params().url); + navigation_request->TransferNavigationHandleOwnership(render_frame_host); render_frame_host->FailedNavigation(navigation_request->common_params(), navigation_request->request_params(), has_stale_copy_in_cache, error_code); @@ -818,6 +847,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_); // Have the current renderer execute its beforeunload event if needed. If it // is not needed (when beforeunload dispatch is not needed or this navigation diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 5e92f5d..aa802dc 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -22,6 +22,7 @@ #include "content/browser/frame_host/frame_mojo_shell.h" #include "content/browser/frame_host/frame_tree.h" #include "content/browser/frame_host/frame_tree_node.h" +#include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/navigation_request.h" #include "content/browser/frame_host/navigator.h" #include "content/browser/frame_host/navigator_impl.h" @@ -799,6 +800,12 @@ void RenderFrameHostImpl::OnDidStartProvisionalLoadForFrame(const GURL& url) { void RenderFrameHostImpl::OnDidFailProvisionalLoadWithError( const FrameHostMsg_DidFailProvisionalLoadWithError_Params& params) { + if (!base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation) && + navigation_handle_) { + navigation_handle_->set_net_error_code( + static_cast<net::Error>(params.error_code)); + } frame_tree_node_->navigator()->DidFailProvisionalLoadWithError(this, params); } @@ -914,6 +921,24 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) { return; } + // If the URL does not match what the NavigationHandle expects, treat the + // commit as a new navigation. This can happen if an ongoing slow + // same-process navigation is interrupted by a synchronous renderer-initiated + // navigation. + if (navigation_handle_ && + navigation_handle_->GetURL() != validated_params.url) { + navigation_handle_.reset(); + } + + // Synchronous renderer-initiated navigations will send a + // DidCommitProvisionalLoad IPC without a prior DidStartProvisionalLoad + // message. + if (!navigation_handle_) { + navigation_handle_ = NavigationHandleImpl::Create( + validated_params.url, frame_tree_node_->IsMainFrame(), + frame_tree_node_->navigator()->GetDelegate()); + } + accessibility_reset_count_ = 0; frame_tree_node()->navigator()->DidNavigate(this, validated_params); @@ -930,6 +955,7 @@ void RenderFrameHostImpl::OnDidDropNavigation() { // If it turns out that the renderer dropped the navigation, the spinner needs // to be turned off. frame_tree_node_->DidStopLoading(); + navigation_handle_.reset(); } RenderWidgetHostImpl* RenderFrameHostImpl::GetRenderWidgetHost() { @@ -960,6 +986,19 @@ int RenderFrameHostImpl::GetEnabledBindings() { return render_view_host_->GetEnabledBindings(); } +void RenderFrameHostImpl::SetNavigationHandle( + scoped_ptr<NavigationHandleImpl> navigation_handle) { + navigation_handle_ = navigation_handle.Pass(); +} + +scoped_ptr<NavigationHandleImpl> +RenderFrameHostImpl::PassNavigationHandleOwnership() { + DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation)); + navigation_handle_->set_is_transferring(true); + return navigation_handle_.Pass(); +} + void RenderFrameHostImpl::OnCrossSiteResponse( const GlobalRequestID& global_request_id, scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request, @@ -1543,6 +1582,7 @@ void RenderFrameHostImpl::OnDidStopLoading() { is_loading_ = false; frame_tree_node_->DidStopLoading(); + navigation_handle_.reset(); } void RenderFrameHostImpl::OnDidChangeLoadProgress(double load_progress) { diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index b4e9442..c7a6c35 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h @@ -58,6 +58,7 @@ class CrossSiteTransferringRequest; class FrameMojoShell; class FrameTree; class FrameTreeNode; +class NavigationHandleImpl; class PermissionServiceContext; class RenderFrameHostDelegate; class RenderFrameProxyHost; @@ -254,6 +255,22 @@ class CONTENT_EXPORT RenderFrameHostImpl // TODO(creis): Make bindings frame-specific, to support cases like <webview>. int GetEnabledBindings(); + NavigationHandleImpl* navigation_handle() const { + return navigation_handle_.get(); + } + + // Called when a new navigation starts in this RenderFrameHost. Ownership of + // |navigation_handle| is transferred. + // PlzNavigate: called when a navigation is ready to commit in this + // RenderFrameHost. + void SetNavigationHandle(scoped_ptr<NavigationHandleImpl> navigation_handle); + + // Gives the ownership of |navigation_handle_| to the caller. + // This happens during transfer navigations, where it should be transferred + // from the RenderFrameHost that issued the initial request to the new + // RenderFrameHost that will issue the transferring request. + scoped_ptr<NavigationHandleImpl> PassNavigationHandleOwnership(); + // Called on the pending RenderFrameHost when the network response is ready to // commit. We should ensure that the old RenderFrameHost runs its unload // handler and determine whether a transfer to a different RenderFrameHost is @@ -752,6 +769,13 @@ class CONTENT_EXPORT RenderFrameHostImpl // Holder of Mojo connection with ImageDownloader service in RenderFrame. image_downloader::ImageDownloaderPtr mojo_image_downloader_; + // Tracks a navigation happening in this frame. Note that while there can be + // two navigations in the same FrameTreeNode, there can only be one + // navigation per RenderFrameHost. + // PlzNavigate: before the navigation is ready to be committed, the + // NavigationHandle for it is owned by the NavigationRequest. + scoped_ptr<NavigationHandleImpl> navigation_handle_; + // NOTE: This must be the last member. base::WeakPtrFactory<RenderFrameHostImpl> weak_ptr_factory_; diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc index b69794f..0c149f6 100644 --- a/content/browser/frame_host/render_frame_host_manager.cc +++ b/content/browser/frame_host/render_frame_host_manager.cc @@ -19,6 +19,7 @@ #include "content/browser/frame_host/interstitial_page_impl.h" #include "content/browser/frame_host/navigation_controller_impl.h" #include "content/browser/frame_host/navigation_entry_impl.h" +#include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/navigation_request.h" #include "content/browser/frame_host/navigator.h" #include "content/browser/frame_host/render_frame_host_factory.h" @@ -410,7 +411,14 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate( cross_site_transferring_request_->request_id() == entry.transferred_global_request_id()) { cross_site_transferring_request_->ReleaseRequest(); + + // The navigating RenderFrameHost should take ownership of the + // NavigationHandle that came from the transferring RenderFrameHost. + DCHECK(transfer_navigation_handle_); + dest_render_frame_host->SetNavigationHandle( + transfer_navigation_handle_.Pass()); } + DCHECK(!transfer_navigation_handle_); return dest_render_frame_host; } @@ -556,14 +564,20 @@ void RenderFrameHostManager::OnCrossSiteResponse( // navigation matches. cross_site_transferring_request_ = cross_site_transferring_request.Pass(); + // Store the NavigationHandle to give it to the appropriate RenderFrameHost + // after it started navigating. + transfer_navigation_handle_ = + pending_render_frame_host->PassNavigationHandleOwnership(); + DCHECK(transfer_navigation_handle_); + // Sanity check that the params are for the correct frame and process. // These should match the RenderFrameHost that made the request. // If it started as a cross-process navigation via OpenURL, this is the - // pending one. If it wasn't cross-process until the transfer, this is the - // current one. - int render_frame_id = pending_render_frame_host_ ? - pending_render_frame_host_->GetRoutingID() : - render_frame_host_->GetRoutingID(); + // pending one. If it wasn't cross-process until the transfer, this is + // the current one. + int render_frame_id = pending_render_frame_host_ + ? pending_render_frame_host_->GetRoutingID() + : render_frame_host_->GetRoutingID(); DCHECK_EQ(render_frame_id, pending_render_frame_host->GetRoutingID()); int process_id = pending_render_frame_host_ ? pending_render_frame_host_->GetProcess()->GetID() : @@ -588,6 +602,10 @@ void RenderFrameHostManager::OnCrossSiteResponse( // The transferring request was only needed during the RequestTransferURL // call, so it is safe to clear at this point. cross_site_transferring_request_.reset(); + + // If the navigation continued, the NavigationHandle should have been + // transfered to a RenderFrameHost. In the other cases, it should be cleared. + transfer_navigation_handle_.reset(); } void RenderFrameHostManager::DidNavigateFrame( diff --git a/content/browser/frame_host/render_frame_host_manager.h b/content/browser/frame_host/render_frame_host_manager.h index add2f3f..1359e9f 100644 --- a/content/browser/frame_host/render_frame_host_manager.h +++ b/content/browser/frame_host/render_frame_host_manager.h @@ -30,6 +30,7 @@ class InterstitialPageImpl; class NavigationControllerImpl; class NavigationEntry; class NavigationEntryImpl; +class NavigationHandleImpl; class NavigationRequest; class NavigatorTestWithBrowserSideNavigation; class RenderFrameHost; @@ -747,6 +748,14 @@ class CONTENT_EXPORT RenderFrameHostManager { // cleaned up if the navigation is cancelled. Otherwise, this is NULL. scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request_; + // This is used to temporarily store the NavigationHandle during + // transferring navigations. The handle needs to be stored because the old + // RenderFrameHost may be discarded before a new RenderFrameHost is created + // for the navigation. + // PlzNavigate: this will never be set since there are no transferring + // navigations in PlzNavigate. + scoped_ptr<NavigationHandleImpl> transfer_navigation_handle_; + // If either of these is non-NULL, the pending navigation is to a chrome: // page. The scoped_ptr is used if pending_web_ui_ != web_ui_, the WeakPtr is // used for when they reference the same object. If either is non-NULL, the diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc index 3a9e222..c3aad10 100644 --- a/content/browser/frame_host/render_frame_host_manager_unittest.cc +++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc @@ -1250,8 +1250,9 @@ TEST_F(RenderFrameHostManagerTest, PageDoesBackAndReload) { params.is_post = false; params.page_state = PageState::CreateFromURL(kUrl2); - contents()->GetFrameTree()->root()->navigator()->DidNavigate(evil_rfh, - params); + evil_rfh->SimulateNavigationStart(kUrl2); + evil_rfh->SendNavigateWithParams(¶ms); + evil_rfh->SimulateNavigationStop(); // That should NOT have cancelled the pending RFH, because the reload did // not have a user gesture. Thus, the pending back navigation will still @@ -1273,8 +1274,9 @@ TEST_F(RenderFrameHostManagerTest, PageDoesBackAndReload) { // Now do the same but as a user gesture. params.gesture = NavigationGestureUser; - contents()->GetFrameTree()->root()->navigator()->DidNavigate(evil_rfh, - params); + evil_rfh->SimulateNavigationStart(kUrl2); + evil_rfh->SendNavigateWithParams(¶ms); + evil_rfh->SimulateNavigationStop(); // User navigation should have cancelled the pending RFH. EXPECT_TRUE(contents()->GetRenderManagerForTesting()-> diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 7065559..287d016 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -36,6 +36,7 @@ #include "content/browser/frame_host/cross_process_frame_connector.h" #include "content/browser/frame_host/interstitial_page_impl.h" #include "content/browser/frame_host/navigation_entry_impl.h" +#include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/navigator_impl.h" #include "content/browser/frame_host/render_frame_host_impl.h" #include "content/browser/frame_host/render_widget_host_view_child_frame.h" @@ -453,12 +454,32 @@ WebContentsImpl::~WebContentsImpl() { frame_tree_.root()->ResetForNewProcess(); GetRenderManager()->ResetProxyHosts(); - // Manually call the observer methods for the root frame tree node. + // Manually call the observer methods for the root frame tree node. It is + // necessary to manually delete all objects tracking navigations + // (NavigationHandle, NavigationRequest) for observers to be properly + // notified of these navigations stopping before the WebContents is + // destroyed. RenderFrameHostManager* root = GetRenderManager(); - if (root->pending_frame_host()) + if (root->pending_frame_host()) { root->pending_frame_host()->SetRenderFrameCreated(false); + root->pending_frame_host()->SetNavigationHandle( + scoped_ptr<NavigationHandleImpl>()); + } root->current_frame_host()->SetRenderFrameCreated(false); + root->current_frame_host()->SetNavigationHandle( + scoped_ptr<NavigationHandleImpl>()); + + // PlzNavigate: clear up state specific to browser-side navigation. + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation)) { + frame_tree_.root()->ResetNavigationRequest(false); + if (root->speculative_frame_host()) { + root->speculative_frame_host()->SetRenderFrameCreated(false); + root->speculative_frame_host()->SetNavigationHandle( + scoped_ptr<NavigationHandleImpl>()); + } + } FOR_EACH_OBSERVER(WebContentsObserver, observers_, FrameDeleted(root->current_frame_host())); @@ -2531,6 +2552,14 @@ void WebContentsImpl::DidGetRedirectForResourceRequest( NOTIFICATION_RESOURCE_RECEIVED_REDIRECT, Source<WebContents>(this), Details<const ResourceRedirectDetails>(&details)); + + if (IsResourceTypeFrame(details.resource_type)) { + NavigationHandleImpl* navigation_handle = + static_cast<RenderFrameHostImpl*>(render_frame_host) + ->navigation_handle(); + if (navigation_handle) + navigation_handle->DidRedirectNavigation(details.new_url); + } } void WebContentsImpl::NotifyWebContentsFocused() { @@ -2748,6 +2777,27 @@ void WebContentsImpl::SetFocusToLocationBar(bool select_all) { delegate_->SetFocusToLocationBar(select_all); } +void WebContentsImpl::DidStartNavigation(NavigationHandle* navigation_handle) { + FOR_EACH_OBSERVER(WebContentsObserver, observers_, + DidStartNavigation(navigation_handle)); +} + +void WebContentsImpl::DidRedirectNavigation( + NavigationHandle* navigation_handle) { + FOR_EACH_OBSERVER(WebContentsObserver, observers_, + DidRedirectNavigation(navigation_handle)); +} + +void WebContentsImpl::DidCommitNavigation(NavigationHandle* navigation_handle) { + FOR_EACH_OBSERVER(WebContentsObserver, observers_, + DidCommitNavigation(navigation_handle)); +} + +void WebContentsImpl::DidFinishNavigation(NavigationHandle* navigation_handle) { + FOR_EACH_OBSERVER(WebContentsObserver, observers_, + DidFinishNavigation(navigation_handle)); +} + void WebContentsImpl::DidStartProvisionalLoad( RenderFrameHostImpl* render_frame_host, const GURL& validated_url, diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 58aefe1..719b419 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -508,6 +508,10 @@ class CONTENT_EXPORT WebContentsImpl // NavigatorDelegate --------------------------------------------------------- + void DidStartNavigation(NavigationHandle* navigation_handle) override; + void DidRedirectNavigation(NavigationHandle* navigation_handle) override; + void DidCommitNavigation(NavigationHandle* navigation_handle) override; + void DidFinishNavigation(NavigationHandle* navigation_handle) override; void DidStartProvisionalLoad(RenderFrameHostImpl* render_frame_host, const GURL& validated_url, bool is_error_page, diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc index 88bdfd7..2400b98 100644 --- a/content/browser/web_contents/web_contents_impl_unittest.cc +++ b/content/browser/web_contents/web_contents_impl_unittest.cc @@ -1472,6 +1472,7 @@ TEST_F(WebContentsImplTest, HistoryNavigationExitsFullscreen) { EXPECT_EQ(orig_rfh, contents()->GetMainFrame()); contents()->TestDidNavigate(orig_rfh, i + 1, entry_id, false, url, ui::PAGE_TRANSITION_FORWARD_BACK); + orig_rfh->SimulateNavigationStop(); // Confirm fullscreen has exited. EXPECT_FALSE(orig_rvh->IsFullscreenGranted()); diff --git a/content/content_browser.gypi b/content/content_browser.gypi index 7c1ba3e..3649a53 100644 --- a/content/content_browser.gypi +++ b/content/content_browser.gypi @@ -182,6 +182,7 @@ 'public/browser/navigation_details.cc', 'public/browser/navigation_details.h', 'public/browser/navigation_entry.h', + 'public/browser/navigation_handle.h', 'public/browser/navigation_type.h', 'public/browser/navigator_connect_context.h', 'public/browser/navigator_connect_service_factory.h', @@ -742,6 +743,8 @@ 'browser/frame_host/navigation_entry_impl.h', 'browser/frame_host/navigation_entry_screenshot_manager.cc', 'browser/frame_host/navigation_entry_screenshot_manager.h', + 'browser/frame_host/navigation_handle_impl.cc', + 'browser/frame_host/navigation_handle_impl.h', 'browser/frame_host/navigation_request.cc', 'browser/frame_host/navigation_request.h', 'browser/frame_host/navigation_request_info.cc', diff --git a/content/public/browser/navigation_handle.h b/content/public/browser/navigation_handle.h new file mode 100644 index 0000000..49d0884 --- /dev/null +++ b/content/public/browser/navigation_handle.h @@ -0,0 +1,40 @@ +// 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_PUBLIC_BROWSER_NAVIGATION_HANDLE_H_ +#define CONTENT_PUBLIC_BROWSER_NAVIGATION_HANDLE_H_ + +#include "content/common/content_export.h" +#include "net/base/net_errors.h" + +class GURL; + +namespace content { + +// A NavigationHandle tracks information related to a single navigation. +class CONTENT_EXPORT NavigationHandle { + public: + virtual ~NavigationHandle() {} + + // The URL the frame is navigating to. This may change during the navigation + // when encountering a server redirect. + virtual const GURL& GetURL() const = 0; + + // The net error code if an error happened prior to commit. Otherwise it will + // be net::OK. + virtual net::Error GetNetErrorCode() const = 0; + + // Whether the navigation is taking place in the main frame or in a subframe. + virtual bool IsInMainFrame() const = 0; + + // Whether the navigation has successfully committed a document. + virtual bool HasCommittedDocument() const = 0; + + // Whether an error page has committed for the navigation. + virtual bool HasCommittedErrorPage() const = 0; +}; + +} // namespace content + +#endif // CONTENT_PUBLIC_BROWSER_NAVIGATION_HANDLE_H_ diff --git a/content/public/browser/web_contents_observer.h b/content/public/browser/web_contents_observer.h index 15fa138..d310cb2 100644 --- a/content/public/browser/web_contents_observer.h +++ b/content/public/browser/web_contents_observer.h @@ -20,6 +20,7 @@ namespace content { class NavigationEntry; +class NavigationHandle; class RenderFrameHost; class RenderViewHost; class WebContents; @@ -116,6 +117,39 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener, virtual void RenderViewHostChanged(RenderViewHost* old_host, RenderViewHost* new_host) {} + // Navigation related events ------------------------------------------------ + + // Called when a navigation started in the WebContents. |navigation_handle| + // is unique to a specific navigation. The same |navigation_handle| will be + // provided on subsequent calls to DidRedirect/Commit/FinishNavigation + // related to this navigation. + // + // Note that this is fired by navigations in any frame of the WebContents, + // not just the main frame. + // + // Note that more than one navigation can be ongoing in the same frame at the + // same time (including the main frame). Each will get its own + // NavigationHandle. + // + // Note that there is no guarantee that DidFinishNavigation will be called + // for any particular navigation before DidStartNavigation is called on the + // next. + virtual void DidStartNavigation(NavigationHandle* navigation_handle) {} + + // Called when a navigation encountered a server redirect. + virtual void DidRedirectNavigation(NavigationHandle* navigation_handle) {} + + // Called when a navigation was committed. + virtual void DidCommitNavigation(NavigationHandle* navigation_handle) {} + + // Called when a navigation stopped in the WebContents. This happens when a + // navigation is either aborted, replaced by a new one, or the document load + // finishes. Note that |navigation_handle| will be destroyed at the end of + // this call, so do not keep a reference to it afterward. + virtual void DidFinishNavigation(NavigationHandle* navigation_handle) {} + + // --------------------------------------------------------------------------- + // This method is invoked after the WebContents decides which RenderFrameHost // to use for the next browser-initiated navigation, but before the navigation // starts. It is not called for most renderer-initiated navigations, and it diff --git a/content/public/test/test_renderer_host.h b/content/public/test/test_renderer_host.h index c429876..4206c16 100644 --- a/content/public/test/test_renderer_host.h +++ b/content/public/test/test_renderer_host.h @@ -75,6 +75,26 @@ class RenderFrameHostTester { // RenderFrameHost is owned by the parent RenderFrameHost. virtual RenderFrameHost* AppendChild(const std::string& frame_name) = 0; + // Simulates a renderer-initiated navigation to |url| starting in the + // RenderFrameHost. + virtual void SimulateNavigationStart(const GURL& url) = 0; + + // Simulates a redirect to |new_url| for the navigation in the + // RenderFrameHost. + virtual void SimulateRedirect(const GURL& new_url) = 0; + + // Simulates a navigation to |url| committing in the RenderFrameHost. + virtual void SimulateNavigationCommit(const GURL& url) = 0; + + // Simulates a navigation to |url| failing with the error code |error_code|. + virtual void SimulateNavigationError(const GURL& url, int error_code) = 0; + + // Simulates the commit of an error page following a navigation failure. + virtual void SimulateNavigationErrorPageCommit() = 0; + + // Simulates a navigation stopping in the RenderFrameHost. + virtual void SimulateNavigationStop() = 0; + // Calls OnDidCommitProvisionalLoad on the RenderFrameHost with the given // information with various sets of parameters. These are helper functions for // simulating the most common types of loads. diff --git a/content/public/test/web_contents_tester.h b/content/public/test/web_contents_tester.h index 1f28160..c3fe691 100644 --- a/content/public/test/web_contents_tester.h +++ b/content/public/test/web_contents_tester.h @@ -67,6 +67,20 @@ class WebContentsTester { // speculative RenderFrameHost for the main frame if one exists. virtual RenderFrameHost* GetPendingMainFrame() const = 0; + // Creates a pending navigation to |url|. Also simulates the + // DidStartProvisionalLoad received from the renderer. To commit the + // navigation, callers should use + // RenderFrameHostTester::SimulateNavigationCommit. Callers can then use + // RenderFrameHostTester::SimulateNavigationStop to simulate the navigation + // stop. + // Note that this function is meant for callers that want to control the + // timing of navigation events precisely. Other callers should use + // NavigateAndCommit. + // PlzNavigate: this does not simulate the DidStartProvisionalLoad from the + // renderer, as it only should be received after the navigation is ready to + // commit. + virtual void StartNavigation(const GURL& url) = 0; + // Creates a pending navigation to the given URL with the default parameters // and then commits the load with a page ID one larger than any seen. This // emulates what happens on a new navigation. diff --git a/content/test/test_navigation_url_loader.cc b/content/test/test_navigation_url_loader.cc index 6eea8b2..d054dc3 100644 --- a/content/test/test_navigation_url_loader.cc +++ b/content/test/test_navigation_url_loader.cc @@ -33,6 +33,10 @@ void TestNavigationURLLoader::SimulateServerRedirect(const GURL& redirect_url) { CallOnRequestRedirected(redirect_info, response); } +void TestNavigationURLLoader::SimulateError(int error_code) { + delegate_->OnRequestFailed(false, error_code); +} + void TestNavigationURLLoader::CallOnRequestRedirected( const net::RedirectInfo& redirect_info, const scoped_refptr<ResourceResponse>& response) { diff --git a/content/test/test_navigation_url_loader.h b/content/test/test_navigation_url_loader.h index f5eb288..9cfe1ba 100644 --- a/content/test/test_navigation_url_loader.h +++ b/content/test/test_navigation_url_loader.h @@ -38,6 +38,8 @@ class TestNavigationURLLoader void SimulateServerRedirect(const GURL& redirect_url); + void SimulateError(int error_code); + void CallOnRequestRedirected(const net::RedirectInfo& redirect_info, const scoped_refptr<ResourceResponse>& response); void CallOnResponseStarted(const scoped_refptr<ResourceResponse>& response, diff --git a/content/test/test_render_frame_host.cc b/content/test/test_render_frame_host.cc index 950aa19..2d081ef 100644 --- a/content/test/test_render_frame_host.cc +++ b/content/test/test_render_frame_host.cc @@ -6,13 +6,16 @@ #include "base/command_line.h" #include "content/browser/frame_host/frame_tree.h" +#include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/navigation_request.h" #include "content/browser/frame_host/navigator.h" #include "content/browser/frame_host/navigator_impl.h" #include "content/browser/frame_host/render_frame_host_delegate.h" +#include "content/browser/web_contents/web_contents_impl.h" #include "content/common/frame_messages.h" #include "content/public/browser/stream_handle.h" #include "content/public/common/content_switches.h" +#include "content/public/common/url_constants.h" #include "content/test/browser_side_navigation_test_utils.h" #include "content/test/test_navigation_url_loader.h" #include "content/test/test_render_view_host.h" @@ -86,6 +89,113 @@ TestRenderFrameHost* TestRenderFrameHost::AppendChild( child_creation_observer_.last_created_frame()); } +void TestRenderFrameHost::SimulateNavigationStart(const GURL& url) { + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation)) { + SendRendererInitiatedNavigationRequest(url, false); + return; + } + + OnDidStartLoading(true); + OnDidStartProvisionalLoadForFrame(url); +} + +void TestRenderFrameHost::SimulateRedirect(const GURL& new_url) { + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation)) { + NavigationRequest* request = frame_tree_node_->navigation_request(); + TestNavigationURLLoader* url_loader = + static_cast<TestNavigationURLLoader*>(request->loader_for_testing()); + CHECK(url_loader); + url_loader->SimulateServerRedirect(new_url); + return; + } + + // Note that this does not simulate + // WebContentsImpl::DidGetRedirectForResourceRequest due to the difficulty in + // creating fake ResourceRequestDetails on the UI thread. + navigation_handle()->DidRedirectNavigation(new_url); +} + +void TestRenderFrameHost::SimulateNavigationCommit(const GURL& url) { + if (frame_tree_node()->navigation_request()) + PrepareForCommit(); + + FrameHostMsg_DidCommitProvisionalLoad_Params params; + params.page_id = ComputeNextPageID(); + params.nav_entry_id = 0; + params.url = url; + params.transition = GetParent() ? ui::PAGE_TRANSITION_MANUAL_SUBFRAME + : ui::PAGE_TRANSITION_LINK; + params.should_update_history = true; + params.did_create_new_entry = true; + params.gesture = NavigationGestureUser; + params.contents_mime_type = contents_mime_type_; + params.is_post = false; + params.http_status_code = 200; + params.socket_address.set_host("2001:db8::1"); + params.socket_address.set_port(80); + params.history_list_was_cleared = simulate_history_list_was_cleared_; + params.original_request_url = url; + + url::Replacements<char> replacements; + replacements.ClearRef(); + params.was_within_same_page = + url.ReplaceComponents(replacements) == + GetLastCommittedURL().ReplaceComponents(replacements); + + params.page_state = PageState::CreateForTesting(url, false, nullptr, nullptr); + + SendNavigateWithParams(¶ms); +} + +void TestRenderFrameHost::SimulateNavigationError(const GURL& url, + int error_code) { + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation)) { + NavigationRequest* request = frame_tree_node_->navigation_request(); + TestNavigationURLLoader* url_loader = + static_cast<TestNavigationURLLoader*>(request->loader_for_testing()); + CHECK(url_loader); + url_loader->SimulateError(error_code); + return; + } + + FrameHostMsg_DidFailProvisionalLoadWithError_Params error_params; + error_params.error_code = error_code; + error_params.url = url; + OnDidFailProvisionalLoadWithError(error_params); +} + +void TestRenderFrameHost::SimulateNavigationErrorPageCommit() { + CHECK(navigation_handle()); + GURL error_url = GURL(kUnreachableWebDataURL); + OnDidStartProvisionalLoadForFrame(error_url); + FrameHostMsg_DidCommitProvisionalLoad_Params params; + params.page_id = ComputeNextPageID(); + params.nav_entry_id = 0; + params.did_create_new_entry = true; + params.url = navigation_handle()->GetURL(); + params.transition = GetParent() ? ui::PAGE_TRANSITION_MANUAL_SUBFRAME + : ui::PAGE_TRANSITION_LINK; + params.was_within_same_page = false; + params.url_is_unreachable = true; + params.page_state = PageState::CreateForTesting(navigation_handle()->GetURL(), + false, nullptr, nullptr); + SendNavigateWithParams(¶ms); +} + +void TestRenderFrameHost::SimulateNavigationStop() { + if (is_loading()) { + OnDidStopLoading(); + } else if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation)) { + // Even if the RenderFrameHost is not loading, there may still be an + // ongoing navigation in the FrameTreeNode. Cancel this one as well. + frame_tree_node()->ResetNavigationRequest(false); + } +} + void TestRenderFrameHost::SetContentsMimeType(const std::string& mime_type) { contents_mime_type_ = mime_type; } @@ -148,6 +258,7 @@ void TestRenderFrameHost::SendNavigateWithParameters( // DidStartProvisionalLoad may delete the pending entry that holds |url|, // so we keep a copy of it to use below. GURL url_copy(url); + OnDidStartLoading(true); OnDidStartProvisionalLoadForFrame(url_copy); FrameHostMsg_DidCommitProvisionalLoad_Params params; @@ -272,4 +383,19 @@ void TestRenderFrameHost::PrepareForCommitWithServerRedirect( url_loader->CallOnResponseStarted(response, MakeEmptyStream()); } +int32 TestRenderFrameHost::ComputeNextPageID() { + const NavigationEntryImpl* entry = static_cast<NavigationEntryImpl*>( + frame_tree_node()->navigator()->GetController()->GetPendingEntry()); + DCHECK_IMPLIES(entry && entry->site_instance(), + entry->site_instance() == GetSiteInstance()); + // Entry can be null when committing an error page (the pending entry was + // cleared during DidFailProvisionalLoad). + int page_id = entry ? entry->GetPageID() : -1; + if (page_id == -1) { + WebContentsImpl* web_contents = static_cast<WebContentsImpl*>(delegate()); + page_id = web_contents->GetMaxPageIDForSiteInstance(GetSiteInstance()) + 1; + } + return page_id; +} + } // namespace content diff --git a/content/test/test_render_frame_host.h b/content/test/test_render_frame_host.h index 339f861..2cd0b99 100644 --- a/content/test/test_render_frame_host.h +++ b/content/test/test_render_frame_host.h @@ -53,6 +53,12 @@ class TestRenderFrameHost : public RenderFrameHostImpl, // RenderFrameHostTester implementation. void InitializeRenderFrameIfNeeded() override; TestRenderFrameHost* AppendChild(const std::string& frame_name) override; + void SimulateNavigationStart(const GURL& url) override; + void SimulateRedirect(const GURL& new_url) override; + void SimulateNavigationCommit(const GURL& url) override; + void SimulateNavigationError(const GURL& url, int error_code) override; + void SimulateNavigationErrorPageCommit() override; + void SimulateNavigationStop() override; void SendNavigate(int page_id, int nav_entry_id, bool did_create_new_entry, @@ -128,6 +134,9 @@ class TestRenderFrameHost : public RenderFrameHostImpl, int response_code, const ModificationCallback& callback); + // Computes the page ID for a pending navigation in this RenderFrameHost; + int32 ComputeNextPageID(); + TestRenderFrameHostCreationObserver child_creation_observer_; std::string contents_mime_type_; diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc index 27d5404..b7b958e 100644 --- a/content/test/test_web_contents.cc +++ b/content/test/test_web_contents.cc @@ -64,6 +64,26 @@ TestRenderFrameHost* TestWebContents::GetPendingMainFrame() const { GetRenderManager()->pending_frame_host()); } +void TestWebContents::StartNavigation(const GURL& url) { + GetController().LoadURL(url, Referrer(), ui::PAGE_TRANSITION_LINK, + std::string()); + GURL loaded_url(url); + bool reverse_on_redirect = false; + BrowserURLHandlerImpl::GetInstance()->RewriteURLIfNecessary( + &loaded_url, GetBrowserContext(), &reverse_on_redirect); + + // This will simulate receiving the DidStartProvisionalLoad IPC from the + // renderer. + if (!base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableBrowserSideNavigation)) { + if (GetMainFrame()->is_waiting_for_beforeunload_ack()) + GetMainFrame()->SendBeforeUnloadACK(true); + TestRenderFrameHost* rfh = + GetPendingMainFrame() ? GetPendingMainFrame() : GetMainFrame(); + rfh->SimulateNavigationStart(url); + } +} + int TestWebContents::DownloadImage(const GURL& url, bool is_favicon, uint32_t max_bitmap_size, @@ -96,6 +116,13 @@ void TestWebContents::TestDidNavigateWithReferrer( const GURL& url, const Referrer& referrer, ui::PageTransition transition) { + TestRenderFrameHost* rfh = + static_cast<TestRenderFrameHost*>(render_frame_host); + rfh->InitializeRenderFrameIfNeeded(); + + if (!rfh->is_loading()) + rfh->SimulateNavigationStart(url); + FrameHostMsg_DidCommitProvisionalLoad_Params params; params.page_id = page_id; @@ -114,10 +141,7 @@ void TestWebContents::TestDidNavigateWithReferrer( params.is_post = false; params.page_state = PageState::CreateFromURL(url); - TestRenderFrameHost* rfh = - static_cast<TestRenderFrameHost*>(render_frame_host); - rfh->InitializeRenderFrameIfNeeded(); - rfh->frame_tree_node()->navigator()->DidNavigate(rfh, params); + rfh->SendNavigateWithParams(¶ms); } const std::string& TestWebContents::GetSaveFrameHeaders() { @@ -157,13 +181,12 @@ WebContents* TestWebContents::Clone() { } void TestWebContents::NavigateAndCommit(const GURL& url) { - GetController().LoadURL( - url, Referrer(), ui::PAGE_TRANSITION_LINK, std::string()); + GetController().LoadURL(url, Referrer(), ui::PAGE_TRANSITION_LINK, + std::string()); GURL loaded_url(url); bool reverse_on_redirect = false; BrowserURLHandlerImpl::GetInstance()->RewriteURLIfNecessary( &loaded_url, GetBrowserContext(), &reverse_on_redirect); - // LoadURL created a navigation entry, now simulate the RenderView sending // a notification that it actually navigated. CommitPendingNavigation(); diff --git a/content/test/test_web_contents.h b/content/test/test_web_contents.h index 3e45d8a..17134f2 100644 --- a/content/test/test_web_contents.h +++ b/content/test/test_web_contents.h @@ -45,6 +45,7 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester { // WebContentsTester implementation. void CommitPendingNavigation() override; TestRenderFrameHost* GetPendingMainFrame() const override; + void StartNavigation(const GURL& url) override; void NavigateAndCommit(const GURL& url) override; void TestSetIsLoading(bool value) override; void ProceedWithCrossSiteNavigation() override; diff --git a/content/test/web_contents_observer_sanity_checker.cc b/content/test/web_contents_observer_sanity_checker.cc index 1e76ca2..09151e8 100644 --- a/content/test/web_contents_observer_sanity_checker.cc +++ b/content/test/web_contents_observer_sanity_checker.cc @@ -7,11 +7,13 @@ #include "base/strings/stringprintf.h" #include "content/browser/frame_host/render_frame_host_impl.h" #include "content/common/frame_messages.h" +#include "content/public/browser/navigation_handle.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_process_host.h" #include "content/public/browser/site_instance.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" +#include "net/base/net_errors.h" namespace content { @@ -119,6 +121,61 @@ void WebContentsObserverSanityChecker::FrameDeleted( CHECK(!web_contents_destroyed_); } +void WebContentsObserverSanityChecker::DidStartNavigation( + NavigationHandle* navigation_handle) { + CHECK(!NavigationIsOngoing(navigation_handle)); + CHECK(!NavigationIsOngoingAndCommitted(navigation_handle)); + + CHECK(navigation_handle->GetNetErrorCode() == net::OK); + CHECK(!navigation_handle->HasCommittedDocument()); + CHECK(!navigation_handle->HasCommittedErrorPage()); + + ongoing_navigations_.insert(navigation_handle); +} + +void WebContentsObserverSanityChecker::DidRedirectNavigation( + NavigationHandle* navigation_handle) { + CHECK(NavigationIsOngoing(navigation_handle)); + CHECK(!NavigationIsOngoingAndCommitted(navigation_handle)); + + CHECK(navigation_handle->GetNetErrorCode() == net::OK); + CHECK(!navigation_handle->HasCommittedDocument()); + CHECK(!navigation_handle->HasCommittedErrorPage()); +} + +void WebContentsObserverSanityChecker::DidCommitNavigation( + NavigationHandle* navigation_handle) { + CHECK(NavigationIsOngoing(navigation_handle)); + CHECK(!NavigationIsOngoingAndCommitted(navigation_handle)); + + CHECK_NE(navigation_handle->HasCommittedDocument(), + navigation_handle->HasCommittedErrorPage()); + CHECK_IMPLIES(navigation_handle->HasCommittedDocument(), + navigation_handle->GetNetErrorCode() == net::OK); + CHECK_IMPLIES(navigation_handle->HasCommittedErrorPage(), + navigation_handle->GetNetErrorCode() != net::OK); + + ongoing_committed_navigations_.insert(navigation_handle); +} + +void WebContentsObserverSanityChecker::DidFinishNavigation( + NavigationHandle* navigation_handle) { + CHECK(NavigationIsOngoing(navigation_handle)); + + CHECK_IMPLIES(NavigationIsOngoingAndCommitted(navigation_handle), + navigation_handle->HasCommittedDocument() != + navigation_handle->HasCommittedErrorPage()); + CHECK_IMPLIES(navigation_handle->HasCommittedDocument(), + navigation_handle->GetNetErrorCode() == net::OK); + CHECK_IMPLIES(navigation_handle->HasCommittedErrorPage(), + navigation_handle->GetNetErrorCode() != net::OK); + + if (NavigationIsOngoingAndCommitted(navigation_handle)) + ongoing_committed_navigations_.erase(navigation_handle); + + ongoing_navigations_.erase(navigation_handle); +} + void WebContentsObserverSanityChecker::DidStartProvisionalLoadForFrame( RenderFrameHost* render_frame_host, const GURL& validated_url, @@ -220,6 +277,8 @@ bool WebContentsObserverSanityChecker::OnMessageReceived( void WebContentsObserverSanityChecker::WebContentsDestroyed() { CHECK(!web_contents_destroyed_); web_contents_destroyed_ = true; + CHECK(ongoing_navigations_.empty()); + CHECK(ongoing_committed_navigations_.empty()); } WebContentsObserverSanityChecker::WebContentsObserverSanityChecker( @@ -266,4 +325,16 @@ std::string WebContentsObserverSanityChecker::Format( render_frame_host->GetSiteInstance()->GetSiteURL().spec().c_str()); } +bool WebContentsObserverSanityChecker::NavigationIsOngoing( + NavigationHandle* navigation_handle) { + auto it = ongoing_navigations_.find(navigation_handle); + return it != ongoing_navigations_.end(); +} + +bool WebContentsObserverSanityChecker::NavigationIsOngoingAndCommitted( + NavigationHandle* navigation_handle) { + auto it = ongoing_committed_navigations_.find(navigation_handle); + return it != ongoing_committed_navigations_.end(); +} + } // namespace content diff --git a/content/test/web_contents_observer_sanity_checker.h b/content/test/web_contents_observer_sanity_checker.h index f7ca421..347754b 100644 --- a/content/test/web_contents_observer_sanity_checker.h +++ b/content/test/web_contents_observer_sanity_checker.h @@ -40,6 +40,10 @@ class WebContentsObserverSanityChecker : public WebContentsObserver, void RenderFrameHostChanged(RenderFrameHost* old_host, RenderFrameHost* new_host) override; void FrameDeleted(RenderFrameHost* render_frame_host) override; + void DidStartNavigation(NavigationHandle* navigation_handle) override; + void DidRedirectNavigation(NavigationHandle* navigation_handle) override; + void DidCommitNavigation(NavigationHandle* navigation_handle) override; + void DidFinishNavigation(NavigationHandle* navigation_handle) override; void DidStartProvisionalLoadForFrame(RenderFrameHost* render_frame_host, const GURL& validated_url, bool is_error_page, @@ -89,10 +93,16 @@ class WebContentsObserverSanityChecker : public WebContentsObserver, void AssertRenderFrameExists(RenderFrameHost* render_frame_host); void AssertMainFrameExists(); + bool NavigationIsOngoing(NavigationHandle* navigation_handle); + bool NavigationIsOngoingAndCommitted(NavigationHandle* navigation_handle); + std::set<std::pair<int, int>> current_hosts_; std::set<std::pair<int, int>> live_routes_; std::set<std::pair<int, int>> deleted_routes_; + std::set<NavigationHandle*> ongoing_navigations_; + std::set<NavigationHandle*> ongoing_committed_navigations_; + bool web_contents_destroyed_; DISALLOW_COPY_AND_ASSIGN(WebContentsObserverSanityChecker); |