ChromeFrame tabs would hang at times while closing. This would randomly occur if the page had an unload handler.

We execute unload handlers in the WM_DESTROY message in the external tab and spin a nested loop waiting for the
unload handlers to finish. This causes a deadlock at times if a windows message is dispatched to IE which is blocked
in DestroyWindow.

The fix is to remove the nested loop mess from the external tab and instead send over a special automation message
to Chrome in which context we execute the unload handlers. The message contains the notification window and the
actual window message to be posted back when the unload handlers finish executing. 

The active document/activex spin a GetMessage loop waiting for this message to arrive. To ensure that we don't wait
indefinitely we have a 1 second timer and exit the loop if this timer is received.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=49132

Bug=49132
Test=Covered by existing unload event test.

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

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

6 files changed, 76 insertions, 2 deletions

diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h
index dc0a955..be8553a 100644
--- a/chrome_frame/chrome_frame_activex_base.h
+++ b/chrome_frame/chrome_frame_activex_base.h
@@ -361,6 +361,43 @@ END_MSG_MAP()
     return true;
   }
 
+  // IOleInPlaceObject overrides.
+  STDMETHOD(InPlaceDeactivate)(void) {
+    static UINT onload_handlers_done_msg =
+        RegisterWindowMessage(L"ChromeFrame_OnloadHandlersDone");
+
+    if (m_bInPlaceActive && IsWindow() && IsValid()) {
+      static const int kChromeFrameUnloadEventTimerId = 0xdeadbeef;
+      static const int kChromeFrameUnloadEventTimeout = 1000;
+
+      // To prevent us from indefinitely waiting for an acknowledgement from
+      // Chrome indicating that unload handlers have been run, we set a 1
+      // second timer and exit the loop when it fires.
+      ::SetTimer(m_hWnd, kChromeFrameUnloadEventTimerId,
+                 kChromeFrameUnloadEventTimeout, NULL);
+
+      automation_client_->RunUnloadHandlers(m_hWnd, onload_handlers_done_msg);
+
+      MSG msg = {0};
+      while (GetMessage(&msg, NULL, 0, 0)) {
+        if (msg.message == onload_handlers_done_msg &&
+            msg.hwnd == m_hWnd) {
+          break;
+        }
+
+        if (msg.message == WM_TIMER &&
+            msg.wParam == kChromeFrameUnloadEventTimerId) {
+          break;
+        }
+        TranslateMessage(&msg);
+        DispatchMessage(&msg);
+      }
+
+      ::KillTimer(m_hWnd, kChromeFrameUnloadEventTimerId);
+    }
+    return IOleInPlaceObjectWindowlessImpl<T>::InPlaceDeactivate();
+  }
+
  protected:
   virtual void GetProfilePath(const std::wstring& profile_name,
                               FilePath* profile_path) {
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index c6d43d7..681f055 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -1233,6 +1233,13 @@ void ChromeFrameAutomationClient::RemoveBrowsingData(int remove_mask) {
       new AutomationMsg_RemoveBrowsingData(0, remove_mask));
 }
 
+void ChromeFrameAutomationClient::RunUnloadHandlers(HWND notification_window,
+                                                    int notification_message) {
+  automation_server_->Send(
+      new AutomationMsg_RunUnloadHandlers(0, tab_handle_, notification_window,
+                                          notification_message));
+}
+
 //////////////////////////////////////////////////////////////////////////
 // PluginUrlRequestDelegate implementation.
 // Forward network related responses to Chrome.
diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h
index 7b9b774..77c82a3 100644
--- a/chrome_frame/chrome_frame_automation.h
+++ b/chrome_frame/chrome_frame_automation.h
@@ -273,6 +273,10 @@ class ChromeFrameAutomationClient
   // For IDeleteBrowsingHistorySupport
   void RemoveBrowsingData(int remove_mask);
 
+  // Sends an IPC message to the external tab container requesting it to run
+  // unload handlers on the page.
+  void RunUnloadHandlers(HWND notification_window, int notification_message);
+
  protected:
   // ChromeFrameAutomationProxy::LaunchDelegate implementation.
   virtual void LaunchComplete(ChromeFrameAutomationProxy* proxy,
diff --git a/chrome_frame/test/data/fulltab_before_unload_event_main.html b/chrome_frame/test/data/fulltab_before_unload_event_main.html
index d4bbc47..51b50b6 100644
--- a/chrome_frame/test/data/fulltab_before_unload_event_main.html
+++ b/chrome_frame/test/data/fulltab_before_unload_event_main.html
@@ -1,6 +1,5 @@
 <html>
   <head>
-    <meta http-equiv="x-ua-compatible" content="chrome=1" />
     <title>ChromeFrame fulltab mode unload event final page</title>
     <script type="text/javascript"
         src="chrome_frame_tester_helpers.js"></script>
diff --git a/chrome_frame/test/data/fulltab_before_unload_event_test.html b/chrome_frame/test/data/fulltab_before_unload_event_test.html
index 562312a..c10aaea 100644
--- a/chrome_frame/test/data/fulltab_before_unload_event_test.html
+++ b/chrome_frame/test/data/fulltab_before_unload_event_test.html
@@ -1,6 +1,5 @@
 <html>
   <head>
-    <meta http-equiv="x-ua-compatible" content="chrome=1" />
     <title>ChromeFrame fulltab mode unload event test</title>
     <script type="text/javascript"
         src="chrome_frame_tester_helpers.js"></script>
diff --git a/chrome_frame/test/navigation_test.cc b/chrome_frame/test/navigation_test.cc
index 50da567..2324f86 100644
--- a/chrome_frame/test/navigation_test.cc
+++ b/chrome_frame/test/navigation_test.cc
@@ -676,5 +676,33 @@ TEST_P(FullTabNavigationTest, FLAKY_FormPostBackForward) {
   LaunchIEAndNavigate(kFormPostUrl);
 }
 
+TEST_P(FullTabNavigationTest, CF_UnloadEventTest) {
+  bool in_cf = GetParam().invokes_cf();
+  if (!in_cf) {
+    LOG(ERROR) << "Test not yet implemented.";
+    return;
+  }
+
+  std::wstring kUnloadEventTestUrl =
+      GetTestUrl(L"fulltab_before_unload_event_test.html");
+
+  std::wstring kUnloadEventMainUrl =
+      GetTestUrl(L"fulltab_before_unload_event_main.html");
+
+  server_mock_.ExpectAndServeAnyRequests(GetParam());
+  InSequence expect_in_sequence_for_scope;
+
+  ie_mock_.ExpectNavigation(in_cf, kUnloadEventTestUrl);
+  EXPECT_CALL(ie_mock_, OnLoad(in_cf, StrEq(kUnloadEventTestUrl)));
+
+  ie_mock_.ExpectNavigation(in_cf, kUnloadEventMainUrl);
+  EXPECT_CALL(ie_mock_, OnLoad(in_cf, StrEq(kUnloadEventMainUrl)));
+
+  EXPECT_CALL(ie_mock_, OnMessage(_, _, _))
+      .WillOnce(CloseBrowserMock(&ie_mock_));
+
+  LaunchIEAndNavigate(kUnloadEventTestUrl);
+}
+
 }  // namespace chrome_frame_test