From 44d8ce76bed0ef853a28dc55164b8e2246f3eddc Mon Sep 17 00:00:00 2001
From: "aa@chromium.org" <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 26 Oct 2010 21:06:18 +0000
Subject: Re-enable ExtensionApiTest.WindowOpen.

There was a bug in the original implementation of this test
that caused it to close early. Depending on the exact
timing, this sometimes failed, which caused it to be marked
flaky. Later, a bug was introduced that broke the underlying
functionality, which is also fixed here.

BUG=60156
TEST=Covered by browser_tests.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63949 0039d316-1c4b-4281-b951-d872f2087c98
---
 chrome/browser/extensions/extension_host.cc        | 54 +++++++++++++---------
 chrome/browser/extensions/window_open_apitest.cc   |  3 +-
 .../api_test/window_open/spanning/infobar.html     |  6 ++-
 3 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index 4de1a5d..d39bcf7 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -565,33 +565,43 @@ void ExtensionHost::ShowCreatedWindow(int route_id,
                                                contents,
                                                initial_pos);
     browser->window()->Show();
-  } else {
-    Browser* browser = BrowserList::FindBrowserWithType(
+    return;
+  }
+
+  // If the tab contents isn't a popup, it's a normal tab. We need to find a
+  // home for it. This is typically a Browser, but it can also be some other
+  // TabContentsDelegate in the case of ChromeFrame.
+
+  // First, if the creating extension view was associated with a tab contents,
+  // use that tab content's delegate. We must be careful here that the
+  // associated tab contents has the same profile as the new tab contents. In
+  // the case of extensions in 'spanning' incognito mode, they can mismatch.
+  // We don't want to end up putting a normal tab into an incognito window, or
+  // vice versa.
+  TabContents* associated_contents = associated_tab_contents();
+  if (associated_contents &&
+      associated_contents->profile() == contents->profile()) {
+    associated_contents->AddNewContents(contents, disposition, initial_pos,
+                                        user_gesture);
+    return;
+  }
+
+  // If there's no associated tab contents, or it doesn't have a matching
+  // profile, try finding an open window. Again, we must make sure to find a
+  // window with the correct profile.
+  Browser* browser = BrowserList::FindBrowserWithType(
         contents->profile(),
         Browser::TYPE_NORMAL,
         false);  // Match incognito exactly.
-    if (!browser) {
-      // If no browser is associated with the created TabContents, then the
-      // created TabContents may be an intermediate struct used during topmost
-      // url navigation from within an experimental extension popup view.
-      //
-      // If the ExtensionHost has an associated TabContents, then register the
-      // new contents with this contents.  This will allow top-level link
-      // navigation within the new contents to function just as navigation
-      // within the current host.
-      TabContents* associated_contents = associated_tab_contents();
-      if (associated_contents) {
-        associated_contents->AddNewContents(contents, disposition, initial_pos,
-                                            user_gesture);
-      } else {
-        browser = Browser::Create(contents->profile());
-        browser->window()->Show();
-      }
-    }
 
-    if (browser)
-      browser->AddTabContents(contents, disposition, initial_pos, user_gesture);
+  // If there's no Browser open with the right profile, create a new one.
+  if (!browser) {
+    browser = Browser::Create(contents->profile());
+    browser->window()->Show();
   }
+
+  if (browser)
+    browser->AddTabContents(contents, disposition, initial_pos, user_gesture);
 }
 
 void ExtensionHost::ShowCreatedWidget(int route_id,
diff --git a/chrome/browser/extensions/window_open_apitest.cc b/chrome/browser/extensions/window_open_apitest.cc
index f0ef19c..5de474b 100644
--- a/chrome/browser/extensions/window_open_apitest.cc
+++ b/chrome/browser/extensions/window_open_apitest.cc
@@ -11,8 +11,7 @@
 #include "chrome/test/ui_test_utils.h"
 #include "net/base/mock_host_resolver.h"
 
-// Disabled, http://crbug.com/60156
-IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_WindowOpen) {
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, WindowOpen) {
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableExperimentalExtensionApis);
 
diff --git a/chrome/test/data/extensions/api_test/window_open/spanning/infobar.html b/chrome/test/data/extensions/api_test/window_open/spanning/infobar.html
index f5d4a60..ac00402 100644
--- a/chrome/test/data/extensions/api_test/window_open/spanning/infobar.html
+++ b/chrome/test/data/extensions/api_test/window_open/spanning/infobar.html
@@ -1,5 +1,7 @@
 <script>
-chrome.test.listenOnce(chrome.tabs.onCreated,
-                       chrome.extension.getBackgroundPage().verifyCreatedTab);
+chrome.tabs.onCreated.addListener(function() {
+  chrome.tabs.onCreated.removeListener(arguments.callee);
+  chrome.extension.getBackgroundPage().verifyCreatedTab.apply(null, arguments);
+});
 window.open('foo.html');
 </script>
-- 
cgit v1.1