Add SwappedOut to TabContentsDelegate to allow us to correctly delete TabContents.

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/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
+#include "base/memory/weak_ptr.h"
+#include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/prerender/prerender_condition.h"
 #include "chrome/browser/prerender/prerender_contents.h"
 #include "chrome/browser/prerender/prerender_field_trial.h"
@@ skipping 50 matching lines @@
   "OPTIONS",
   "POST",
   "TRACE",
 };
 
 // Length of prerender history, for display in chrome://net-internals
 const int kHistoryLength = 100;
 
 }  // namespace
 
-class PrerenderManager::OnCloseTabContentsDeleter : public TabContentsDelegate {
+class PrerenderManager::OnCloseTabContentsDeleter
+    : public TabContentsDelegate,
+      public base::SupportsWeakPtr<
+          PrerenderManager::OnCloseTabContentsDeleter> {
  public:
   OnCloseTabContentsDeleter(PrerenderManager* manager,
                             TabContentsWrapper* tab)
       : manager_(manager),
         tab_(tab) {
     tab_->tab_contents()->set_delegate(this);
+    MessageLoop::current()->PostDelayedTask(FROM_HERE,

[cbentzel, 2011/11/15 12:20:34]

Why is this needed? Do the other mechanisms not guarantee that CloseContents or
SwappedOut are called at the correct time?

+        base::Bind(&OnCloseTabContentsDeleter::ScheduleTabContentsForDeletion,

[cbentzel, 2011/11/15 12:20:34]

There's already some hang timers in RenderViewHost/RenderWidgetHost which will
eventually call TabContentsDelegate::CloseContents. Why is this timeout needed
as well? 

[dominich, 2011/11/15 15:33:28]

The hang timers didn't fire in this case as the RVH wasn't aware that the
TabContents should have closed. This timeout will catch any other edge cases
that we might miss, and the UMA stats should let us know if there's a
regression.

+                   this->AsWeakPtr(), true), kDeleteWithExtremePrejudiceTimeMs);
   }
 
   virtual void CloseContents(TabContents* source) OVERRIDE {
-    tab_->tab_contents()->set_delegate(NULL);
-    manager_->ScheduleDeleteOldTabContents(tab_.release(), this);
+    DCHECK_EQ(tab_->tab_contents(), source);
+    ScheduleTabContentsForDeletion(false);
+  }
+
+  virtual void SwappedOut(TabContents* source) OVERRIDE {
+    DCHECK_EQ(tab_->tab_contents(), source);
+    ScheduleTabContentsForDeletion(false);
   }
 
   virtual bool ShouldSuppressDialogs() OVERRIDE {
     return true;
   }
 
  private:
+  static const int kDeleteWithExtremePrejudiceTimeMs = 3000;
+
+  void ScheduleTabContentsForDeletion(bool timeout) {
+    tab_->tab_contents()->set_delegate(NULL);
+    manager_->ScheduleDeleteOldTabContents(tab_.release(), this);
+    UMA_HISTOGRAM_BOOLEAN("Prerender.TabContentsDeleterTimeout", timeout);
+  }
+
   PrerenderManager* manager_;
   scoped_ptr<TabContentsWrapper> tab_;
 
   DISALLOW_COPY_AND_ASSIGN(OnCloseTabContentsDeleter);
 };
 
 // static
 int PrerenderManager::prerenders_per_session_count_ = 0;
 
 // static
@@ skipping 948 matching lines @@
   if (!render_process_host || !render_process_host->browser_context())
     return NULL;
   Profile* profile = Profile::FromBrowserContext(
       render_process_host->browser_context());
   if (!profile)
     return NULL;
   return PrerenderManagerFactory::GetInstance()->GetForProfile(profile);
 }
 
 }  // namespace prerender