Make UI or API driven extension options navigations refocus any existing options page.
This is already the way that the options pages embedded in chrome://extensions
work. Options pages not embedded (those declared via "options_page" or with
{"open_in_tab": true} in "options_ui") currently always open in a new tab - now
it will refocus instead.
This will affect any caller of ExtensionTabUtil::OpenOptionsPage - called from
UI like the toolbar button's context menu, and from the
chrome.browser.openOptionsPage API call.
An important side-effect of all of this is that the old behavior is buggy when
a devtools window is focused (see crash report), and this fixes it. This was
impossible to trigger until we added the chrome.browser.openOptionsPage() - now
quite likely that a devtools window is focused.
BUG=430054, 463341
R=rockot@chromium.org
Review URL: https://codereview.chromium.org/1005673002
Cr-Commit-Position: refs/heads/master@{#320426}
diff --git a/chrome/browser/extensions/extension_tab_util.cc b/chrome/browser/extensions/extension_tab_util.cc
index d5feb70..706f41e 100644
--- a/chrome/browser/extensions/extension_tab_util.cc
+++ b/chrome/browser/extensions/extension_tab_util.cc
@@ -590,35 +590,26 @@
browser = displayer->browser();
}
- if (!OptionsPageInfo::ShouldOpenInTab(extension)) {
- // If we should embed the options page for this extension, open
- // chrome://extensions in a new tab and show the extension options in an
- // embedded popup.
- chrome::NavigateParams params(chrome::GetSingletonTabNavigateParams(
- browser, GURL(chrome::kChromeUIExtensionsURL)));
- params.path_behavior = chrome::NavigateParams::IGNORE_AND_NAVIGATE;
-
+ GURL url_to_navigate;
+ if (OptionsPageInfo::ShouldOpenInTab(extension)) {
+ // Options page tab is simply e.g. chrome-extension://.../options.html.
+ url_to_navigate = OptionsPageInfo::GetOptionsPage(extension);
+ } else {
+ // Options page tab is Extension settings pointed at that Extension's ID,
+ // e.g. chrome://extensions?options=...
+ url_to_navigate = GURL(chrome::kChromeUIExtensionsURL);
GURL::Replacements replacements;
std::string query =
base::StringPrintf("options=%s", extension->id().c_str());
replacements.SetQueryStr(query);
- params.url = params.url.ReplaceComponents(replacements);
-
- chrome::ShowSingletonTabOverwritingNTP(browser, params);
- } else {
- // Otherwise open a new tab with the extension's options page
- content::OpenURLParams params(OptionsPageInfo::GetOptionsPage(extension),
- content::Referrer(),
- SINGLETON_TAB,
- ui::PAGE_TRANSITION_LINK,
- false);
- browser->OpenURL(params);
- browser->window()->Show();
- WebContents* web_contents =
- browser->tab_strip_model()->GetActiveWebContents();
- web_contents->GetDelegate()->ActivateContents(web_contents);
+ url_to_navigate = url_to_navigate.ReplaceComponents(replacements);
}
+ chrome::NavigateParams params(
+ chrome::GetSingletonTabNavigateParams(browser, url_to_navigate));
+ params.path_behavior = chrome::NavigateParams::IGNORE_AND_NAVIGATE;
+ params.url = url_to_navigate;
+ chrome::ShowSingletonTabOverwritingNTP(browser, params);
return true;
}
diff --git a/chrome/test/data/extensions/api_test/runtime/open_options_page/manifest.json b/chrome/test/data/extensions/api_test/runtime/open_options_page/manifest.json
index ea13b2b..81f6f54 100644
--- a/chrome/test/data/extensions/api_test/runtime/open_options_page/manifest.json
+++ b/chrome/test/data/extensions/api_test/runtime/open_options_page/manifest.json
@@ -8,5 +8,6 @@
},
"options_ui": {
"page": "options.html"
- }
+ },
+ "permissions": ["tabs"]
}
diff --git a/chrome/test/data/extensions/api_test/runtime/open_options_page/test.js b/chrome/test/data/extensions/api_test/runtime/open_options_page/test.js
index 471da12..049167a8 100644
--- a/chrome/test/data/extensions/api_test/runtime/open_options_page/test.js
+++ b/chrome/test/data/extensions/api_test/runtime/open_options_page/test.js
@@ -5,14 +5,64 @@
// browser_tests --gtest_filter=ExtensionApiTest.OpenOptionsPage
var assertEq = chrome.test.assertEq;
+var assertTrue = chrome.test.assertTrue;
+var listenOnce = chrome.test.listenOnce;
+var pass = chrome.test.callbackPass;
-function test() {
- chrome.test.listenOnce(chrome.runtime.onMessage, function(m, sender) {
- assertEq('success', m);
- assertEq(chrome.runtime.id, sender.id);
- assertEq(chrome.runtime.getURL('options.html'), sender.url);
- });
- chrome.runtime.openOptionsPage();
+var optionsTabUrl = 'chrome://extensions/?options=' + chrome.runtime.id;
+
+// Finds the Tab for an options page, or null if no options page is open.
+// Asserts that there is at most 1 options page open.
+// Result is passed to |callback|.
+function findOptionsTab(callback) {
+ chrome.tabs.query({url: optionsTabUrl}, pass(function(tabs) {
+ assertTrue(tabs.length <= 1);
+ callback(tabs.length == 0 ? null : tabs[0]);
+ }));
}
-chrome.test.runTests([test]);
+// Tests opening a new options page.
+function testNewOptionsPage() {
+ findOptionsTab(function(tab) {
+ assertEq(null, tab);
+ listenOnce(chrome.runtime.onMessage, function(m, sender) {
+ assertEq('success', m);
+ assertEq(chrome.runtime.id, sender.id);
+ assertEq(chrome.runtime.getURL('options.html'), sender.url);
+ });
+ chrome.runtime.openOptionsPage();
+ });
+}
+
+// Gets the active tab, or null if no tab is active. Asserts that there is at
+// most 1 active tab. Result is passed to |callback|.
+function getActiveTab(callback) {
+ chrome.tabs.query({active: true}, pass(function(tabs) {
+ assertTrue(tabs.length <= 1);
+ callback(tabs.length == 0 ? null : tabs[0]);
+ }));
+}
+
+// Tests refocusing an existing page.
+function testRefocusExistingOptionsPage() {
+ var testUrl = 'chrome://chrome/';
+
+ // There will already be an options page open from the last test. Find it,
+ // focus away from it, then make sure openOptionsPage() refocuses it.
+ findOptionsTab(function(optionsTab) {
+ assertTrue(optionsTab != null);
+ chrome.tabs.create({url: testUrl}, pass(function(tab) {
+ // Make sure the new tab is active.
+ getActiveTab(function(activeTab) {
+ assertEq(testUrl, activeTab.url);
+ // Open options page should refocus it.
+ chrome.runtime.openOptionsPage();
+ getActiveTab(function(activeTab) {
+ assertEq(optionsTabUrl, activeTab.url);
+ });
+ });
+ }));
+ });
+}
+
+chrome.test.runTests([testNewOptionsPage, testRefocusExistingOptionsPage]);
