Make a ShowExtensionActionPopup function

chrome/browser/extensions/api/extension_action/extension_action_api.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
 
 #include "base/base64.h"
 #include "base/lazy_instance.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ skipping 18 matching lines @@
 #include "content/public/browser/notification_service.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_function_registry.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/image_util.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/browser/state_store.h"
 #include "extensions/common/error_utils.h"
+#include "extensions/common/feature_switch.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
 
 using content::WebContents;
 
 namespace page_actions_keys = extension_page_actions_api_constants;
 
 namespace extensions {
 
@@ skipping 24 matching lines @@
 // Whether the browser action is visible in the toolbar.
 const char kBrowserActionVisible[] = "browser_action_visible";
 
 // Errors.
 const char kNoExtensionActionError[] =
     "This extension has no action specified.";
 const char kNoTabError[] = "No tab with id: *.";
 const char kOpenPopupError[] =
     "Failed to show popup either because there is an existing popup or another "
     "error occurred.";
-const char kInternalError[] = "Internal error.";
 
 struct IconRepresentationInfo {
   // Size as a string that will be used to retrieve representation value from
   // SetIcon function arguments.
   const char* size_string;
   // Scale factor for which the represantion should be used.
   ui::ScaleFactor scale;
 };
 
 const IconRepresentationInfo kIconSizes[] = {
@@ skipping 252 matching lines @@
     const Extension* extension,
     Browser* browser,
     bool grant_active_tab_permissions) {
   content::WebContents* web_contents =
       browser->tab_strip_model()->GetActiveWebContents();
   if (!web_contents)
     return ExtensionAction::ACTION_NONE;
 
   int tab_id = SessionTabHelper::IdForTab(web_contents);
 
-  ExtensionActionManager* action_manager =
-      ExtensionActionManager::Get(static_cast<Profile*>(browser_context_));
-  // TODO(devlin): Make a ExtensionActionManager::GetExtensionAction method.
   ExtensionAction* extension_action =
-      action_manager->GetBrowserAction(*extension);
-  if (!extension_action)
-    extension_action = action_manager->GetPageAction(*extension);
+      ExtensionActionManager::Get(browser_context_)->GetExtensionAction(
+          *extension);
 
   // Anything that calls this should have a page or browser action.
   DCHECK(extension_action);
   if (!extension_action->GetIsVisible(tab_id))
     return ExtensionAction::ACTION_NONE;
 
   // Grant active tab if appropriate.
   if (grant_active_tab_permissions) {
     TabHelper::FromWebContents(web_contents)->active_tab_permission_granter()->
         GrantIfRequested(extension);
   }
 
   // Notify ActiveScriptController that the action was clicked, if appropriate.
   ActiveScriptController* active_script_controller =
       ActiveScriptController::GetForWebContents(web_contents);
   if (active_script_controller &&
       active_script_controller->GetActionForExtension(extension)) {
     active_script_controller->OnClicked(extension);
   }
 
   if (extension_action->HasPopup(tab_id))
     return ExtensionAction::ACTION_SHOW_POPUP;
 
   ExtensionActionExecuted(*extension_action, web_contents);
   return ExtensionAction::ACTION_NONE;
 }
 
+bool ExtensionActionAPI::ShowExtensionActionPopup(
+    const Extension* extension,
+    Browser* browser,
+    bool grant_active_tab_permissions) {
+  ExtensionAction* extension_action =
+      ExtensionActionManager::Get(browser_context_)->GetExtensionAction(
+          *extension);
+  if (!extension_action)
+    return false;
+
+  if (extension_action->action_type() == ActionInfo::TYPE_PAGE &&
+      !FeatureSwitch::extension_action_redesign()->IsEnabled()) {
+    // We show page actions in the location bar unless the new toolbar is
+    // enabled.
+    return browser->window()->GetLocationBar()->ShowPageActionPopup(
+        extension, grant_active_tab_permissions);
+  } else {
+    return ExtensionToolbarModel::Get(browser->profile())->
+        ShowExtensionActionPopup(
+            extension, browser, grant_active_tab_permissions);
+  }
+}
+
 void ExtensionActionAPI::NotifyChange(ExtensionAction* extension_action,
                                       content::WebContents* web_contents,
                                       content::BrowserContext* context) {
   FOR_EACH_OBSERVER(
       Observer,
       observers_,
       OnExtensionActionUpdated(extension_action, web_contents, context));
 
   if (extension_action->action_type() == ActionInfo::TYPE_PAGE)
     NotifyPageActionsChanged(web_contents);
@@ skipping 426 matching lines @@
       new base::FundamentalValue(static_cast<int>(SkColorGetA(color))));
   SetResult(list);
   return true;
 }
 
 BrowserActionOpenPopupFunction::BrowserActionOpenPopupFunction()
     : response_sent_(false) {
 }
 
 bool BrowserActionOpenPopupFunction::RunAsync() {
-  ExtensionToolbarModel* model = ExtensionToolbarModel::Get(GetProfile());
-  if (!model) {
-    error_ = kInternalError;
-    return false;
-  }
+  // We only allow the a popup in the active window.

[Finnur, 2014/08/27 11:53:02]

s/the a/the/

[Devlin, 2014/08/27 15:43:22]

Done.

+  Browser* browser = chrome::FindLastActiveWithProfile(

[Finnur, 2014/08/27 11:53:02]

Hmmm... is there no way around looking up the browser object?

[Devlin, 2014/08/27 15:43:22]

Sadly, I don't think so, no :(  I think there are a few flaws of the
chrome.browserAction.showPopup() function, one of which is that it doesn't take
a tab id and simply tries to open in the active browser, whichever that happens
to be.  Since it can be called from any context, I think the only way to find
the active browser is to look it up directly.

This is pretty similar to the old behavior, in which we'd ask every single
browser (indirectly, via a BrowserActionsContainer) to display the popup, and it
would iff it was the active browser.

Of course, I'm constantly discovering new functions here.  Is there an easier
way to find the active Browser?

+                         GetProfile(), chrome::GetActiveDesktop());
 
-  if (!model->ShowBrowserActionPopup(extension_.get())) {
+  // If there's no active browser, or the Toolbar isn't visible, abort.
+  // Otherwise, try to open a popup in the active browser.
+  // TODO(justinlin): Remove toolbar check when http://crbug.com/308645 is
+  // fixed.
+  if (!browser ||
+      !browser->window()->IsActive() ||
+      !browser->window()->IsToolbarVisible() ||
+      !ExtensionActionAPI::Get(GetProfile())->ShowExtensionActionPopup(
+          extension_.get(), browser, false)) {
     error_ = kOpenPopupError;
     return false;
   }
 
   registrar_.Add(this,
                  NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
                  content::Source<Profile>(GetProfile()));
 
   // Set a timeout for waiting for the notification that the popup is loaded.
   // Waiting is required so that the popup view can be retrieved by the custom
@@ skipping 28 matching lines @@
   if (host->extension_host_type() != VIEW_TYPE_EXTENSION_POPUP ||
       host->extension()->id() != extension_->id())
     return;
 
   SendResponse(true);
   response_sent_ = true;
   registrar_.RemoveAll();
 }
 
 }  // namespace extensions