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

chrome/browser/prerender/prerender_manager.cc

Index: chrome/browser/prerender/prerender_manager.cc
===================================================================
--- chrome/browser/prerender/prerender_manager.cc	(revision 85626)
+++ chrome/browser/prerender/prerender_manager.cc	(working copy)
@@ -284,13 +284,16 @@
 
   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 {
@@ -574,7 +577,7 @@
   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();
@@ -583,14 +586,17 @@
     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 {