diff options
author | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-17 00:06:34 +0000 |
---|---|---|
committer | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-17 00:06:34 +0000 |
commit | 053a1db400e2f07bd214320c9175b40d56d74eaf (patch) | |
tree | 8c16531188a17a1f98c2dc427a49855539ad88dc | |
parent | 918b1d33b9e86c31827c0aacb5715ed2cd05fc94 (diff) | |
download | chromium_src-053a1db400e2f07bd214320c9175b40d56d74eaf.zip chromium_src-053a1db400e2f07bd214320c9175b40d56d74eaf.tar.gz chromium_src-053a1db400e2f07bd214320c9175b40d56d74eaf.tar.bz2 |
Add back the code to PrerenderManager::PendingSwap to handle RenderViewCreated being called for a cross-site navigation.
The two tests are from David's https://codereview.chromium.org/140073003.
BUG=330735
R=davidben@chromium.org, tburkard@chromium.org
Review URL: https://codereview.chromium.org/138583003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245386 0039d316-1c4b-4281-b951-d872f2087c98
8 files changed, 197 insertions, 24 deletions
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc index 798807f..4236c97 100644 --- a/chrome/browser/prerender/prerender_browsertest.cc +++ b/chrome/browser/prerender/prerender_browsertest.cc @@ -787,6 +787,87 @@ void CreateHangingFirstRequestProtocolHandlerOnIO(const GURL& url, url, never_respond_handler.Pass()); } +// Wrapper over URLRequestMockHTTPJob that exposes extra callbacks. +class MockHTTPJob : public content::URLRequestMockHTTPJob { + public: + MockHTTPJob(net::URLRequest* request, + net::NetworkDelegate* delegate, + const base::FilePath& file) + : content::URLRequestMockHTTPJob(request, delegate, file) { + } + + void set_start_callback(const base::Closure& start_callback) { + start_callback_ = start_callback; + } + + virtual void Start() OVERRIDE { + if (!start_callback_.is_null()) + start_callback_.Run(); + content::URLRequestMockHTTPJob::Start(); + } + + private: + virtual ~MockHTTPJob() {} + + base::Closure start_callback_; +}; + +// Dummy counter class to live on the UI thread for counting requests. +class RequestCounter : public base::SupportsWeakPtr<RequestCounter> { + public: + RequestCounter() : count_(0) {} + int count() const { return count_; } + void RequestStarted() { count_++; } + private: + int count_; +}; + +// Protocol handler which counts the number of requests that start. +class CountingProtocolHandler + : public net::URLRequestJobFactory::ProtocolHandler { + public: + CountingProtocolHandler(const base::FilePath& file, + const base::WeakPtr<RequestCounter>& counter) + : file_(file), + counter_(counter), + weak_factory_(this) { + } + virtual ~CountingProtocolHandler() {} + + virtual net::URLRequestJob* MaybeCreateJob( + net::URLRequest* request, + net::NetworkDelegate* network_delegate) const OVERRIDE { + MockHTTPJob* job = new MockHTTPJob(request, network_delegate, file_); + job->set_start_callback(base::Bind(&CountingProtocolHandler::RequestStarted, + weak_factory_.GetWeakPtr())); + return job; + } + + void RequestStarted() { + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&RequestCounter::RequestStarted, counter_)); + } + + private: + base::FilePath file_; + base::WeakPtr<RequestCounter> counter_; + mutable base::WeakPtrFactory<CountingProtocolHandler> weak_factory_; +}; + +// Makes |url| respond to requests with the contents of |file|, counting the +// number that start in |counter|. +void CreateCountingProtocolHandlerOnIO( + const GURL& url, + const base::FilePath& file, + const base::WeakPtr<RequestCounter>& counter) { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + scoped_ptr<net::URLRequestJobFactory::ProtocolHandler> protocol_handler( + new CountingProtocolHandler(file, counter)); + net::URLRequestFilter::GetInstance()->AddUrlProtocolHandler( + url, protocol_handler.Pass()); +} + // Makes |url| respond to requests with the contents of |file|. void CreateMockProtocolHandlerOnIO(const GURL& url, const base::FilePath& file) { @@ -869,6 +950,12 @@ class NeverRunsExternalProtocolHandlerDelegate } }; +base::FilePath GetTestPath(const std::string& file_name) { + return ui_test_utils::GetTestFilePath( + base::FilePath(FILE_PATH_LITERAL("prerender")), + base::FilePath().AppendASCII(file_name)); +} + } // namespace // Many of these tests are flaky. See http://crbug.com/249179 @@ -1711,8 +1798,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNoCommitNoSwap) { // Navigate to a page that triggers a prerender for a URL that never commits. const GURL kNoCommitUrl("http://never-respond.example.com"); - base::FilePath file(FILE_PATH_LITERAL( - "chrome/test/data/prerender/prerender_page.html")); + base::FilePath file(GetTestPath("prerender_page.html")); base::RunLoop prerender_start_loop; BrowserThread::PostTask( @@ -1734,8 +1820,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNoCommitNoSwap) { IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNoCommitNoSwap2) { // Navigate to a page that then navigates to a URL that never commits. const GURL kNoCommitUrl("http://never-respond.example.com"); - base::FilePath file(FILE_PATH_LITERAL( - "chrome/test/data/prerender/prerender_page.html")); + base::FilePath file(GetTestPath("prerender_page.html")); base::RunLoop prerender_start_loop; BrowserThread::PostTask( @@ -3409,8 +3494,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, std::string webstore_url = extension_urls::GetWebstoreLaunchURL(); // Mock out requests to the Web Store. - base::FilePath file(FILE_PATH_LITERAL( - "chrome/test/data/prerender/prerender_page.html")); + base::FilePath file(GetTestPath("prerender_page.html")); BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, base::Bind(&CreateMockProtocolHandlerOnIO, @@ -3578,16 +3662,81 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNewNavigationEntry) { // Attempt a swap-in in a new tab, verifying that session storage namespace // merging works. IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageNewTab) { - PrerenderTestURL("files/prerender/prerender_session_storage.html", - FINAL_STATUS_USED, 1); + // Mock out some URLs and count the number of requests to one of them. Both + // prerender_session_storage.html and init_session_storage.html need to be + // mocked so they are same-origin. + const GURL kInitURL("http://prerender.test/init_session_storage.html"); + base::FilePath init_file = GetTestPath("init_session_storage.html"); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&CreateMockProtocolHandlerOnIO, kInitURL, init_file)); + + const GURL kTestURL("http://prerender.test/prerender_session_storage.html"); + base::FilePath test_file = GetTestPath("prerender_session_storage.html"); + RequestCounter counter; + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&CreateCountingProtocolHandlerOnIO, + kTestURL, test_file, counter.AsWeakPtr())); + + PrerenderTestURL(kTestURL, FINAL_STATUS_USED, 1); // Open a new tab to navigate in. ui_test_utils::NavigateToURLWithDisposition( - current_browser(), - test_server()->GetURL("files/prerender/init_session_storage.html"), - NEW_FOREGROUND_TAB, + current_browser(), kInitURL, NEW_FOREGROUND_TAB, + ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); + + // Now navigate in the new tab. Set expect_swap_to_succeed to false because + // the swap does not occur synchronously. + // + // TODO(davidben): When all swaps become asynchronous, remove the OpenURL + // return value assertion and let this go through the usual successful-swap + // codepath. + NavigateToDestURLWithDisposition(CURRENT_TAB, false); + + // Verify DidDisplayPass manually since the previous call skipped it. + EXPECT_TRUE(DidDisplayPass( + current_browser()->tab_strip_model()->GetActiveWebContents())); + + // Only one request to the test URL started. + EXPECT_EQ(1, counter.count()); +} + +// Attempt a swap-in in a new tab, verifying that session storage namespace +// merging works. Unlike the above test, the swap is for a navigation that would +// normally be cross-process. +IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageNewTabCrossProcess) { + base::FilePath test_data_dir; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir)); + + // Mock out some URLs and count the number of requests to one of them. Both + // prerender_session_storage.html and init_session_storage.html need to be + // mocked so they are same-origin. + const GURL kInitURL("http://prerender.test/init_session_storage.html"); + base::FilePath init_file = GetTestPath("init_session_storage.html"); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&CreateMockProtocolHandlerOnIO, kInitURL, init_file)); + + const GURL kTestURL("http://prerender.test/prerender_session_storage.html"); + base::FilePath test_file = GetTestPath("prerender_session_storage.html"); + RequestCounter counter; + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&CreateCountingProtocolHandlerOnIO, + kTestURL, test_file, counter.AsWeakPtr())); + + PrerenderTestURL(kTestURL, FINAL_STATUS_USED, 1); + + // Open a new tab to navigate in. + ui_test_utils::NavigateToURLWithDisposition( + current_browser(), kInitURL, NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); + // Navigate to about:blank so the next navigation is cross-process. + ui_test_utils::NavigateToURL(current_browser(), + GURL(content::kAboutBlankURL)); + // Now navigate in the new tab. Set expect_swap_to_succeed to false because // the swap does not occur synchronously. // @@ -3599,6 +3748,9 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageNewTab) { // Verify DidDisplayPass manually since the previous call skipped it. EXPECT_TRUE(DidDisplayPass( current_browser()->tab_strip_model()->GetActiveWebContents())); + + // Only one request to the test URL started. + EXPECT_EQ(1, counter.count()); } // Verify that session storage conflicts don't merge. diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc index 6ba96bf..863160f 100644 --- a/chrome/browser/prerender/prerender_manager.cc +++ b/chrome/browser/prerender/prerender_manager.cc @@ -1127,20 +1127,14 @@ PrerenderManager::PendingSwap::PendingSwap( start_time_(base::TimeTicks::Now()), swap_successful_(false), weak_factory_(this) { - // Record the RFH id in the tracker to install throttles on MAIN_FRAME - // requests from that route. - int render_process_id = - target_contents->GetMainFrame()->GetProcess()->GetID(); - int render_frame_id = target_contents->GetMainFrame()->GetRoutingID(); - render_frame_route_id_pair_ = PrerenderTracker::ChildRouteIdPair( - render_process_id, render_frame_id); - manager_->prerender_tracker()->AddPrerenderPendingSwap( - render_frame_route_id_pair_, url_); + RenderViewCreated(target_contents->GetRenderViewHost()); } PrerenderManager::PendingSwap::~PendingSwap() { - manager_->prerender_tracker()->RemovePrerenderPendingSwap( - render_frame_route_id_pair_, swap_successful_); + for (size_t i = 0; i < main_rfh_ids_.size(); i++) { + manager_->prerender_tracker()->RemovePrerenderPendingSwap( + main_rfh_ids_[i], swap_successful_); + } } void PrerenderManager::PendingSwap::BeginSwap() { @@ -1185,6 +1179,20 @@ void PrerenderManager::PendingSwap::DidCommitProvisionalLoadForFrame( prerender_data_->ClearPendingSwap(); } +void PrerenderManager::PendingSwap::RenderViewCreated( + content::RenderViewHost* render_view_host) { + // Record the RFH id in the tracker to install throttles on MAIN_FRAME + // requests from that route. + int render_process_id = + render_view_host->GetMainFrame()->GetProcess()->GetID(); + int render_frame_id = render_view_host->GetMainFrame()->GetRoutingID(); + PrerenderTracker::ChildRouteIdPair render_frame_route_id_pair( + render_process_id, render_frame_id); + main_rfh_ids_.push_back(render_frame_route_id_pair); + manager_->prerender_tracker()->AddPrerenderPendingSwap( + render_frame_route_id_pair, url_); +} + void PrerenderManager::PendingSwap::DidFailProvisionalLoad( int64 frame_id, const base::string16& frame_unique_name, diff --git a/chrome/browser/prerender/prerender_manager.h b/chrome/browser/prerender/prerender_manager.h index c5c7d14c..ec2c05d 100644 --- a/chrome/browser/prerender/prerender_manager.h +++ b/chrome/browser/prerender/prerender_manager.h @@ -454,6 +454,8 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>, const GURL& validated_url, content::PageTransition transition_type, content::RenderViewHost* render_view_host) OVERRIDE; + virtual void RenderViewCreated( + content::RenderViewHost* render_view_host) OVERRIDE; virtual void DidFailProvisionalLoad( int64 frame_id, const base::string16& frame_unique_name, @@ -479,7 +481,7 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>, bool should_replace_current_entry_; base::TimeTicks start_time_; - PrerenderTracker::ChildRouteIdPair render_frame_route_id_pair_; + std::vector<PrerenderTracker::ChildRouteIdPair> main_rfh_ids_; base::OneShotTimer<PendingSwap> merge_timeout_; bool swap_successful_; diff --git a/chrome/browser/prerender/prerender_tracker.cc b/chrome/browser/prerender/prerender_tracker.cc index e41c81c..71798c9 100644 --- a/chrome/browser/prerender/prerender_tracker.cc +++ b/chrome/browser/prerender/prerender_tracker.cc @@ -118,6 +118,7 @@ void PrerenderTracker::AddPendingSwapThrottleOnIOThread( DCHECK(it != pending_swap_throttle_map_.end()); if (it == pending_swap_throttle_map_.end()) return; + CHECK(!it->second.throttle); it->second.throttle = throttle; } diff --git a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc index 794a2b0..eb5c884 100644 --- a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc +++ b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc @@ -469,7 +469,7 @@ void ChromeResourceDispatcherHostDelegate::AppendStandardResourceThrottles( request, prerender_tracker_)); } if (prerender_tracker_->IsPendingSwapRequestOnIOThread( - info->GetChildID(), info->GetRouteID(), request->url())) { + info->GetChildID(), info->GetRenderFrameID(), request->url())) { throttles->push_back(new prerender::PrerenderPendingSwapThrottle( request, prerender_tracker_)); } diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc index 22aa83e..b0796af 100644 --- a/content/browser/renderer_host/render_view_host_impl.cc +++ b/content/browser/renderer_host/render_view_host_impl.cc @@ -58,6 +58,7 @@ #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" +#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_widget_host_iterator.h" #include "content/public/browser/user_metrics.h" #include "content/public/common/bindings_policy.h" @@ -1059,6 +1060,10 @@ void RenderViewHostImpl::DragSourceSystemDragEnded() { Send(new DragMsg_SourceSystemDragEnded(GetRoutingID())); } +RenderFrameHost* RenderViewHostImpl::GetMainFrame() { + return RenderFrameHost::FromID(GetProcess()->GetID(), main_frame_routing_id_); +} + void RenderViewHostImpl::AllowBindings(int bindings_flags) { // Never grant any bindings to browser plugin guests. if (GetProcess()->IsGuest()) { diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h index d36e6a6..52f36ea 100644 --- a/content/browser/renderer_host/render_view_host_impl.h +++ b/content/browser/renderer_host/render_view_host_impl.h @@ -124,6 +124,7 @@ class CONTENT_EXPORT RenderViewHostImpl virtual ~RenderViewHostImpl(); // RenderViewHost implementation. + virtual RenderFrameHost* GetMainFrame() OVERRIDE; virtual void AllowBindings(int binding_flags) OVERRIDE; virtual void ClearFocusedNode() OVERRIDE; virtual void ClosePage() OVERRIDE; diff --git a/content/public/browser/render_view_host.h b/content/public/browser/render_view_host.h index a304a67..fb6e852 100644 --- a/content/public/browser/render_view_host.h +++ b/content/public/browser/render_view_host.h @@ -42,6 +42,7 @@ struct WebPluginAction; namespace content { class ChildProcessSecurityPolicy; +class RenderFrameHost; class RenderViewHostDelegate; class SessionStorageNamespace; class SiteInstance; @@ -71,6 +72,9 @@ class CONTENT_EXPORT RenderViewHost : virtual public RenderWidgetHost { virtual ~RenderViewHost() {} + // Returns the main frame for this render view. + virtual RenderFrameHost* GetMainFrame() = 0; + // Tell the render view to enable a set of javascript bindings. The argument // should be a combination of values from BindingsPolicy. virtual void AllowBindings(int binding_flags) = 0; |