Initial skeleton for System Indicator API

chrome/browser/extensions/api/extension_action/extension_actions_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_actions_api.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/string_number_conversions.h"
@@ skipping 268 matching lines @@
       extension_action_(NULL) {
 }
 
 ExtensionActionFunction::~ExtensionActionFunction() {
 }
 
 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());
     if (!extension_action_) {
       extension_action_ = extensions::ExtensionActionManager::Get(profile_)->
           GetPageAction(*GetExtension());
     }
   }
   if (!extension_action_) {
     // TODO(kalman): ideally the browserAction/pageAction APIs wouldn't event
     // exist for extensions that don't have one declared. This should come as
     // part of the Feature system.

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

This can be convered to a EXTENSION_FUNCTION_VALIDATE when you change all
extension actions to be like the system indicator one in extension.cc.

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

OK

     error_ = kNoExtensionActionError;
     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);
     if (!contents_) {
       error_ = extensions::ErrorUtils::FormatErrorMessage(
           kNoTabError, base::IntToString(tab_id_));
       return false;
     }
   }
 
   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:
     case extensions::Extension::ActionInfo::TYPE_PAGE:
       if (extensions::ExtensionActionManager::Get(profile_)->
           GetBrowserAction(*extension_)) {
         NotifyBrowserActionChange();
       } else if (extensions::ExtensionActionManager::Get(profile_)->
                  GetPageAction(*extension_)) {
         NotifyLocationBarChange();
       }
       return;
     case extensions::Extension::ActionInfo::TYPE_SCRIPT_BADGE:
       NotifyLocationBarChange();
       return;
+    case extensions::Extension::ActionInfo::TYPE_SYSTEM_INDICATOR:
+      NotifyStatusTrayChange();
+      return;
   }
   NOTREACHED();
 }
 
 void ExtensionActionFunction::NotifyBrowserActionChange() {
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_UPDATED,
       content::Source<ExtensionAction>(extension_action_),
       content::Details<Profile>(profile()));
 }
 
 void ExtensionActionFunction::NotifyLocationBarChange() {
   extensions::TabHelper::FromWebContents(contents_)->
       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,
     SkColor* result) {
   std::string formatted_color = "#";
   // Check the string for incorrect formatting.
   if (color_string[0] != '#')
     return false;
 
   // Convert the string from #FFF format to #FFFFFF format.
@@ skipping 151 matching lines @@
 bool ExtensionActionGetBadgeBackgroundColorFunction::RunExtensionAction() {
   ListValue* list = new ListValue();
   SkColor color = extension_action_->GetBadgeBackgroundColor(tab_id_);
   list->Append(Value::CreateIntegerValue(SkColorGetR(color)));
   list->Append(Value::CreateIntegerValue(SkColorGetG(color)));
   list->Append(Value::CreateIntegerValue(SkColorGetB(color)));
   list->Append(Value::CreateIntegerValue(SkColorGetA(color)));
   SetResult(list);
   return true;
 }