Fix issues when PrerenderContents::Destroy() is called during TabContents destruction.

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"
@@ skipping 119 matching lines @@
     : 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_been_cancelled_(false),
       child_id_(-1),
       route_id_(-1) {
   DCHECK(prerender_manager != NULL);
 }
 
 bool PrerenderContents::Init() {
   if (!AddAliasURL(prerender_url_))
     return false;
   return true;
 }
@@ skipping 207 matching lines @@
 
   final_status_ = final_status;
 }
 
 FinalStatus PrerenderContents::final_status() const {
   return final_status_;
 }
 
 PrerenderContents::~PrerenderContents() {
   DCHECK(final_status_ != FINAL_STATUS_MAX);
+  DCHECK(prerendering_has_been_cancelled_ ||
+         final_status_ == FINAL_STATUS_USED ||
+         final_status_ == FINAL_STATUS_CONTROL_GROUP);
 
   // 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_);
 
   // Only delete the RenderViewHost if we own it.
   if (render_view_host_)
     render_view_host_->Shutdown();
 
@@ skipping 326 matching lines @@
 
 void PrerenderContents::OnRenderViewGone(int status, int exit_code) {
   Destroy(FINAL_STATUS_RENDERER_CRASHED);
 }
 
 void PrerenderContents::DidStopLoading() {
   has_stopped_loading_ = true;
 }
 
 void PrerenderContents::Destroy(FinalStatus final_status) {
-  if (prerender_manager_->IsPendingDelete(this))
+  if (prerendering_has_been_cancelled_)
     return;
 
+  prerendering_has_been_cancelled_ = true;
+  prerender_manager_->MoveEntryToPendingDelete(this);

[cbentzel, 2011/05/23 18:51:28]

Is there a reason MoveEntryToPendingDelete moved up here?

[mmenke, 2011/05/23 18:55:36]

Yes, it's just for style - prerender_has_been_cancelled_ is closely associated
with being queued for deletion, so it makes the most sense to me to set
prerender_has_been_cancelled_ at the same time we queue the deletion.  Also
makes sense to set prerender_has_been_cancelled_ to true right after checking if
it's already true.

Of course, the same argument could be made for setting final_status as well.

+
   if (child_id_ != -1 && route_id_ != -1) {
     // 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);
     CHECK(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();
     }
   }
+  set_final_status(final_status);
 
-  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,
                                              bool is_during_unload) {
   DCHECK_EQ(render_view_host_, render_view_host);
@@ skipping 76 matching lines @@
   }
   return render_view_host_;
 }
 
 void PrerenderContents::CommitHistory(TabContents* tc) {
   if (tab_contents_delegate_.get())
     tab_contents_delegate_->CommitHistory(tc);
 }
 
 }  // namespace prerender