Chrome side of the fix for http://b/issue?id=1694574, which is a bug caused when a new automation client instance is launched and attempts to attach to an existing external tab. An example of where this could happen is javascript on a page attempting a window.open with target _blank. In this case the Chrome browser creates a TabContents instance which is attached to an ExternalTabContainer instance. The automation client then attaches to this ExternalTabContainer. This all works if the automation client is in the same client process. If a new process is launched a separate automation channel is created between the client and the chrome browser which causes this to not work as expected.

Fix is have a floating ExternalTabContainer instance which is eventually connected to by the client. When we receive a notification from the client that it is about to connect to the ExternalTabContainer instance we setup the automation channel and other info in the underlying automation profile.

The new TabContents is created with the same profile instance as the current TabContents. This does not
work correctly if the underlying profile is an automation profile as its lifetime is tied to the
ExternalTabContainer. To fix this I added a setter for the new policy to the NavigationController. Not doing
this causes the browser to crash if the original ExternalTabContainer instance dies.

There is a bigger issue here which is that all this profile sharing would cause session cookies to not work correctly if multiple automation clients are connected to the same Chrome browser instance over the same profile. I will file a separate bug to track this issue.

Bug=1694574
 
Review URL: http://codereview.chromium.org/200003

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

7 files changed, 192 insertions, 46 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index b0c09ac..d4c4561 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -425,6 +425,9 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER_DELAY_REPLY(
         AutomationMsg_GoForwardBlockUntilNavigationsComplete,
         GoForwardBlockUntilNavigationsComplete)
+#if defined(OS_WIN)
+    IPC_MESSAGE_HANDLER(AutomationMsg_ConnectExternalTab, ConnectExternalTab)
+#endif
   IPC_END_MESSAGE_MAP()
 }
 
diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h
index 6543a2f..3223ad4 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -299,6 +299,11 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
                          gfx::NativeWindow* tab_window,
                          int* tab_handle);
 
+  void ConnectExternalTab(intptr_t cookie,
+                          gfx::NativeWindow* tab_container_window,
+                          gfx::NativeWindow* tab_window,
+                          int* tab_handle);
+
   void NavigateInExternalTab(
       int handle, const GURL& url,
       AutomationMsg_NavigationResponseValues* status);
diff --git a/chrome/browser/automation/automation_provider_win.cc b/chrome/browser/automation/automation_provider_win.cc
index cbc0452..5cf6007 100644
--- a/chrome/browser/automation/automation_provider_win.cc
+++ b/chrome/browser/automation/automation_provider_win.cc
@@ -335,10 +335,14 @@ void AutomationProvider::CreateExternalTab(
   *tab_handle = 0;
   *tab_container_window = NULL;
   *tab_window = NULL;
-  ExternalTabContainer* external_tab_container =
+  scoped_refptr<ExternalTabContainer> external_tab_container =
       new ExternalTabContainer(this, automation_resource_message_filter_);
+
   Profile* profile = settings.is_off_the_record ?
       profile_->GetOffTheRecordProfile() : profile_;
+
+  // When the ExternalTabContainer window is created we grab a reference on it
+  // which is released when the window is destroyed.
   external_tab_container->Init(profile, settings.parent, settings.dimensions,
       settings.style, settings.load_requests_via_automation,
       settings.handle_top_level_requests, NULL);
@@ -350,7 +354,7 @@ void AutomationProvider::CreateExternalTab(
     *tab_container_window = external_tab_container->GetNativeView();
     *tab_window = tab_contents->GetNativeView();
   } else {
-    delete external_tab_container;
+    external_tab_container->Uninitialize();
   }
 }
 
@@ -451,3 +455,33 @@ void AutomationProvider::OnForwardContextMenuCommandToChrome(int tab_handle,
     tab_contents->delegate()->ExecuteContextMenuCommand(command);
   }
 }
