Multiple chrome frame activex controls should instantiate and navigate correctly in IE. This was not

the case due to a race condition between put_src getting called for subsequent activex instances
and the external tab to hold the chrome frame instance getting created. 

Fix is to pass in the URL if we have it when the automation client is initialized to launch the chrome
automation server. If not we navigate when the external tab is created. To achieve this we stuff in
all relevant parameters into a structure which is populated when the automation client is initialized.

I also changed the CreateExternalTab message to carry the referrer for the initial navigation.

Fixes http://code.google.com/p/chromium/issues/detail?id=28236
Test=added unit tests for the same. The firefox one is not working at this point. Disabled this test
     for now while I debug it.

Bug=28236

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

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

5 files changed, 188 insertions, 45 deletions

diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 1e63dab..6826434 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -172,16 +172,15 @@ ProxyFactory::~ProxyFactory() {
   }
 }
 
-void* ProxyFactory::GetAutomationServer(int launch_timeout,
-    const std::wstring& profile_name,
-    const std::wstring& extra_argument,
-    bool perform_version_check,
-    LaunchDelegate* delegate) {
+void ProxyFactory::GetAutomationServer(
+    LaunchDelegate* delegate, const ChromeFrameLaunchParams& params,
+    void** automation_server_id) {
   ProxyCacheEntry* entry = NULL;
   // Find already existing launcher thread for given profile
   AutoLock lock(lock_);
   for (size_t i = 0; i < proxies_.container().size(); ++i) {
-    if (!lstrcmpiW(proxies_[i]->profile_name.c_str(), profile_name.c_str())) {
+    if (!lstrcmpiW(proxies_[i]->profile_name.c_str(),
+                   params.profile_name.c_str())) {
       entry = proxies_[i];
       DCHECK(entry->thread.get() != NULL);
       break;
@@ -189,34 +188,31 @@ void* ProxyFactory::GetAutomationServer(int launch_timeout,
   }
 
   if (entry == NULL) {
-    entry = new ProxyCacheEntry(profile_name);
+    entry = new ProxyCacheEntry(params.profile_name);
     proxies_.container().push_back(entry);
   } else {
     entry->ref_count++;
   }
 
+  DCHECK(delegate != NULL);
+  DCHECK(automation_server_id != NULL);
 
+  *automation_server_id = entry;
   // Note we always queue request to the launch thread, even if we already
   // have established proxy object. A simple lock around entry->proxy = proxy
   // would allow calling LaunchDelegate directly from here if
   // entry->proxy != NULL. Drawback is that callback may be invoked either in
   // main thread or in background thread, which may confuse the client.
   entry->thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(this,
-      &ProxyFactory::CreateProxy, entry,
-      launch_timeout, extra_argument,
-      perform_version_check, delegate));
+      &ProxyFactory::CreateProxy, entry, params, delegate));
 
   entry->thread->message_loop()->PostDelayedTask(FROM_HERE,
       NewRunnableMethod(this, &ProxyFactory::SendUMAData, entry),
       uma_send_interval_);
-
-  return entry;
 }
 
 void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry,
-                               int launch_timeout,
-                               const std::wstring& extra_chrome_arguments,
-                               bool perform_version_check,
+                               const ChromeFrameLaunchParams& params,
                                LaunchDelegate* delegate) {
   DCHECK(entry->thread->thread_id() == PlatformThread::CurrentId());
   if (entry->proxy) {
@@ -231,7 +227,8 @@ void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry,
 
   // At same time we must destroy/stop the thread from another thread.
   ChromeFrameAutomationProxyImpl* proxy =
-      new ChromeFrameAutomationProxyImpl(launch_timeout);
+      new ChromeFrameAutomationProxyImpl(
+          params.automation_server_launch_timeout);
 
   // Launch browser
   scoped_ptr<CommandLine> command_line(
@@ -278,8 +275,8 @@ void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry,
 
   std::wstring command_line_string(command_line->command_line_string());
   // If there are any extra arguments, append them to the command line.
-  if (!extra_chrome_arguments.empty()) {
-    command_line_string += L' ' + extra_chrome_arguments;
+  if (!params.extra_chrome_arguments.empty()) {
+    command_line_string += L' ' + params.extra_chrome_arguments;
   }
 
   automation_server_launch_start_time_ = base::TimeTicks::Now();
@@ -411,13 +408,13 @@ ChromeFrameAutomationClient::ChromeFrameAutomationClient()
       automation_server_(NULL),
       automation_server_id_(NULL),
       ui_thread_id_(NULL),
-      incognito_(false),
       init_state_(UNINITIALIZED),
       use_chrome_network_(false),
       proxy_factory_(g_proxy_factory.get()),
       handle_top_level_requests_(false),
       tab_handle_(-1),
-      external_tab_cookie_(NULL) {
+      external_tab_cookie_(NULL),
+      navigate_after_initialization_(false) {
 }
 
 ChromeFrameAutomationClient::~ChromeFrameAutomationClient() {
@@ -434,7 +431,6 @@ bool ChromeFrameAutomationClient::Initialize(
     bool incognito) {
   DCHECK(!IsWindow());
   chrome_frame_delegate_ = chrome_frame_delegate;
-  incognito_ = incognito;
   ui_thread_id_ = PlatformThread::CurrentId();
 #ifndef NDEBUG
   // In debug mode give more time to work with a debugger.
@@ -464,10 +460,17 @@ bool ChromeFrameAutomationClient::Initialize(
   // InitializeComplete is called successfully.
   init_state_ = INITIALIZING;
 
-  automation_server_id_ = proxy_factory_->GetAutomationServer(
-      automation_server_launch_timeout,
-      profile_name, extra_chrome_arguments, perform_version_check,
-      static_cast<ProxyFactory::LaunchDelegate*>(this));
+  chrome_launch_params_.automation_server_launch_timeout =
+      automation_server_launch_timeout;
+  chrome_launch_params_.profile_name = profile_name;
+  chrome_launch_params_.extra_chrome_arguments = extra_chrome_arguments;
+  chrome_launch_params_.perform_version_check = perform_version_check;
+  chrome_launch_params_.url = url_;
+  chrome_launch_params_.incognito_mode = incognito;
+
+  proxy_factory_->GetAutomationServer(
+      static_cast<ProxyFactory::LaunchDelegate*>(this),
+      chrome_launch_params_, &automation_server_id_);
 
   return true;
 }
@@ -518,6 +521,7 @@ bool ChromeFrameAutomationClient::InitiateNavigation(
     return false;
 
   url_ = GURL(url);
+  referrer_ = GURL(referrer);
 
   // Catch invalid URLs early.
   if (!url_.is_valid() || !IsValidUrlScheme(UTF8ToWide(url), is_privileged)) {
@@ -526,8 +530,12 @@ bool ChromeFrameAutomationClient::InitiateNavigation(
     return false;
   }
 
+  navigate_after_initialization_ = false;
+
   if (is_initialized()) {
-    BeginNavigate(GURL(url), GURL(referrer));
+    BeginNavigate(url_, referrer_);
+  } else {
+    navigate_after_initialization_ = true;
   }
 
   return true;
@@ -707,14 +715,17 @@ void ChromeFrameAutomationClient::CreateExternalTab() {
   DCHECK(IsWindow());
   DCHECK(automation_server_ != NULL);
 
+  // TODO(ananta)
+  // We should pass in the referrer for the initial navigation.
   const IPC::ExternalTabSettings settings = {
     m_hWnd,
     gfx::Rect(),
     WS_CHILD,
-    incognito_,
+    chrome_launch_params_.incognito_mode,
     !use_chrome_network_,
     handle_top_level_requests_,
-    GURL(url_)
+    chrome_launch_params_.url,
+    chrome_launch_params_.referrer,
   };
 
   UMA_HISTOGRAM_CUSTOM_COUNTS(
@@ -822,6 +833,12 @@ void ChromeFrameAutomationClient::InitializeComplete(
     // If the host already have a window, ask Chrome to re-parent.
     if (parent_window_)
       SetParentWindow(parent_window_);
+
+    // If host specified destination URL - navigate. Apparently we do not use
+    // accelerator table.
+    if (navigate_after_initialization_) {
+      BeginNavigate(url_, referrer_);
+    }
   }
 
   if (chrome_frame_delegate_) {
diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h
index 90c1951..aa62948 100644
--- a/chrome_frame/chrome_frame_automation.h
+++ b/chrome_frame/chrome_frame_automation.h
@@ -79,6 +79,17 @@ class ChromeFrameAutomationProxyImpl : public ChromeFrameAutomationProxy,
   friend class ProxyFactory;
 };
 
+// This structure contains information used for launching chrome.
+struct ChromeFrameLaunchParams {
+  int automation_server_launch_timeout;
+  GURL url;
+  GURL referrer;
+  std::wstring profile_name;
+  std::wstring extra_chrome_arguments;
+  bool perform_version_check;
+  bool incognito_mode;
+};
+
 // We must create and destroy automation proxy in a thread with a message loop.
 // Hence thread cannot be a member of the proxy.
 class ProxyFactory {
@@ -92,14 +103,10 @@ class ProxyFactory {
 
   ProxyFactory();
   ~ProxyFactory();
-  // FIXME: we should pass the result as output parameter, not as return value
-  // since, LaunchDelegate can be invoked before this function returns.
-  virtual void* GetAutomationServer(int launch_timeout,
-                                    const std::wstring& profile_name,
-                            // Extra command line argument when launching Chrome
-                                    const std::wstring& extra_argument,
-                                    bool perform_version_check,
-                                    LaunchDelegate* delegate);
+
+  virtual void GetAutomationServer(LaunchDelegate* delegate,
+                                   const ChromeFrameLaunchParams& params,
+                                   void** automation_server_id);
   virtual bool ReleaseAutomationServer(void* server_id);
 
  private:
@@ -113,9 +120,7 @@ class ProxyFactory {
   };
 
   void CreateProxy(ProxyCacheEntry* entry,
-                   int launch_timeout,
-                   const std::wstring& extra_chrome_arguments,
-                   bool perform_version_check,
+                   const ChromeFrameLaunchParams& params,
                    LaunchDelegate* delegate);
   void DestroyProxy(ProxyCacheEntry* entry);
 
@@ -300,7 +305,6 @@ class ChromeFrameAutomationClient
     return init_state_ == INITIALIZED;
   }
 
-  bool incognito_;
   HWND parent_window_;
   PlatformThreadId ui_thread_id_;
 
@@ -310,6 +314,7 @@ class ChromeFrameAutomationClient
   scoped_refptr<TabProxy> tab_;
   ChromeFrameDelegate* chrome_frame_delegate_;
   GURL url_;
+  GURL referrer_;
 
   // Handle to the underlying chrome window. This is a child of the external
   // tab window.
@@ -335,6 +340,12 @@ class ChromeFrameAutomationClient
   int tab_handle_;
   // Only used if we attach to an existing tab.
   intptr_t external_tab_cookie_;
+
+  // Set to true if we received a navigation request prior to the automation
+  // server being initialized.
+  bool navigate_after_initialization_;
+
+  ChromeFrameLaunchParams chrome_launch_params_;
 };
 
 #endif  // CHROME_FRAME_CHROME_FRAME_AUTOMATION_H_
diff --git a/chrome_frame/test/chrome_frame_unittests.cc b/chrome_frame/test/chrome_frame_unittests.cc
index fc4fd1a..9302105 100644
--- a/chrome_frame/test/chrome_frame_unittests.cc
+++ b/chrome_frame/test/chrome_frame_unittests.cc
@@ -652,7 +652,16 @@ TEST(ProxyFactoryTest, CreateDestroy) {
   ProxyFactory f;
   LaunchDelegateMock d;
   EXPECT_CALL(d, LaunchComplete(testing::NotNull(), testing::_)).Times(1);
-  void* id = f.GetAutomationServer(0, L"Adam.N.Epilinter", L"", false, &d);
+
+  ChromeFrameLaunchParams params;
+  params.automation_server_launch_timeout = 0;
+  params.profile_name = L"Adam.N.Epilinter";
+  params.extra_chrome_arguments = L"";
+  params.perform_version_check = false;
+  params.incognito_mode = false;
+
+  void* id = NULL;
+  f.GetAutomationServer(&d, params, &id);
   f.ReleaseAutomationServer(id);
 }
 
@@ -660,8 +669,20 @@ TEST(ProxyFactoryTest, CreateSameProfile) {
   ProxyFactory f;
   LaunchDelegateMock d;
   EXPECT_CALL(d, LaunchComplete(testing::NotNull(), testing::_)).Times(2);
-  void* i1 = f.GetAutomationServer(0, L"Dr. Gratiano Forbeson", L"", false, &d);
-  void* i2 = f.GetAutomationServer(0, L"Dr. Gratiano Forbeson", L"", false, &d);
+
+  ChromeFrameLaunchParams params;
+  params.automation_server_launch_timeout = 0;
+  params.profile_name = L"Dr. Gratiano Forbeson";
+  params.extra_chrome_arguments = L"";
+  params.perform_version_check = false;
+  params.incognito_mode = false;
+
+  void* i1 = NULL;
+  void* i2 = NULL;
+
+  f.GetAutomationServer(&d, params, &i1);
+  f.GetAutomationServer(&d, params, &i2);
+
   EXPECT_EQ(i1, i2);
   f.ReleaseAutomationServer(i2);
   f.ReleaseAutomationServer(i1);
@@ -671,8 +692,27 @@ TEST(ProxyFactoryTest, CreateDifferentProfiles) {
   ProxyFactory f;
   LaunchDelegateMock d;
   EXPECT_CALL(d, LaunchComplete(testing::NotNull(), testing::_)).Times(2);
-  void* i1 = f.GetAutomationServer(0, L"Adam.N.Epilinter", L"", false, &d);
-  void* i2 = f.GetAutomationServer(0, L"Dr. Gratiano Forbeson", L"", false, &d);
+
+  ChromeFrameLaunchParams params1;
+  params1.automation_server_launch_timeout = 0;
+  params1.profile_name = L"Adam.N.Epilinter";
+  params1.extra_chrome_arguments = L"";
+  params1.perform_version_check = false;
+  params1.incognito_mode = false;
+
+  ChromeFrameLaunchParams params2;
+  params2.automation_server_launch_timeout = 0;
+  params2.profile_name = L"Dr. Gratiano Forbeson";
+  params2.extra_chrome_arguments = L"";
+  params2.perform_version_check = false;
+  params2.incognito_mode = false;
+
+  void* i1 = NULL;
+  void* i2 = NULL;
+
+  f.GetAutomationServer(&d, params1, &i1);
+  f.GetAutomationServer(&d, params2, &i2);
+
   EXPECT_NE(i1, i2);
   f.ReleaseAutomationServer(i2);
   f.ReleaseAutomationServer(i1);
@@ -1667,3 +1707,19 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_ChromeFrameXHRTest) {
   ASSERT_TRUE(CheckResultFile(L"FullTab_XMLHttpRequestTest", "OK"));
 }
 
+const wchar_t kMultipleCFInstancesTestUrl[] =
+    L"files/multiple_cf_instances_main.html";
+
+TEST_F(ChromeFrameTestWithWebServer, WidgetModeIE_MultipleCFInstances) {
+  SimpleBrowserTest(IE, kMultipleCFInstancesTestUrl,
+                    L"WidgetMode_MultipleInstancesTest");
+}
+
+// TODO(ananta)
+// Disabled until I figure out why this does not work on Firefox.
+TEST_F(ChromeFrameTestWithWebServer,
+       DISABLED_WidgetModeFF_MultipleCFInstances) {
+  SimpleBrowserTest(FIREFOX, kMultipleCFInstancesTestUrl,
+                    L"WidgetMode_MultipleInstancesTest");
+}
+
diff --git a/chrome_frame/test/data/multiple_cf_instances_main.html b/chrome_frame/test/data/multiple_cf_instances_main.html
new file mode 100644
index 0000000..da6586d
--- /dev/null
+++ b/chrome_frame/test/data/multiple_cf_instances_main.html
@@ -0,0 +1,37 @@
+<html>
+<head>
+<title>Multiple chrome frame instances test</title>
+<script type="text/javascript">
+  function createSecondChromeFrameInstance() {
+    var dummy = document.createElement('span');
+    document.body.appendChild(dummy);
+    dummy.innerHTML = '<object border="1" width="100%" height ="200px"' +
+                      'classid="clsid:E0A900DF-9611-4446-86BD-4B1D47E7DB2A">' +
+                      '<param name="src"' +
+                      'value="multiple_cf_instances_test.html">' +
+                      '<embed id="CFPlugin" width="500" height="500"' +
+                        'name="ChromeFrame" type="application/chromeframe"' +
+                        'src="multiple_cf_instances_test.html">' +
+                      '</embed></object>';
+  }
+
+  function onLoad() {
+    createSecondChromeFrameInstance();
+  }
+</script>
+</head>
+
+<body onload="onLoad();">
+    <object id="ChromeFrame1" width="500" height="500"
+        classid="CLSID:E0A900DF-9611-4446-86BD-4B1D47E7DB2A">
+        <param name="src" value="about:blank">
+        <embed id="ChromeFramePlugin" width="500" height="500"
+            name="ChromeFrame" type="application/chromeframe"
+            src="about:blank">
+        </embed>
+    </object>
+ChromeFrame multiple widget instances test page. This testcase verifies
+whether multiple chrome frame widget instances can be created on the
+same page.
+</body>
+</html>
diff --git a/chrome_frame/test/data/multiple_cf_instances_test.html b/chrome_frame/test/data/multiple_cf_instances_test.html
new file mode 100644
index 0000000..3208802
--- /dev/null
+++ b/chrome_frame/test/data/multiple_cf_instances_test.html
@@ -0,0 +1,22 @@
+<html>
+  <head>
+    <title>Multiple ChromeFrame instances target page.</title>
+    <script type="text/javascript"
+        src="chrome_frame_tester_helpers.js"></script>
+
+    <script type="text/javascript">
+      function onLoad() {
+        if (!isRunningInChrome()) {
+          onFailure("WidgetMode_MultipleInstancesTest", 1,
+                    "Page not running in Chrome");
+        } else {
+          onSuccess("WidgetMode_MultipleInstancesTest", 1);
+        }
+      }
+    </script>
+  </head>
+
+  <body onLoad="onLoad()">
+    ChromeFrame multiple widget instances test
+  </body>
+</html>