Don't terminate the process when a tab becomes unresponsive during

unload/beforeunload. Instead, just call close on it. If two tabs are in the
same process, then terminating the process is totally wrong.
This also avoids the bugs where we show sad tab, or don't remove the
tab from the tabstrip.

Also, remove a couple of bogus DCHECKS.

BUG=1314995,1301757

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@685 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 14 insertions, 24 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 472fdd9..a2b7928 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -680,7 +680,7 @@ void Browser::ReplaceContents(TabContents* source, TabContents* new_contents) {
     // Need to do this asynchronously as it will close the tab, which is
     // currently on the call stack above us.
     MessageLoop::current()->PostTask(FROM_HERE,
-        method_factory_.NewRunnableMethod(&Browser::ClearUnloadStateOnCrash,
+        method_factory_.NewRunnableMethod(&Browser::ClearUnloadState,
         Source<TabContents>(source).ptr()));
   }
   // Need to remove ourselves as an observer for disconnection on the replaced
@@ -790,7 +790,7 @@ void Browser::CloseContents(TabContents* source) {
     // waiting for unload to fire. Don't actually try to close the tab as it 
     // will go down the slow shutdown path instead of the fast path of killing
     // all the renderer processes.
-    UnloadFired(source);
+    ClearUnloadState(source);
     return;
   }
 
@@ -847,7 +847,7 @@ void Browser::Observe(NotificationType type,
       // Need to do this asynchronously as it will close the tab, which is
       // currently on the call stack above us.
       MessageLoop::current()->PostTask(FROM_HERE,
-          method_factory_.NewRunnableMethod(&Browser::ClearUnloadStateOnCrash,
+          method_factory_.NewRunnableMethod(&Browser::ClearUnloadState,
               Source<TabContents>(source).ptr()));
     }
   } else {
@@ -1147,13 +1147,7 @@ void Browser::BeforeUnloadFired(TabContents* tab,
   *proceed_to_fire_unload = true;
 }
 
-void Browser::UnloadFired(TabContents* tab) {
-  DCHECK(is_attempting_to_close_browser_);
-  RemoveFromVector(&tabs_needing_unload_fired_, tab);
-  ProcessPendingTabs();
-}
-
-void Browser::ClearUnloadStateOnCrash(TabContents* tab) {
+void Browser::ClearUnloadState(TabContents* tab) {
   DCHECK(is_attempting_to_close_browser_);
   RemoveFromVector(&tabs_needing_before_unload_fired_, tab);
   RemoveFromVector(&tabs_needing_unload_fired_, tab);
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 42018257..35ccb37 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -194,9 +194,6 @@ class Browser : public TabStripModelDelegate,
                          bool proceed, 
                          bool* proceed_to_fire_unload);
 
-  // Tells us that we've finished firing this tab's unload event.
-  void UnloadFired(TabContents* source);
-
   // Invoked when the window containing us is closing. Performs the necessary
   // cleanup.
   void OnWindowClosing();
@@ -548,9 +545,11 @@ class Browser : public TabStripModelDelegate,
   // was in the vector in the first place.
   bool RemoveFromVector(UnloadListenerVector* vector, TabContents* tab);
 
-  // Cleans up state appropriately when we are trying to close the browser
-  // and a tab crashes in it's beforeunload/unload handler.
-  void ClearUnloadStateOnCrash(TabContents* tab);
+  // Cleans up state appropriately when we are trying to close the browser and
+  // the tab has finished firing it's unload handler. We also use this in the 
+  // cases where a tab crashes or hangs even if the beforeunload/unload haven't
+  // successfully fired.
+  void ClearUnloadState(TabContents* tab);
 
   // The frame
   BrowserWindow* window_;
diff --git a/chrome/browser/render_view_host.cc b/chrome/browser/render_view_host.cc
index ba466a8..ec016a2 100644
--- a/chrome/browser/render_view_host.cc
+++ b/chrome/browser/render_view_host.cc
@@ -248,7 +248,6 @@ void RenderViewHost::FirePageBeforeUnload() {
   if (IsRenderViewLive()) {
     // Start the hang monitor in case the renderer hangs in the beforeunload
     // handler.
-    DCHECK(!is_waiting_for_unload_ack_);
     is_waiting_for_unload_ack_ = true;
     StartHangMonitorTimeout(kUnloadTimeoutMS);
     Send(new ViewMsg_ShouldClose(routing_id_));
@@ -261,7 +260,6 @@ void RenderViewHost::FirePageBeforeUnload() {
 
 void RenderViewHost::FirePageUnload() {
   // Start the hang monitor in case the renderer hangs in the unload handler.
-  DCHECK(!is_waiting_for_unload_ack_);
   is_waiting_for_unload_ack_ = true;
   StartHangMonitorTimeout(kUnloadTimeoutMS);
   ClosePage(site_instance()->process_host_id(), 
@@ -1164,9 +1162,11 @@ void RenderViewHost::OnUnloadListenerChanged(bool has_listener) {
 void RenderViewHost::NotifyRendererUnresponsive() {
   if (is_waiting_for_unload_ack_) {
     // If the tab hangs in the beforeunload/unload handler there's really
-    // nothing we can do to recover. We can safely kill the process and the
-    // Browser will deal with the crash appropriately.
-    TerminateProcess(process()->process(), ResultCodes::HUNG);
+    // nothing we can do to recover. Pretend the unload listeners have
+    // all fired and close the tab. If the hang is in the beforeunload handler
+    // then the user will not have the option of cancelling the close.
+    UnloadListenerHasFired();
+    delegate_->Close(this);
     return;
   }
 
diff --git a/chrome/browser/tab_contents_delegate.h b/chrome/browser/tab_contents_delegate.h
index 2dd077c7d..30d6cbc 100644
--- a/chrome/browser/tab_contents_delegate.h
+++ b/chrome/browser/tab_contents_delegate.h
@@ -190,9 +190,6 @@ class TabContentsDelegate : public PageNavigator {
                                  bool* proceed_to_fire_unload) { 
     *proceed_to_fire_unload = true;
   }
-
-  // Tells us that we've finished firing this tab's unload event.
-  virtual void UnloadFired(TabContents* tab) {}
 };
 
 #endif  // CHROME_BROWSER_TAB_CONTENTS_DELEGATE_H__