Maintain a refcounted AutomationProvider pointer in the ExternalTabContainer. This ensures that we don't crash the browser while trying to dereference a freed AutomationProvider pointer.

When a Chrome browser instance starts up, we attempt to locate an already running instance and defer to it to complete the navigation request. However it is quite possible for the running instance to exit while we attempt to send a WM_COPYDATA message to it. This caused a bunch
of ASSERTS to fire off in the browser. 

Fixes as below:-
1. If GetWindowThreadProcessId fails, we bail out and try to launch a new chrome instance
2. If SendMessageTimeout fails, we check if the window is still valid. If not we bail out and try to launch a new chrome instance.
3. We return an error from the WM_COPYDATA handler if the chrome instance is in the process of shutting down. We handle this at the caller end, i.e. in NotifyOtherProcess.

Bug=1643310

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

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

2 files changed, 22 insertions, 4 deletions

diff --git a/chrome/browser/external_tab_container.h b/chrome/browser/external_tab_container.h
index 89e2a02..7aaca7f 100644
--- a/chrome/browser/external_tab_container.h
+++ b/chrome/browser/external_tab_container.h
@@ -134,7 +134,7 @@ class ExternalTabContainer : public TabContentsDelegate,
 
  protected:
   TabContents *tab_contents_;
-  AutomationProvider* automation_;
+  scoped_refptr<AutomationProvider> automation_;
 
   NotificationRegistrar registrar_;
 
diff --git a/chrome/browser/message_window.cc b/chrome/browser/message_window.cc
index d198254..2174667 100644
--- a/chrome/browser/message_window.cc
+++ b/chrome/browser/message_window.cc
@@ -68,7 +68,12 @@ bool MessageWindow::NotifyOtherProcess() {
   // window (otherwise it will just flash in the taskbar).
   DWORD process_id = 0;
   DWORD thread_id = GetWindowThreadProcessId(remote_window_, &process_id);
-  DCHECK(process_id);
+  // It is possible that the process owning this window may have died by now.
+  if (!thread_id || !process_id) {
+    remote_window_ = NULL;
+    return false;
+  }
+
   AllowSetForegroundWindow(process_id);
 
   // Gives 20 seconds timeout for the current browser process to respond.
@@ -85,9 +90,20 @@ bool MessageWindow::NotifyOtherProcess() {
                          SMTO_ABORTIFHUNG,
                          kTimeout,
                          &result)) {
+    // It is possible that the process owning this window may have died by now.
+    if (!result) {
+      remote_window_ = NULL;
+      return false;
+    }
     return true;
   }
 
+  // It is possible that the process owning this window may have died by now.
+  if (!IsWindow(remote_window_)) {
+    remote_window_ = NULL;
+    return false;
+  }
+
   // The window is hung. Scan for every window to find a visible one.
   bool visible_window = false;
   EnumThreadWindows(thread_id,
@@ -137,8 +153,10 @@ void MessageWindow::Create() {
 
 LRESULT MessageWindow::OnCopyData(HWND hwnd, const COPYDATASTRUCT* cds) {
   // Ignore the request if the browser process is already in shutdown path.
-  if (!g_browser_process || g_browser_process->IsShuttingDown())
-    return TRUE;
+  if (!g_browser_process || g_browser_process->IsShuttingDown()) {
+    LOG(WARNING) << "Not handling WM_COPYDATA as browser is shutting down";
+    return FALSE;
+  }
 
   // If locked, it means we are not ready to process this message because
   // we are probably in a first run critical phase.