Open embedded extension options from the extension action context menu

chrome/browser/extensions/extension_tab_util.cc

Index: chrome/browser/extensions/extension_tab_util.cc
diff --git a/chrome/browser/extensions/extension_tab_util.cc b/chrome/browser/extensions/extension_tab_util.cc
index 75991084362ed7a7bf035ce92d800d3c9c5b584b..c8c6e81f470c1b4b6015883abd041d223fff1134 100644
--- a/chrome/browser/extensions/extension_tab_util.cc
+++ b/chrome/browser/extensions/extension_tab_util.cc
@@ -7,6 +7,7 @@
 #include "apps/app_window.h"
 #include "apps/app_window_registry.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/tabs/tabs_constants.h"
 #include "chrome/browser/extensions/chrome_extension_function.h"
 #include "chrome/browser/extensions/tab_helper.h"
@@ -19,6 +20,7 @@
 #include "chrome/browser/ui/browser_iterator.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
+#include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_iterator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/extensions/api/tabs.h"
@@ -31,6 +33,7 @@
 #include "extensions/common/constants.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/manifest_handlers/incognito_info.h"
 #include "extensions/common/permissions/api_permission.h"
@@ -566,16 +569,33 @@ void ExtensionTabUtil::OpenOptionsPage(const Extension* extension,
     browser = displayer->browser();
   }
 
-  content::OpenURLParams params(ManifestURL::GetOptionsPage(extension),
-                                content::Referrer(),
-                                SINGLETON_TAB,
-                                content::PAGE_TRANSITION_LINK,
-                                false);
-  browser->OpenURL(params);
-  browser->window()->Show();
-  WebContents* web_contents =
-      browser->tab_strip_model()->GetActiveWebContents();
-  web_contents->GetDelegate()->ActivateContents(web_contents);
+  if (FeatureSwitch::embedded_extension_options()->IsEnabled()) {
+    // If embedded extension options are enabled, 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;
+    if (!extension->id().empty()) {

[not at google - send to devlin, 2014/08/25 18:11:07]

I don't think it's possible for Extensions to have an empty ID. Is this
defensive or did you run into it?

[ericzeng, 2014/08/25 21:42:34]

This part was adapted from chrome_pages.cc, which handles the 'Manage' entry in
the context menu. They do this check as well (I have no idea if its necessary). 

[not at google - send to devlin, 2014/08/25 21:53:29]

Let's kill both. They're silly.

[ericzeng, 2014/08/25 22:04:04]

But then I would need an owner for chrome/browser/ui/ :(

[not at google - send to devlin, 2014/08/25 22:07:53]

Ok, just remove this one then :)

+      GURL::Replacements replacements;
+      std::string query =
+          base::StringPrintf("options=%s", extension->id().c_str());

[not at google - send to devlin, 2014/08/25 18:11:07]

Using StringPrintf here is overkill. Just "options=" + extension->id() is fine.
I'd even inline it.

[Devlin, 2014/08/25 18:12:51]

I told him to, as I've been told repeatedly to use it (even when just concatting
two).

[not at google - send to devlin, 2014/08/25 21:53:29]

Btw Devlin and I follow up on this offline with Jeffrey.

Conclusion: a  single "+" is the same efficiency as StringPrintf, and the
readability of using StringPrintf is "debatable". IMO given it's twice the
amount of typing, "+" is much better.

2 or more "+"s is likely less efficient than StringPrintf.

Anyhow, in the spirit of not changing anything, leave it the way it is. But no
need to change it next time.

+      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(ManifestURL::GetOptionsPage(extension),
+                                  content::Referrer(),
+                                  SINGLETON_TAB,
+                                  content::PAGE_TRANSITION_LINK,
+                                  false);
+    browser->OpenURL(params);
+    browser->window()->Show();
+    WebContents* web_contents =
+        browser->tab_strip_model()->GetActiveWebContents();
+    web_contents->GetDelegate()->ActivateContents(web_contents);
+  }
 }
 
 }  // namespace extensions