summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoraa@chromium.org <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-10-26 21:06:18 +0000
committeraa@chromium.org <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-10-26 21:06:18 +0000
commit44d8ce76bed0ef853a28dc55164b8e2246f3eddc (patch)
tree6c354f2907c3f0fdea488a38ef9ec401d511b9f3
parentcd10e28d8993d2cf4dbcd1fc5534958a0ee3c84a (diff)
downloadchromium_src-44d8ce76bed0ef853a28dc55164b8e2246f3eddc.zip
chromium_src-44d8ce76bed0ef853a28dc55164b8e2246f3eddc.tar.gz
chromium_src-44d8ce76bed0ef853a28dc55164b8e2246f3eddc.tar.bz2
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
-rw-r--r--chrome/browser/extensions/extension_host.cc54
-rw-r--r--chrome/browser/extensions/window_open_apitest.cc3
-rw-r--r--chrome/test/data/extensions/api_test/window_open/spanning/infobar.html6
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>