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_);
+
+  // Dispatch notification for popup not having been opened after a timeout.
+  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());

[Finnur, 2013/10/16 15:01:26]

It seems like we're jumping through hoops just to execute a DVLOG command, which
doesn't seem useful. You might as well just call ShowBrowserActionPopup and call
it a day. I don't see the caller benefiting from all this extra complexity.

Actually, wait. Is this because of the view returned in custom bindings? You
might want to document that that's whats going on behind the scenes. But this
seems brittle though... 1 sec, will that suffice in tests under memory bots
(which slow everything down)?

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

Hmm, that's a good point and right it's for the custom bindings. We can't just
send a response right away because we won't have a popup view to return yet.
This either waits for the loaded notification (which I'm hoping won't take over
a second on memory bots) or a second (where the popup might already be loaded by
the time we send the response). Clarified the comment.

+  response_sent_ = true;
+}
+
+void BrowserActionOpenPopupFunction::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  if (response_sent_ ||
+      type != chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING)
+    return;

[Finnur, 2013/10/16 15:01:26]

Should be enough to DCHECK at the top that type == HOST_DID_STOP_LOADING. 

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

Done.

+
+  ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
+  if (host->extension()->id() != extension_->id())
+    return;
+
+  SendResponse(new base::ListValue());
+  response_sent_ = true;
+  // Don't need to de-register notification listener since the last ref for this
+  // 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);
 }