Fix sudden termination after the latest WebKit merge.

BUG=10927

Review URL: http://codereview.chromium.org/93104

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

11 files changed, 99 insertions, 54 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 2c7a637..2b81a0bb 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -159,7 +159,7 @@ bool TabHasUnloadListener(TabContents* contents) {
   WebContents* web_contents = contents->AsWebContents();
   return web_contents && web_contents->notify_disconnection() &&
       !web_contents->showing_interstitial_page() &&
-      web_contents->render_view_host()->HasUnloadListener();
+      !web_contents->render_view_host()->SuddenTerminationAllowed();
 }
 
 }  // namespace
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index a75d9de..824f5a9 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -480,7 +480,16 @@ bool BrowserRenderProcessHost::FastShutdownIfPossible() {
   if (BrowserRenderProcessHost::run_renderer_in_process())
     return false;  // Since process mode can't do fast shutdown.
 
-  // Test if there's an unload listener
+  // Test if there's an unload listener.
+  // NOTE: It's possible that an onunload listener may be installed
+  // while we're shutting down, so there's a small race here.  Given that
+  // the window is small, it's unlikely that the web page has much
+  // state that will be lost by not calling its unload handlers properly.
+  if (!sudden_termination_allowed())
+    return false;
+
+  // Check for any external tab containers, since they may still be running even
+  // though this window closed.
   BrowserRenderProcessHost::listeners_iterator iter;
   // NOTE: This is a bit dangerous.  We know that for now, listeners are
   // always RenderWidgetHosts.  But in theory, they don't have to be.
@@ -490,13 +499,8 @@ bool BrowserRenderProcessHost::FastShutdownIfPossible() {
     if (!widget || !widget->IsRenderView())
       continue;
     RenderViewHost* rvh = static_cast<RenderViewHost*>(widget);
-    if (!rvh->CanTerminate()) {
-      // NOTE: It's possible that an onunload listener may be installed
-      // while we're shutting down, so there's a small race here.  Given that
-      // the window is small, it's unlikely that the web page has much
-      // state that will be lost by not calling its unload handlers properly.
+    if (rvh->delegate()->IsExternalTabContainer())
       return false;
-    }
   }
 
   // Otherwise, we're allowed to just terminate the process. Using exit code 0
@@ -582,6 +586,8 @@ void BrowserRenderProcessHost::OnMessageReceived(const IPC::Message& msg) {
       IPC_MESSAGE_HANDLER(ViewHostMsg_PageContents, OnPageContents)
       IPC_MESSAGE_HANDLER(ViewHostMsg_UpdatedCacheStats,
                           OnUpdatedCacheStats)
+      IPC_MESSAGE_HANDLER(ViewHostMsg_SuddenTerminationChanged,
+                          SuddenTerminationChanged);
       IPC_MESSAGE_UNHANDLED_ERROR()
     IPC_END_MESSAGE_MAP_EX()
 
@@ -702,6 +708,10 @@ void BrowserRenderProcessHost::OnUpdatedCacheStats(
   WebCacheManager::GetInstance()->ObserveStats(pid(), stats);
 }
 
+void BrowserRenderProcessHost::SuddenTerminationChanged(bool enabled) {
+  set_sudden_termination_allowed(enabled);
+}
+
 void BrowserRenderProcessHost::SetBackgrounded(bool backgrounded) {
   // If the process_ is NULL, the process hasn't been created yet.
   if (process_.handle()) {
diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h
index c6517b8..1914432 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.h
+++ b/chrome/browser/renderer_host/browser_render_process_host.h
@@ -90,16 +90,8 @@ class BrowserRenderProcessHost : public RenderProcessHost,
   // Control message handlers.
   void OnPageContents(const GURL& url, int32 page_id,
                       const std::wstring& contents);
-  // Clipboard messages
-  void OnClipboardWriteHTML(const std::wstring& markup, const GURL& src_url);
-  void OnClipboardWriteBookmark(const std::wstring& title, const GURL& url);
-  void OnClipboardWriteBitmap(base::SharedMemoryHandle bitmap, gfx::Size size);
-  void OnClipboardIsFormatAvailable(unsigned int format, bool* result);
-  void OnClipboardReadText(string16* result);
-  void OnClipboardReadAsciiText(std::string* result);
-  void OnClipboardReadHTML(string16* markup, GURL* src_url);
-
   void OnUpdatedCacheStats(const WebKit::WebCache::UsageStats& stats);
+  void SuddenTerminationChanged(bool enabled);
 
   // Initialize support for visited links. Send the renderer process its initial
   // set of visited links.
diff --git a/chrome/browser/renderer_host/render_process_host.cc b/chrome/browser/renderer_host/render_process_host.cc
index 0fdde82..e70596743 100644
--- a/chrome/browser/renderer_host/render_process_host.cc
+++ b/chrome/browser/renderer_host/render_process_host.cc
@@ -66,7 +66,8 @@ bool RenderProcessHost::run_renderer_in_process_ = false;
 RenderProcessHost::RenderProcessHost(Profile* profile)
     : max_page_id_(-1),
       pid_(-1),
-      profile_(profile) {
+      profile_(profile),
+      sudden_termination_allowed_(true) {
 }
 
 RenderProcessHost::~RenderProcessHost() {
diff --git a/chrome/browser/renderer_host/render_process_host.h b/chrome/browser/renderer_host/render_process_host.h
index f972d39..0347427 100644
--- a/chrome/browser/renderer_host/render_process_host.h
+++ b/chrome/browser/renderer_host/render_process_host.h
@@ -46,6 +46,13 @@ class RenderProcessHost : public IPC::Channel::Sender,
   // May return NULL if there is no connection.
   IPC::SyncChannel* channel() { return channel_.get(); }
 
+  bool sudden_termination_allowed() const {
+    return sudden_termination_allowed_;
+  }
+  void set_sudden_termination_allowed(bool enabled) {
+    sudden_termination_allowed_ = enabled;
+  }
+
   // Used for refcounting, each holder of this object must Attach and Release
   // just like it would for a COM object. This object should be allocated on
   // the heap; when no listeners own it any more, it will delete itself.
@@ -196,6 +203,14 @@ class RenderProcessHost : public IPC::Channel::Sender,
   // set of listeners that expect the renderer process to close
   std::set<int> listeners_expecting_close_;
 
+  // True if the process can be shut down suddenly.  If this is true, then we're
+  // sure that all the RenderViews in the process can be shutdown suddenly.  If
+  // it's false, then specific RenderViews might still be allowed to be shutdown
+  // suddenly by checking their SuddenTerminationAllowed() flag.  This can occur
+  // if one tab has an unload event listener but another tab in the same process
+  // doesn't.
+  bool sudden_termination_allowed_;
+
   // See getter above.
   static bool run_renderer_in_process_;
 
diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index 73d888d6..21b8d34 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -100,9 +100,9 @@ RenderViewHost::RenderViewHost(SiteInstance* instance,
       navigations_suspended_(false),
       suspended_nav_message_(NULL),
       run_modal_reply_msg_(NULL),
-      has_unload_listener_(false),
       is_waiting_for_unload_ack_(false),
-      are_javascript_messages_suppressed_(false) {
+      are_javascript_messages_suppressed_(false),
+      sudden_termination_allowed_(false) {
   DCHECK(instance_);
   DCHECK(delegate_);
   if (modal_dialog_event == NULL)
@@ -285,7 +285,7 @@ void RenderViewHost::ClosePageIgnoringUnloadEvents(int render_process_host_id,
   rvh->StopHangMonitorTimeout();
   rvh->is_waiting_for_unload_ack_ = false;
 
-  rvh->UnloadListenerHasFired();
+  rvh->set_sudden_termination_allowed(true);
   rvh->delegate()->Close(rvh);
 }
 
@@ -658,11 +658,8 @@ void RenderViewHost::LoadStateChanged(const GURL& url,
   delegate_->LoadStateChanged(url, load_state);
 }
 
-bool RenderViewHost::CanTerminate() const {
-  if (!delegate_->CanTerminate())
-    return false;
-
-  return !has_unload_listener_;
+bool RenderViewHost::SuddenTerminationAllowed() const {
+  return sudden_termination_allowed_ || process()->sudden_termination_allowed();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -761,8 +758,6 @@ void RenderViewHost::OnMessageReceived(const IPC::Message& msg) {
     IPC_MESSAGE_FORWARD(ViewHostMsg_JSOutOfMemory, delegate_,
                         RenderViewHostDelegate::OnJSOutOfMemory);
     IPC_MESSAGE_HANDLER(ViewHostMsg_ShouldClose_ACK, OnMsgShouldCloseACK);
-    IPC_MESSAGE_HANDLER(ViewHostMsg_UnloadListenerChanged,
-                        OnUnloadListenerChanged);
     IPC_MESSAGE_HANDLER(ViewHostMsg_QueryFormFieldAutofill,
                         OnQueryFormFieldAutofill)
     IPC_MESSAGE_HANDLER(ViewHostMsg_RemoveAutofillEntry,
@@ -1298,10 +1293,6 @@ void RenderViewHost::OnMsgShouldCloseACK(bool proceed) {
   delegate_->ShouldClosePage(proceed);
 }
 
-void RenderViewHost::OnUnloadListenerChanged(bool has_listener) {
-  has_unload_listener_ = has_listener;
-}
-
 void RenderViewHost::OnQueryFormFieldAutofill(const std::wstring& field_name,
                                               const std::wstring& user_text,
                                               int64 node_id,
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index 635f587..f15131c 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -377,15 +377,10 @@ class RenderViewHost : public RenderWidgetHost {
   // Notifies the RenderViewHost that its load state changed.
   void LoadStateChanged(const GURL& url, net::LoadState load_state);
 
-  // Does the associated view have an onunload or onbeforeunload handler?
-  bool HasUnloadListener() { return has_unload_listener_; }
-
-  // If the associated view can be terminated without any side effects
-  bool CanTerminate() const;
-
-  // Clears the has_unload_listener_ bit since the unload handler has fired
-  // and we're necessarily leaving the page.
-  void UnloadListenerHasFired() { has_unload_listener_ = false; }
+  bool SuddenTerminationAllowed() const;
+  void set_sudden_termination_allowed(bool enabled) {
+    sudden_termination_allowed_ = enabled;
+  }
 
   // Forward a message from external host to chrome renderer.
   void ForwardMessageFromExternalHost(const std::string& message,
@@ -539,7 +534,6 @@ class RenderViewHost : public RenderWidgetHost {
   void OnDidGetApplicationInfo(int32 page_id,
                                const webkit_glue::WebApplicationInfo& info);
   void OnMsgShouldCloseACK(bool proceed);
-  void OnUnloadListenerChanged(bool has_handler);
   void OnQueryFormFieldAutofill(const std::wstring& field_name,
                                 const std::wstring& user_text,
                                 int64 node_id,
@@ -613,8 +607,6 @@ class RenderViewHost : public RenderWidgetHost {
   // must return to the renderer to unblock it.
   IPC::Message* run_modal_reply_msg_;
 
-  bool has_unload_listener_;
-
   bool is_waiting_for_unload_ack_;
 
   bool are_javascript_messages_suppressed_;
@@ -622,6 +614,9 @@ class RenderViewHost : public RenderWidgetHost {
   // Handles processing IPC messages request extension functions be executed.
   scoped_ptr<ExtensionFunctionDispatcher> extension_function_dispatcher_;
 
+  // True if the render view can be shut down suddenly.
+  bool sudden_termination_allowed_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderViewHost);
 };
 
diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h
index ebe8df1..ba69b03 100644
--- a/chrome/browser/renderer_host/render_view_host_delegate.h
+++ b/chrome/browser/renderer_host/render_view_host_delegate.h
@@ -416,8 +416,8 @@ class RenderViewHostDelegate {
   // bombing), see DownloadRequestManager for details.
   virtual void OnEnterOrSpace() { }
 
-  // If this view can be terminated without any side effects
-  virtual bool CanTerminate() const { return true; }
+  // If this view is used to host an external tab container.
+  virtual bool IsExternalTabContainer() const { return false; }
 
   // A find operation in the current page completed.
   virtual void OnFindReply(int request_id,
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index e8eb9bd..fe4ed7a 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -2244,7 +2244,7 @@ void TabContents::RendererUnresponsive(RenderViewHost* rvh,
   if (is_during_unload) {
     // Hang occurred while firing the beforeunload/unload handler.
     // Pretend the handler fired so tab closing continues as if it had.
-    rvh->UnloadListenerHasFired();
+    rvh->set_sudden_termination_allowed(true);
 
     if (!render_manager_.ShouldCloseTabOnUnresponsiveRenderer())
       return;
@@ -2326,11 +2326,11 @@ void TabContents::OnFindReply(int request_id,
       Details<FindNotificationDetails>(&find_result_));
 }
 
-bool TabContents::CanTerminate() const {
+bool TabContents::IsExternalTabContainer() const {
   if (!delegate())
-    return true;
+    return false;
 
-  return !delegate()->IsExternalTabContainer();
+  return delegate()->IsExternalTabContainer();
 }
 
 void TabContents::FileSelected(const FilePath& path,
@@ -2438,4 +2438,3 @@ void TabContents::Observe(NotificationType type,
       NOTREACHED();
   }
 }
-
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index d45d1dc..4709cab 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -835,7 +835,7 @@ class TabContents : public PageNavigator,
                            const gfx::Rect& selection_rect,
                            int active_match_ordinal,
                            bool final_update);
-  virtual bool CanTerminate() const;
+  virtual bool IsExternalTabContainer() const;
 
   // SelectFileDialog::Listener ------------------------------------------------
 
diff --git a/chrome/browser/unload_uitest.cc b/chrome/browser/unload_uitest.cc
index 10a3a4a..71f1fd1 100644
--- a/chrome/browser/unload_uitest.cc
+++ b/chrome/browser/unload_uitest.cc
@@ -8,6 +8,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/message_box_flags.h"
 #include "chrome/test/automation/browser_proxy.h"
+#include "chrome/test/automation/tab_proxy.h"
 #include "chrome/test/ui/ui_test.h"
 #include "net/url_request/url_request_unittest.h"
 
@@ -71,8 +72,25 @@ const std::string TWO_SECOND_BEFORE_UNLOAD_ALERT_HTML =
       "alert('foo');"
     "}</script></body></html>";
 
+const std::string CLOSE_TAB_WHEN_OTHER_TAB_HAS_LISTENER =
+    "<html><head><title>only_one_unload</title></head>"
+    "<body onload=\"window.open('data:text/html,<html><head><title>second_tab</title></head></body>')\" "
+    "onbeforeunload='return;'"
+    "</body></html>";
+
 class UnloadTest : public UITest {
  public:
+  virtual void SetUp() {
+    const testing::TestInfo* const test_info =
+        testing::UnitTest::GetInstance()->current_test_info();
+    if (strcmp(test_info->name(),
+        "BrowserCloseTabWhenOtherTabHasListener") == 0) {
+      launch_arguments_.AppendSwitch(switches::kDisablePopupBlocking);
+    }
+
+    UITest::SetUp();
+  }
+
   void WaitForBrowserClosed() {
     const int kCheckDelayMs = 100;
     int max_wait_time = 5000;
@@ -226,7 +244,7 @@ TEST_F(UnloadTest, BrowserCloseUnload) {
 
 // Tests closing the browser with a beforeunload handler and clicking
 // OK in the beforeunload confirm dialog.
-TEST_F(UnloadTest, DISABLED_BrowserCloseBeforeUnloadOK) {
+TEST_F(UnloadTest, BrowserCloseBeforeUnloadOK) {
   scoped_ptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
   NavigateToDataURL(BEFORE_UNLOAD_HTML, L"beforeunload");
 
@@ -238,7 +256,7 @@ TEST_F(UnloadTest, DISABLED_BrowserCloseBeforeUnloadOK) {
 
 // Tests closing the browser with a beforeunload handler and clicking
 // CANCEL in the beforeunload confirm dialog.
-TEST_F(UnloadTest, DISABLED_BrowserCloseBeforeUnloadCancel) {
+TEST_F(UnloadTest, BrowserCloseBeforeUnloadCancel) {
   scoped_ptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
   NavigateToDataURL(BEFORE_UNLOAD_HTML, L"beforeunload");
 
@@ -313,5 +331,29 @@ TEST_F(UnloadTest, BrowserCloseTwoSecondBeforeUnloadAlert) {
                         L"twosecondbeforeunloadalert");
 }
 
+// Tests that if there's a renderer process with two tabs, one of which has an
+// unload handler, and the other doesn't, the tab that doesn't have an unload
+// handler can be closed.  If this test fails, the Close() call will hang.
+TEST_F(UnloadTest, BrowserCloseTabWhenOtherTabHasListener) {
+  NavigateToDataURL(CLOSE_TAB_WHEN_OTHER_TAB_HAS_LISTENER, L"second_tab");
+
+  scoped_ptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
+  EXPECT_TRUE(browser_proxy.get());
+
+  int tab_count;
+  EXPECT_TRUE(browser_proxy->GetTabCount(&tab_count));
+  EXPECT_EQ(tab_count, 2);
+
+  scoped_ptr<TabProxy> second_tab(browser_proxy->GetActiveTab());
+  EXPECT_TRUE(second_tab.get()!= NULL);
+  EXPECT_TRUE(second_tab->Close(true));
+
+  scoped_ptr<TabProxy> first_tab(browser_proxy->GetActiveTab());
+  std::wstring title;
+  EXPECT_TRUE(first_tab.get() != NULL);
+  EXPECT_TRUE(first_tab->GetTabTitle(&title));
+  EXPECT_EQ(title, L"only_one_unload");
+}
+
 // TODO(ojan): Add tests for unload/beforeunload that have multiple tabs
 // and multiple windows.