Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(849)

Unified Diff: chrome/browser/prerender/prerender_contents.cc

Issue 7038012: Safely cancel prerenders on threads other than the UI thread (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Fix linux Created 9 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/prerender/prerender_contents.cc
===================================================================
--- chrome/browser/prerender/prerender_contents.cc (revision 85771)
+++ chrome/browser/prerender/prerender_contents.cc (working copy)
@@ -15,6 +15,7 @@
#include "chrome/browser/prerender/prerender_final_status.h"
#include "chrome/browser/prerender/prerender_manager.h"
#include "chrome/browser/prerender/prerender_render_widget_host_view.h"
+#include "chrome/browser/prerender/prerender_tracker.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/renderer_preferences_util.h"
#include "chrome/browser/ui/login/login_prompt.h"
@@ -59,18 +60,6 @@
GURL url_;
};
-void AddChildRoutePair(ResourceDispatcherHost* resource_dispatcher_host,
- int child_id, int route_id) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- resource_dispatcher_host->AddPrerenderChildRoutePair(child_id, route_id);
-}
-
-void RemoveChildRoutePair(ResourceDispatcherHost* resource_dispatcher_host,
- int child_id, int route_id) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- resource_dispatcher_host->RemovePrerenderChildRoutePair(child_id, route_id);
-}
-
} // end namespace
class PrerenderContentsFactoryImpl : public PrerenderContents::Factory {
@@ -95,7 +84,9 @@
ALLOW_THIS_IN_INITIALIZER_LIST(tab_contents_observer_registrar_(this)),
has_stopped_loading_(false),
final_status_(FINAL_STATUS_MAX),
- prerendering_has_started_(false) {
+ prerendering_has_started_(false),
+ child_id_(-1),
+ route_id_(-1) {
DCHECK(prerender_manager != NULL);
}
@@ -136,18 +127,14 @@
new PrerenderRenderWidgetHostView(render_view_host_, this);
view->Init(source_render_view_host->view());
- // Register this with the ResourceDispatcherHost as a prerender
- // RenderViewHost. This must be done before the Navigate message to catch all
- // resource requests, but as it is on the same thread as the Navigate message
- // (IO) there is no race condition.
- int process_id = render_view_host_->process()->id();
- int view_id = render_view_host_->routing_id();
+ child_id_ = render_view_host_->process()->id();
+ route_id_ = render_view_host_->routing_id();
- BrowserThread::PostTask(
- BrowserThread::IO, FROM_HERE,
- NewRunnableFunction(&AddChildRoutePair,
- g_browser_process->resource_dispatcher_host(),
- process_id, view_id));
+ // Register this with the PrerenderTracker as a prerendering RenderViewHost.
+ // This must be done before the Navigate message to catch all resource
+ // requests.
+ PrerenderTracker::GetInstance()->OnPrerenderingStarted(child_id_, route_id_,
+ prerender_manager_);
// Close ourselves when the application is shutting down.
notification_registrar_.Add(this, NotificationType::APP_TERMINATING,
@@ -238,20 +225,15 @@
render_view_host_observer_.reset(
new PrerenderRenderViewHostObserver(this, render_view_host_mutable()));
- int process_id;
- int view_id;
- CHECK(GetChildId(&process_id));
- CHECK(GetRouteId(&view_id));
+ child_id_ = render_view_host()->process()->id();
+ route_id_ = render_view_host()->routing_id();
// Register this with the ResourceDispatcherHost as a prerender
// RenderViewHost. This must be done before the Navigate message to catch all
// resource requests, but as it is on the same thread as the Navigate message
// (IO) there is no race condition.
- BrowserThread::PostTask(
- BrowserThread::IO, FROM_HERE,
- NewRunnableFunction(&AddChildRoutePair,
- g_browser_process->resource_dispatcher_host(),
- process_id, view_id));
+ PrerenderTracker::GetInstance()->OnPrerenderingStarted(child_id_, route_id_,
+ prerender_manager_);
// Close ourselves when the application is shutting down.
notification_registrar_.Add(this, NotificationType::APP_TERMINATING,
@@ -299,22 +281,14 @@
bool PrerenderContents::GetChildId(int* child_id) const {
CHECK(child_id);
- const RenderViewHost* prerender_render_view_host = render_view_host();
- if (prerender_render_view_host) {
- *child_id = prerender_render_view_host->process()->id();
- return true;
- }
- return false;
+ *child_id = child_id_;
+ return child_id_ >= 0;
dominich 2011/05/19 15:49:33 This should be a != -1 as you never expect a value
mmenke 2011/05/19 16:40:42 Done (for all the checks in this file, also update
}
bool PrerenderContents::GetRouteId(int* route_id) const {
CHECK(route_id);
- const RenderViewHost* prerender_render_view_host = render_view_host();
- if (prerender_render_view_host) {
- *route_id = prerender_render_view_host->routing_id();
- return true;
- }
- return false;
+ *route_id = route_id_;
+ return route_id_ >= 0;
dominich 2011/05/19 15:49:33 See above comment re != -1.
}
void PrerenderContents::set_final_status(FinalStatus final_status) {
@@ -343,21 +317,15 @@
RecordFinalStatus(final_status_);
if (render_view_host_ || prerender_contents_.get()) {
- RenderViewHost* prerender_render_view_host = render_view_host_mutable();
-
- int process_id = prerender_render_view_host->process()->id();
- int view_id = prerender_render_view_host->routing_id();
-
- BrowserThread::PostTask(
- BrowserThread::IO, FROM_HERE,
- NewRunnableFunction(&RemoveChildRoutePair,
- g_browser_process->resource_dispatcher_host(),
- process_id, view_id));
-
// Only delete the RenderViewHost if we own it.
if (render_view_host_)
render_view_host_->Shutdown();
}
+
+ if (child_id_ >= 0 && route_id_ >= 0) {
+ PrerenderTracker::GetInstance()->OnPrerenderingFinished(
+ child_id_, route_id_);
+ }
}
RenderViewHostDelegate::View* PrerenderContents::GetViewDelegate() {
@@ -681,6 +649,24 @@
if (prerender_manager_->IsPendingDelete(this))
return;
+ if (child_id_ >= 0 && route_id_ >= 0) {
+ // Cancel the prerender in the PrerenderTracker. This is needed
+ // because destroy may be called directly from the UI thread without calling
+ // TryCancel(). This is difficult to completely avoid, since prerendering
+ // can be cancelled before a RenderView is created.
+ bool is_cancelled =
+ PrerenderTracker::GetInstance()->TryCancel(child_id_, route_id_,
+ final_status);
+ DCHECK(is_cancelled);
+
+ // A different final status may have been set already from another thread.
+ // If so, use it instead.
+ if (!PrerenderTracker::GetInstance()->GetFinalStatus(child_id_, route_id_,
+ &final_status)) {
+ NOTREACHED();
dominich 2011/05/19 15:49:33 Can this not be CHECK(PrerenderTracker::GetInstanc
mmenke 2011/05/19 16:40:42 A CHECK results in a crash in release builds, whic
dominich 2011/05/19 16:55:47 I thought NOTREACHED was a CHECK rather than a DCH
+ }
+ }
+
prerender_manager_->MoveEntryToPendingDelete(this);
set_final_status(final_status);
// We may destroy the PrerenderContents before we have initialized the

Powered by Google App Engine
This is Rietveld 408576698