Initial skeleton for System Indicator API

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

Index: chrome/browser/extensions/api/extension_action/extension_actions_api.cc
diff --git a/chrome/browser/extensions/api/extension_action/extension_actions_api.cc b/chrome/browser/extensions/api/extension_action/extension_actions_api.cc
index c9c0b846c3a06060d89f27a2335328f93ac870cd..f640799d0b2dc9bb6044f93ea698ad3d853561dd 100644
--- a/chrome/browser/extensions/api/extension_action/extension_actions_api.cc
+++ b/chrome/browser/extensions/api/extension_action/extension_actions_api.cc
@@ -286,6 +286,9 @@ bool ExtensionActionFunction::RunImpl() {
   if (base::StringPiece(name()).starts_with("scriptBadge.")) {
     extension_action_ = extensions::ExtensionActionManager::Get(profile_)->
         GetScriptBadge(*GetExtension());
+  } else if (base::StringPiece(name()).starts_with("systemIndicator.")) {
+    extension_action_ = extensions::ExtensionActionManager::Get(profile_)->
+        GetSystemIndicator(*GetExtension());
   } else {
     extension_action_ = extensions::ExtensionActionManager::Get(profile_)->
         GetBrowserAction(*GetExtension());
@@ -302,51 +305,20 @@ bool ExtensionActionFunction::RunImpl() {
     return false;
   }
 
-  // There may or may not be details (depends on the function).
-  // The tabId might appear in details (if it exists) or as the first
-  // argument besides the action type (depends on the function).
-  {
-    base::Value* first_arg = NULL;
-    EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &first_arg));
-
-    switch (first_arg->GetType()) {
-      case Value::TYPE_INTEGER:
-        CHECK(first_arg->GetAsInteger(&tab_id_));
-        break;
-
-      case Value::TYPE_DICTIONARY: {
-        details_ = static_cast<base::DictionaryValue*>(first_arg);
-        base::Value* tab_id_value = NULL;
-        if (details_->Get("tabId", &tab_id_value)) {
-          switch (tab_id_value->GetType()) {
-            case Value::TYPE_NULL:
-              // Fine, equivalent to it being not-there, and tabId is optional.
-              break;
-            case Value::TYPE_INTEGER:
-              CHECK(tab_id_value->GetAsInteger(&tab_id_));
-              break;
-            default:
-              // Boom.
-              EXTENSION_FUNCTION_VALIDATE(false);
-          }
-        }
-        break;
-      }
-
-      case Value::TYPE_NULL:
-        // The tabId might be an optional argument.
-        break;
-
-      default:
-        EXTENSION_FUNCTION_VALIDATE(false);
-    }
+  if (!ValidateArguments()) {
+    return false;

[not at google - send to devlin, 2012/11/21 23:39:23]

EXTENSION_FUNCTION_VALIDATE(ValidateArguments())

[dewittj, 2012/11/26 18:32:58]

Done.  Renamed to more accurately reflect what's going on.

   }
 
   // Find the WebContents that contains this tab id if one is required.
   if (tab_id_ == ExtensionAction::kDefaultTabId) {
-    EXTENSION_FUNCTION_VALIDATE(
-        extensions::ExtensionActionManager::Get(profile_)->
-        GetBrowserAction(*GetExtension()));
+    extensions::ExtensionActionManager* mgr =
+        extensions::ExtensionActionManager::Get(profile_);

[not at google - send to devlin, 2012/11/21 23:39:23]

nit: do this at top, then re-use is everywhere else above given
extensions::ExtensionActionManager::Get.

also, the abbr is a bit unnecessary, just "manager" is fine.

[dewittj, 2012/11/26 18:32:58]

Done.

+    if (extension_action_->action_type() ==
+        extensions::Extension::ActionInfo::TYPE_BROWSER) {
+      EXTENSION_FUNCTION_VALIDATE(mgr->GetBrowserAction(*GetExtension()));
+    } else {
+      EXTENSION_FUNCTION_VALIDATE(mgr->GetSystemIndicator(*GetExtension()));

[not at google - send to devlin, 2012/11/21 23:39:23]

actually... a more sensible check here is probably just

EXTENSION_FUNCTION_VALIDATE(type == BROWER ||
                            type == SYSTEM_INDICATOR)

since we've already validated there is an action associated with that value.

[dewittj, 2012/11/26 18:32:58]

Good idea. Done.

+    }
   } else {
     ExtensionTabUtil::GetTabById(
         tab_id_, profile(), include_incognito(), NULL, NULL, &contents_, NULL);
@@ -360,6 +332,48 @@ bool ExtensionActionFunction::RunImpl() {
   return RunExtensionAction();
 }
 
+bool ExtensionActionFunction::ValidateArguments() {
+  // There may or may not be details (depends on the function).
+  // The tabId might appear in details (if it exists) or as the first
+  // argument besides the action type (depends on the function).
+  base::Value* first_arg = NULL;
+  EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &first_arg));

[not at google - send to devlin, 2012/11/21 23:39:23]

let's push up the call to EXTENSION_FUNCTION_VALIDATE to the caller, and just
return bools from here.

[dewittj, 2012/11/26 18:32:58]

Done.

+
+  switch (first_arg->GetType()) {
+    case Value::TYPE_INTEGER:
+      CHECK(first_arg->GetAsInteger(&tab_id_));
+      break;
+
+    case Value::TYPE_DICTIONARY: {
+      details_ = static_cast<base::DictionaryValue*>(first_arg);
+      base::Value* tab_id_value = NULL;
+      if (details_->Get("tabId", &tab_id_value)) {
+        switch (tab_id_value->GetType()) {
+          case Value::TYPE_NULL:
+            // Fine, equivalent to it being not-there, and tabId is optional.
+            break;
+          case Value::TYPE_INTEGER:
+            CHECK(tab_id_value->GetAsInteger(&tab_id_));
+            break;
+          default:
+            // Boom.
+            EXTENSION_FUNCTION_VALIDATE(false);
+        }
+      }
+      break;
+    }
+
+    case Value::TYPE_NULL:
+      // The tabId might be an optional argument.
+      break;
+
+    default:
+      EXTENSION_FUNCTION_VALIDATE(false);
+  }
+
+  return true;
+}
+
 void ExtensionActionFunction::NotifyChange() {
   switch (extension_action_->action_type()) {
     case extensions::Extension::ActionInfo::TYPE_BROWSER:
@@ -375,6 +389,9 @@ void ExtensionActionFunction::NotifyChange() {
     case extensions::Extension::ActionInfo::TYPE_SCRIPT_BADGE:
       NotifyLocationBarChange();
       return;
+    case extensions::Extension::ActionInfo::TYPE_SYSTEM_INDICATOR:
+      NotifyStatusTrayChange();
+      return;
   }
   NOTREACHED();
 }
@@ -391,6 +408,9 @@ void ExtensionActionFunction::NotifyLocationBarChange() {
       location_bar_controller()->NotifyChange();
 }
 
+void ExtensionActionFunction::NotifyStatusTrayChange() {

[not at google - send to devlin, 2012/11/21 23:39:23]

TODO(dewittj): implement (see http://crbug.com/142450).

or whatever

[dewittj, 2012/11/26 18:32:58]

Done.

+}
+
 // static
 bool ExtensionActionFunction::ParseCSSColorString(
     const std::string& color_string,