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

chrome/browser/prerender/prerender_manager.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_manager.h"
 
 #include <string>
 
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
@@ skipping 266 matching lines @@
     // Don't prerender page if parent RenderViewHost no longer exists, or it has
     // no view.  The latter should only happen when the RenderView has closed.
     if (!source_render_view_host || !source_render_view_host->view()) {
       RecordFinalStatus(FINAL_STATUS_SOURCE_RENDER_VIEW_CLOSED);
       return false;
     }
   }
 
   PrerenderContents* prerender_contents =
       CreatePrerenderContents(url, referrer);
-  if (!prerender_contents || !prerender_contents->Init())
+  if (!prerender_contents)
     return false;
 
   // TODO(cbentzel): Move invalid checks here instead of PrerenderContents?
   PrerenderContentsData data(prerender_contents, GetCurrentTime());
+  prerender_list_.push_back(data);
 
-  prerender_list_.push_back(data);
+  if (!prerender_contents->Init())
+    return false;
+
   if (IsControlGroup()) {
     data.contents_->set_final_status(FINAL_STATUS_CONTROL_GROUP);
   } else {
     last_prerender_start_time_ = GetCurrentTimeTicks();
     data.contents_->StartPrerendering(source_render_view_host);
   }
   while (prerender_list_.size() > max_elements_) {
     data = prerender_list_.front();
     prerender_list_.pop_front();
     data.contents_->Destroy(FINAL_STATUS_EVICTED);
@@ skipping 263 matching lines @@
     }
     pending_prerender_list_.erase(pending_it);
   }
 
   old_tab_contents_list_.push_back(old_tab_contents);
   // Destroy the old TabContents relatively promptly to reduce resource usage.
   PostCleanupTask();
   return true;
 }
 
-void PrerenderManager::MoveEntryToPendingDelete(PrerenderContents* entry) {
+bool PrerenderManager::MoveEntryToPendingDelete(PrerenderContents* entry) {
   DCHECK(CalledOnValidThread());
   DCHECK(!IsPendingDelete(entry));
   for (std::list<PrerenderContentsData>::iterator it = prerender_list_.begin();
        it != prerender_list_.end();
        ++it) {
     if (it->contents_ == entry) {
       RemovePendingPreload(entry);
       prerender_list_.erase(it);
-      break;
+      pending_delete_list_.push_back(entry);

[Shishir, 2011/05/18 18:34:32]

I had initially pushed this outside the loop because:
1. Having it here causes leaks in prerender_manager unittest.
2. It is incompatible with the change I am making in window.opener where I need
to destroy an entry after I call GetEntry() which removes the entry from the
prerender_list. Please look at
http://codereview.chromium.org/6915019/patch/37001/38005

[mmenke, 2011/05/18 18:50:37]

Ok...  Seems like the simplest way to do this robustly, then, is to just have
the PrerenderContents itself track whether it's being destroyed, and have it
delete its TabContents (if any) explicitly in its destructor.  The latter may
not cause issues, but I prefer not to get receive any callbacks while implicitly
destroying class members.

We could stop watching everything instead, but I'd be concerned about
regressions if we did that.

+
+      // Destroy the old TabContents relatively promptly to reduce resource
+      // usage, and in the case of HTML5 media, reduce the chance of playing
+      // any sound.
+      PostCleanupTask();
+
+      return true;
     }
   }
-  pending_delete_list_.push_back(entry);
-
-  // Destroy the old TabContents relatively promptly to reduce resource usage,
-  // and in the case of HTML5 media, reduce the change of playing any sound.
-  PostCleanupTask();
+  return false;
 }
 
 bool PrerenderManager::IsPendingDelete(PrerenderContents* entry) const {
   DCHECK(CalledOnValidThread());
   for (std::list<PrerenderContents*>::const_iterator it =
           pending_delete_list_.begin();
        it != pending_delete_list_.end();
        ++it) {
     if (*it == entry)
       return true;
@@ skipping 308 matching lines @@
   return prerendered_tab_contents_set_.count(tab_contents) > 0;
 }
 
 bool PrerenderManager::WouldTabContentsBePrerendered(
     TabContents* tab_contents) const {
   DCHECK(CalledOnValidThread());
   return would_be_prerendered_tab_contents_set_.count(tab_contents) > 0;
 }
 
 }  // namespace prerender