diff options
author | aa@chromium.org <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-26 21:06:18 +0000 |
---|---|---|
committer | aa@chromium.org <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-26 21:06:18 +0000 |
commit | 44d8ce76bed0ef853a28dc55164b8e2246f3eddc (patch) | |
tree | 6c354f2907c3f0fdea488a38ef9ec401d511b9f3 | |
parent | cd10e28d8993d2cf4dbcd1fc5534958a0ee3c84a (diff) | |
download | chromium_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.cc | 54 | ||||
-rw-r--r-- | chrome/browser/extensions/window_open_apitest.cc | 3 | ||||
-rw-r--r-- | chrome/test/data/extensions/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> |