Safely cancel prerenders on threads other than the UI thread

chrome/browser/prerender/prerender_contents.cc

 // Copyright (c) 2011 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 "chrome/browser/prerender/prerender_contents.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/process_util.h"
 #include "base/task.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/background_contents_service.h"
 #include "chrome/browser/browser_process.h"
 #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"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "chrome/common/icon_messages.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "content/browser/browsing_instance.h"
@@ skipping 24 matching lines @@
   bool operator()(const GURL& url) const {
     return url.scheme() == url_.scheme() &&
            url.host() == url_.host() &&
            url.port() == url_.port() &&
            url.path() == url_.path() &&
            url.query() == url_.query();
   }
   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 {
  public:
   virtual PrerenderContents* CreatePrerenderContents(
       PrerenderManager* prerender_manager, Profile* profile, const GURL& url,
       const GURL& referrer) OVERRIDE {
     return new PrerenderContents(prerender_manager, profile, url, referrer);
   }
 };
 
 PrerenderContents::PrerenderContents(PrerenderManager* prerender_manager,
                                      Profile* profile,
                                      const GURL& url,
                                      const GURL& referrer)
     : prerender_manager_(prerender_manager),
       render_view_host_(NULL),
       prerender_url_(url),
       referrer_(referrer),
       profile_(profile),
       page_id_(0),
       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);
 }
 
 bool PrerenderContents::Init() {
   if (!AddAliasURL(prerender_url_))
     return false;
   return true;
 }
 
 // static
@@ skipping 20 matching lines @@
   render_view_host_->CreateRenderView(string16());
 
   OnRenderViewHostCreated(render_view_host_);
 
   // Give the RVH a PrerenderRenderWidgetHostView, both so its size can be set
   // and so that the prerender can be cancelled under certain circumstances.
   PrerenderRenderWidgetHostView* view =
       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,
                               NotificationService::AllSources());
 
   // Register for our parent profile to shutdown, so we can shut ourselves down
   // as well (should only be called for OTR profiles, as we should receive
   // APP_TERMINATING before non-OTR profiles are destroyed).
   // TODO(tburkard): figure out if this is needed.
   notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
@@ skipping 70 matching lines @@
     // Set the size of the new TC to that of the old TC.
     gfx::Rect tab_bounds;
     source_tc->view()->GetContainerBounds(&tab_bounds);
     prerender_contents_->view()->SizeContents(tab_bounds.size());
   }
 
   // Register as an observer of the RenderViewHost so we get messages.
   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,
                               NotificationService::AllSources());
 
   // Register for our parent profile to shutdown, so we can shut ourselves down
   // as well (should only be called for OTR profiles, as we should receive
   // APP_TERMINATING before non-OTR profiles are destroyed).
   // TODO(tburkard): figure out if this is needed.
   notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
@@ skipping 27 matching lines @@
 
   DCHECK(load_start_time_.is_null());
   load_start_time_ = base::TimeTicks::Now();
 
   new_contents->controller().LoadURL(prerender_url_,
                                      referrer_, PageTransition::LINK);
 }
 
 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 < -1. You could DCHECK that
too.

[mmenke, 2011/05/19 16:40:42]

Done (for all the checks in this file, also updated the tracker's corresponding
DCHECKs)

 }
 
 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) {
   DCHECK(final_status >= FINAL_STATUS_USED && final_status < FINAL_STATUS_MAX);
   DCHECK(final_status_ == FINAL_STATUS_MAX ||
          final_status_ == FINAL_STATUS_CONTROL_GROUP);
 
   // Don't override final_status_ if it's FINAL_STATUS_CONTROL_GROUP,
   // otherwise data will be collected in the Prerender.FinalStatus histogram.
   if (final_status_ == FINAL_STATUS_CONTROL_GROUP)
     return;
 
   final_status_ = final_status;
 }
 
 FinalStatus PrerenderContents::final_status() const {
   return final_status_;
 }
 
 PrerenderContents::~PrerenderContents() {
   DCHECK(final_status_ != FINAL_STATUS_MAX);
 
   // If we haven't even started prerendering, we were just in the control
   // group, which means we do not want to record the status.
   if (prerendering_has_started())
     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() {
   return this;
 }
 
 const GURL& PrerenderContents::GetURL() const {
   return url_;
 }
 
@@ skipping 303 matching lines @@
 }
 
 void PrerenderContents::DidStopLoading() {
   has_stopped_loading_ = true;
 }
 
 void PrerenderContents::Destroy(FinalStatus final_status) {
   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::GetInstance()->GetFinalStatus(...)) ?

[mmenke, 2011/05/19 16:40:42]

A CHECK results in a crash in release builds, which I prefer to avoid unless
we're about to crash anyways.

[dominich, 2011/05/19 16:55:47]

I thought NOTREACHED was a CHECK rather than a DCHECK. My bad.

+    }
+  }
+
   prerender_manager_->MoveEntryToPendingDelete(this);
   set_final_status(final_status);
   // We may destroy the PrerenderContents before we have initialized the
   // RenderViewHost. Otherwise set the Observer's PrerenderContents to NULL to
   // avoid any more messages being sent.
   if (render_view_host_observer_.get())
     render_view_host_observer_->set_prerender_contents(NULL);
 }
 
 void PrerenderContents::RendererUnresponsive(RenderViewHost* render_view_host,
@@ skipping 60 matching lines @@
   //               used.
   if (UseTabContents()) {
     if (!prerender_contents_.get())
       return NULL;
     return prerender_contents_->render_view_host();
   }
   return render_view_host_;
 }
 
 }  // namespace prerender