+
+void AutomationProvider::ConnectExternalTab(
+    intptr_t cookie,
+    gfx::NativeWindow* tab_container_window,
+    gfx::NativeWindow* tab_window,
+    int* tab_handle) {
+  *tab_handle = 0;
+  *tab_container_window = NULL;
+  *tab_window = NULL;
+
+  scoped_refptr<ExternalTabContainer> external_tab_container =
+      ExternalTabContainer::RemovePendingTab(cookie);
+  if (!external_tab_container.get()) {
+    NOTREACHED();
+    return;
+  }
+
+  if (AddExternalTab(external_tab_container)) {
+    external_tab_container->Reinitialize(this,
+                                         automation_resource_message_filter_);
+    TabContents* tab_contents = external_tab_container->tab_contents();
+    *tab_handle = external_tab_container->tab_handle();
+    external_tab_container->set_tab_handle(*tab_handle);
+    *tab_container_window = external_tab_container->GetNativeView();
+    *tab_window = tab_contents->GetNativeView();
+  } else {
+    external_tab_container->Uninitialize();
+  }
+}
+
diff --git a/chrome/browser/automation/automation_resource_message_filter.cc b/chrome/browser/automation/automation_resource_message_filter.cc
index ee8be8eb..9d899b9 100644
--- a/chrome/browser/automation/automation_resource_message_filter.cc
+++ b/chrome/browser/automation/automation_resource_message_filter.cc
@@ -46,7 +46,18 @@ void AutomationResourceMessageFilter::OnChannelConnected(int32 peer_pid) {
 void AutomationResourceMessageFilter::OnChannelClosing() {
   channel_ = NULL;
   request_map_.clear();
-  filtered_render_views_.clear();
+
+  // Only erase RenderViews which are associated with this
+  // AutomationResourceMessageFilter instance.
+  RenderViewMap::iterator index = filtered_render_views_.begin();
+  while (index != filtered_render_views_.end()) {
+    const AutomationDetails& details = (*index).second;
+    if (details.filter.get() == this) {
+      filtered_render_views_.erase(index++);
+    } else {
+      index++;
+    }
+  }
 }
 
 // Called on the IPC thread:
diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc
index 38d748e..a3d76c4 100644
--- a/chrome/browser/external_tab_container.cc
+++ b/chrome/browser/external_tab_container.cc
@@ -27,6 +27,8 @@
 
 static const wchar_t kWindowObjectKey[] = L"ChromeWindowObject";
 
+ExternalTabContainer::PendingTabs ExternalTabContainer::pending_tabs_;
+
 ExternalTabContainer::ExternalTabContainer(
     AutomationProvider* automation, AutomationResourceMessageFilter* filter)
     : automation_(automation),
@@ -40,7 +42,7 @@ ExternalTabContainer::ExternalTabContainer(
 }
 
 ExternalTabContainer::~ExternalTabContainer() {
-  Uninitialize(GetNativeView());
+  Uninitialize();
 }
 
 bool ExternalTabContainer::Init(Profile* profile,
@@ -81,12 +83,15 @@ bool ExternalTabContainer::Init(Profile* profile,
     profile = automation_profile_.get();
   }
 
-  if (existing_contents)
+  if (existing_contents) {
     tab_contents_ = existing_contents;
-  else
+    tab_contents_->controller().set_profile(profile);
+  } else {
     tab_contents_ = new TabContents(profile, NULL, MSG_ROUTING_NONE, NULL);
+  }
 
   tab_contents_->set_delegate(this);
+
   tab_contents_->GetMutableRendererPrefs()->browser_handles_top_level_requests =
       handle_top_level_requests;
 
@@ -139,6 +144,47 @@ bool ExternalTabContainer::Init(Profile* profile,
   return true;
 }
 
+void ExternalTabContainer::Uninitialize() {
+  registrar_.RemoveAll();
+  if (tab_contents_) {
+    NotificationService::current()->Notify(
+        NotificationType::EXTERNAL_TAB_CLOSED,
+        Source<NavigationController>(&tab_contents_->controller()),
+        Details<ExternalTabContainer>(this));
+
+    delete tab_contents_;
+    tab_contents_ = NULL;
+  }
+}
+
+bool ExternalTabContainer::Reinitialize(
+    AutomationProvider* automation_provider,
+    AutomationResourceMessageFilter* filter) {
+  if (!automation_provider || !filter) {
+    NOTREACHED();
+    return false;
+  }
+
+  automation_ = automation_provider;
+  automation_resource_message_filter_ = filter;
+
+  if (load_requests_via_automation_) {
+    RenderViewHost* rvh = tab_contents_->render_view_host();
+    if (rvh) {
+      AutomationResourceMessageFilter::RegisterRenderView(
+          rvh->process()->id(), rvh->routing_id(),
+          tab_handle_, automation_resource_message_filter_);
+    }
+
+    DCHECK(automation_profile_.get() != NULL);
+    Profile* profile = tab_contents_->profile()->GetOriginalProfile();
+    DCHECK(profile != NULL);
+    automation_profile_->Initialize(profile, filter);
+  }
+
+  return true;
+}
+
 void ExternalTabContainer::ProcessUnhandledAccelerator(const MSG& msg) {
   DefWindowProc(msg.hwnd, msg.message, msg.wParam, msg.lParam);
 }
@@ -229,24 +275,32 @@ void ExternalTabContainer::AddNewContents(TabContents* source,
     case NEW_BACKGROUND_TAB: {
       DCHECK(automation_ != NULL);
 
-      ExternalTabContainer* new_container =
-          new ExternalTabContainer(automation_,
-                                   automation_resource_message_filter_);
-      bool result = new_container->Init(automation_profile_.get(),
-                                        NULL,
-                                        initial_pos,
-                                        WS_CHILD,
-                                        load_requests_via_automation_,
-                                        handle_top_level_requests_,
-                                        new_contents);
-      DCHECK(result);
-      result = automation_->AddExternalTab(new_container);
-      DCHECK(result);
-
-      automation_->Send(new AutomationMsg_AttachExternalTab(
-          0, tab_handle_, new_container->tab_handle(),
-          new_container->GetNativeView(), new_contents->GetNativeView(),
-          disposition));
+      scoped_refptr<ExternalTabContainer> new_container =
+          new ExternalTabContainer(NULL, NULL);
+
+      // Make sure that ExternalTabContainer instance is initialized with
+      // an unwrapped Profile.
+      bool result = new_container->Init(
+          new_contents->profile()->GetOriginalProfile(),
+          NULL,
+          initial_pos,
+          WS_CHILD,
+          load_requests_via_automation_,
+          handle_top_level_requests_,
+          new_contents);
+
+      if (result) {
+        pending_tabs_[reinterpret_cast<intptr_t>(new_container.get())] =
+            new_container;
+
+        automation_->Send(new AutomationMsg_AttachExternalTab(
+            0,
+            tab_handle_,
+            reinterpret_cast<intptr_t>(new_container.get()),
+            disposition));
+      } else {
+        NOTREACHED();
+      }
       break;
     }
 
@@ -453,30 +507,30 @@ void ExternalTabContainer::Observe(NotificationType type,
 ////////////////////////////////////////////////////////////////////////////////
 // ExternalTabContainer, views::WidgetWin overrides:
 
+LRESULT ExternalTabContainer::OnCreate(LPCREATESTRUCT create_struct) {
+  LRESULT result = views::WidgetWin::OnCreate(create_struct);
+  if (result == 0) {
+    // Grab a reference here which will be released in OnFinalMessage
+    AddRef();
+  }
+  return result;
+}
+
 void ExternalTabContainer::OnDestroy() {
-  Uninitialize(GetNativeView());
+  Uninitialize();
   WidgetWin::OnDestroy();
   if (browser_.get()) {
     ::DestroyWindow(browser_->window()->GetNativeHandle());
   }
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// ExternalTabContainer, private:
-
-void ExternalTabContainer::Uninitialize(HWND window) {
-  registrar_.RemoveAll();
-  if (tab_contents_) {
-    NotificationService::current()->Notify(
-        NotificationType::EXTERNAL_TAB_CLOSED,
-        Source<NavigationController>(&tab_contents_->controller()),
-        Details<ExternalTabContainer>(this));
-
-    delete tab_contents_;
-    tab_contents_ = NULL;
-  }
+void ExternalTabContainer::OnFinalMessage(HWND window) {
+  // Release the reference which we grabbed in WM_CREATE.
+  Release();
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// ExternalTabContainer, private:
 bool ExternalTabContainer::ProcessUnhandledKeyStroke(HWND window,
                                                      UINT message,
                                                      WPARAM wparam,
@@ -534,3 +588,16 @@ bool ExternalTabContainer::InitNavigationInfo(IPC::NavigationInfo* nav_info,
   return true;
 }
 
+ExternalTabContainer* ExternalTabContainer::RemovePendingTab(intptr_t cookie) {
+  PendingTabs::iterator index = pending_tabs_.find(cookie);
+  if (index != pending_tabs_.end()) {
+    scoped_refptr<ExternalTabContainer> container = (*index).second;
+    pending_tabs_.erase(index);
+    return container.release();
+  }
+
+  NOTREACHED() << "Failed to find ExternalTabContainer for cookie: "
+               << cookie;
+  return NULL;
+}
+
diff --git a/chrome/browser/external_tab_container.h b/chrome/browser/external_tab_container.h
index 1724bb4..00eec54 100644
--- a/chrome/browser/external_tab_container.h
+++ b/chrome/browser/external_tab_container.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_EXTERNAL_TAB_CONTAINER_H_
 
 #include <vector>
+#include <map>
 
 #include "chrome/browser/automation/automation_resource_message_filter.h"
 #include "chrome/browser/automation/automation_profile_impl.h"
@@ -32,8 +33,11 @@ struct NavigationInfo;
 // TODO(beng): Should override WidgetWin instead of Widget.
 class ExternalTabContainer : public TabContentsDelegate,
                              public NotificationObserver,
-                             public views::WidgetWin {
+                             public views::WidgetWin,
+                             public base::RefCounted<ExternalTabContainer> {
  public:
+  typedef std::map<intptr_t, scoped_refptr<ExternalTabContainer> > PendingTabs;
+
   ExternalTabContainer(AutomationProvider* automation,
       AutomationResourceMessageFilter* filter);
   ~ExternalTabContainer();
@@ -59,6 +63,19 @@ class ExternalTabContainer : public TabContentsDelegate,
             bool handle_top_level_requests,
             TabContents* existing_tab_contents);
 
+  // Unhook the keystroke listener and notify about the closing TabContents.
+  // This function gets called from three places, which is fine.
+  // 1. OnFinalMessage
+  // 2. In the destructor.
+  // 3. In AutomationProvider::CreateExternalTab
+  void Uninitialize();
+
+  // Used to reinitialize the automation channel and related information
+  // for this container. Typically used when an ExternalTabContainer
+  // instance is created by Chrome and attached to an automation client.
+  bool Reinitialize(AutomationProvider* automation_provider,
+                    AutomationResourceMessageFilter* filter);
+
   // This is invoked when the external host reflects back to us a keyboard
   // message it did not process
   void ProcessUnhandledAccelerator(const MSG& msg);
@@ -127,21 +144,22 @@ class ExternalTabContainer : public TabContentsDelegate,
   virtual void ShowHtmlDialog(HtmlDialogUIDelegate* delegate,
                               gfx::NativeWindow parent_window);
 
+  // Returns the ExternalTabContainer instance associated with the cookie
+  // passed in. It also erases the corresponding reference from the map.
+  // Returns NULL if we fail to find the cookie in the map.
+  static ExternalTabContainer* RemovePendingTab(intptr_t cookie);
 
  protected:
   // Overridden from views::WidgetWin:
+  virtual LRESULT OnCreate(LPCREATESTRUCT create_struct);
   virtual void OnDestroy();
+  virtual void OnFinalMessage(HWND window);
+
   bool InitNavigationInfo(IPC::NavigationInfo* nav_info,
                           NavigationType::Type nav_type,
                           int relative_offset);
 
  private:
-  // Unhook the keystroke listener and notify about the closing TabContents.
-  // This function gets called from two places, which is fine.
-  // 1. OnFinalMessage
-  // 2. In the destructor.
-  void Uninitialize(HWND window);
-
   // Helper function for processing keystokes coming back from the renderer
   // process.
   bool ProcessUnhandledKeyStroke(HWND window, UINT message, WPARAM wparam,
@@ -182,6 +200,9 @@ class ExternalTabContainer : public TabContentsDelegate,
   // A customized profile for automation specific needs.
   scoped_ptr<AutomationProfileImpl> automation_profile_;
 
+  // Contains ExternalTabContainers that have not been connected to as yet.
+  static PendingTabs pending_tabs_;
+
   DISALLOW_COPY_AND_ASSIGN(ExternalTabContainer);
 };
 
diff --git a/chrome/browser/tab_contents/navigation_controller.h b/chrome/browser/tab_contents/navigation_controller.h
index 9be8437..3d0d6cd 100644
--- a/chrome/browser/tab_contents/navigation_controller.h
+++ b/chrome/browser/tab_contents/navigation_controller.h
@@ -138,6 +138,11 @@ class NavigationController {
     return profile_;
   }
 
+  // Sets the profile for this controller.
+  void set_profile(Profile* profile) {
+    profile_ = profile;
+  }
+
   // Initializes this NavigationController with the given saved navigations,
   // using selected_navigation as the currently loaded entry. Before this call
   // the controller should be unused (there should be no current entry). This is