Implement initial chrome.browserAction.openPopup API.

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 <string>
-
 #include "base/base64.h"
 #include "base/lazy_instance.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/extension_action/extension_page_actions_api_constants.h"
 #include "chrome/browser/extensions/event_router.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_function_registry.h"
+#include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/location_bar_controller.h"
 #include "chrome/browser/extensions/state_store.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "chrome/common/render_messages.h"
 #include "content/public/browser/navigation_entry.h"
@@ skipping 22 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 kNoPageActionError[] =
     "This extension has no page action specified.";
 const char kUrlNotActiveError[] = "This url is no longer active: *.";
+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 130 matching lines @@
   registry->RegisterFunction<BrowserActionSetTitleFunction>();
   registry->RegisterFunction<BrowserActionSetBadgeTextFunction>();
   registry->RegisterFunction<BrowserActionSetBadgeBackgroundColorFunction>();
   registry->RegisterFunction<BrowserActionSetPopupFunction>();
   registry->RegisterFunction<BrowserActionGetTitleFunction>();
   registry->RegisterFunction<BrowserActionGetBadgeTextFunction>();
   registry->RegisterFunction<BrowserActionGetBadgeBackgroundColorFunction>();
   registry->RegisterFunction<BrowserActionGetPopupFunction>();
   registry->RegisterFunction<BrowserActionEnableFunction>();
   registry->RegisterFunction<BrowserActionDisableFunction>();
+  registry->RegisterFunction<BrowserActionOpenPopupFunction>();
 
   // Page Actions
   registry->RegisterFunction<EnablePageActionsFunction>();
   registry->RegisterFunction<DisablePageActionsFunction>();
   registry->RegisterFunction<PageActionShowFunction>();
   registry->RegisterFunction<PageActionHideFunction>();
   registry->RegisterFunction<PageActionSetIconFunction>();
   registry->RegisterFunction<PageActionSetTitleFunction>();
   registry->RegisterFunction<PageActionSetPopupFunction>();
   registry->RegisterFunction<PageActionGetTitleFunction>();
@@ skipping 576 matching lines @@
   list->Append(
       new base::FundamentalValue(static_cast<int>(SkColorGetG(color))));
   list->Append(
       new base::FundamentalValue(static_cast<int>(SkColorGetB(color))));
   list->Append(
       new base::FundamentalValue(static_cast<int>(SkColorGetA(color))));
   SetResult(list);
   return true;
 }
 
+BrowserActionOpenPopupFunction::BrowserActionOpenPopupFunction()
+    : response_sent_(false) {
+}
+
+bool BrowserActionOpenPopupFunction::RunImpl() {
+  ExtensionToolbarModel* model = extensions::ExtensionSystem::Get(profile_)->
+      extension_service()->toolbar_model();
+  if (!model) {
+    error_ = kInternalError;
+    return false;
+  }
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
+      content::Source<Profile>(profile_));
+  model->ShowBrowserActionPopup(extension_);
+
+  // 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
+  // bindings for response callback.
+  base::MessageLoopForUI::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&BrowserActionOpenPopupFunction::OpenPopupTimedOut, this),
+      base::TimeDelta::FromSeconds(1));
+  return true;
+}
+
+void BrowserActionOpenPopupFunction::OpenPopupTimedOut() {
+  if (response_sent_)
+    return;
+
+  DVLOG(1) << "chrome.browserAction.openPopup did not show a popup.";
+  // Custom binding will still try to get the popup, but may fail.
+  SendResponse(new base::ListValue());
+  response_sent_ = true;
+}
+
+void BrowserActionOpenPopupFunction::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(type, chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING);
+  if (response_sent_)
+    return;
+
+  ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
+  if (host->extension()->id() != extension_->id())
+    return;
+
+  SendResponse(new base::ListValue());

[mark a. foltz, 2013/10/16 19:00:53]

Is this what you intended?  The signature for ExtensionFunction::SendResponse
is:

virtual void SendResponse(bool success) = 0;

[justinlin, 2013/10/16 19:31:29]

Oh good catch. So, apparently base::Value* can be implicitly cast to bool..
that's somewhat unsettling.

+  response_sent_ = true;

[mark a. foltz, 2013/10/16 19:00:53]

You could also check for NULL-ness of results_, but that may be adding a
dependency on an implementation detail of ExtensionFunction you don't want.

[justinlin, 2013/10/16 19:31:29]

I think it's okay not to set result here since we don't need or use it anyway.

+  // Don't need to de-register notification listener since the last ref for this

[mark a. foltz, 2013/10/16 19:00:53]

Would it hurt to de-register it anyway?

[justinlin, 2013/10/16 19:31:29]

Done.

+  // object will go away after OpenPopupTimedOut is called.
+}
+
 //
 // ScriptBadgeGetAttentionFunction
 //
 
 ScriptBadgeGetAttentionFunction::~ScriptBadgeGetAttentionFunction() {}
 
 bool ScriptBadgeGetAttentionFunction::RunExtensionAction() {
   tab_helper().location_bar_controller()->GetAttentionFor(extension_id());
   return true;
 }
@@ skipping 66 matching lines @@
   return true;
 }
 
 bool EnablePageActionsFunction::RunImpl() {
   return SetPageActionEnabled(true);
 }
 
 bool DisablePageActionsFunction::RunImpl() {
   return SetPageActionEnabled(false);
 }