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 265 matching lines @@
     : details_(NULL),
       tab_id_(ExtensionAction::kDefaultTabId),
       contents_(NULL),
       extension_action_(NULL) {
 }
 
 ExtensionActionFunction::~ExtensionActionFunction() {
 }
 
 bool ExtensionActionFunction::RunImpl() {
-  if (base::StringPiece(name()).starts_with("scriptBadge.")) {
-    extension_action_ = extensions::ExtensionActionManager::Get(profile_)->
-        GetScriptBadge(*GetExtension());
+  extensions::ExtensionActionManager* manager =
+      extensions::ExtensionActionManager::Get(profile_);
+  const extensions::Extension* extension = GetExtension();
+  base::StringPiece functionName(name());

[not at google - send to devlin, 2012/11/26 19:44:54]

chromium is unix_style not camelCase.

however, name() is just a simple accessor:
https://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/extensions...

so using StartsWith(name(), "...") from string_util.h is probably more
appropriate than constructing a new StringPiece.

[dewittj, 2012/11/27 01:53:55]

Done. StartsWithASCII added.

+
+  if (functionName.starts_with("scriptBadge.")) {
+    extension_action_ = manager->GetScriptBadge(*extension);
+  } else if (functionName.starts_with("systemIndicator.")) {
+    extension_action_ = manager->GetSystemIndicator(*extension);
   } else {
-    extension_action_ = extensions::ExtensionActionManager::Get(profile_)->
-        GetBrowserAction(*GetExtension());
+    extension_action_ = manager->GetBrowserAction(*extension);
     if (!extension_action_) {
-      extension_action_ = extensions::ExtensionActionManager::Get(profile_)->
-          GetPageAction(*GetExtension());
+      extension_action_ = manager->GetPageAction(*extension);
     }
   }
   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.
     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);
-    }
-  }
+  // Populates the tab_id_ and details_ members.
+  EXTENSION_FUNCTION_VALIDATE(ExtractDataFromArguments());
 
   // 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()));
-  } else {
+  if (tab_id_ != ExtensionAction::kDefaultTabId) {
     ExtensionTabUtil::GetTabById(
         tab_id_, profile(), include_incognito(), NULL, NULL, &contents_, NULL);
     if (!contents_) {
       error_ = extensions::ErrorUtils::FormatErrorMessage(
           kNoTabError, base::IntToString(tab_id_));
       return false;
     }
+  } else {
+    // Only browser actions and system indicators have a default tabId.
+    const extensions::Extension::ActionInfo::Type type_browser =
+        extensions::Extension::ActionInfo::TYPE_BROWSER;
+    const extensions::Extension::ActionInfo::Type type_system_indicator =
+        extensions::Extension::ActionInfo::TYPE_SYSTEM_INDICATOR;

[not at google - send to devlin, 2012/11/26 19:44:54]

this is just making it more verbose?

[dewittj, 2012/11/27 01:53:55]

This is making it fit in the 80 column limit.  Replaced with a block-local
typedef.

+    extensions::Extension::ActionInfo::Type action_type =
+        extension_action_->action_type();
+
+    EXTENSION_FUNCTION_VALIDATE(
+        action_type == type_browser || action_type == type_system_indicator);
+  }
+  return RunExtensionAction();
+}
+
+bool ExtensionActionFunction::ExtractDataFromArguments() {
+  // There may or may not be details (depends on the function).
+  // The tabId might appear in details (if it exists), as the first
+  // argument besides the action type (depends on the function), or be omitted
+  // entirely.
+  base::Value* first_arg = NULL;
+  if (!args_->Get(0, &first_arg)) {
+    return true;
   }

[not at google - send to devlin, 2012/11/26 19:44:54]

generally leave out {} if it's just a 1 line body.

[dewittj, 2012/11/27 01:53:55]

Done.

 
-  return RunExtensionAction();
+  switch (first_arg->GetType()) {
+    case Value::TYPE_INTEGER:
+      CHECK(first_arg->GetAsInteger(&tab_id_));
+      break;
+
+    case Value::TYPE_DICTIONARY: {
+      // Found the details argument.
+      details_ = static_cast<base::DictionaryValue*>(first_arg);
+      // Still need to check for the tabId within details.
+      base::Value* tab_id_value = NULL;
+      if (details_->Get("tabId", &tab_id_value)) {
+        switch (tab_id_value->GetType()) {
+          case Value::TYPE_NULL:
+            // OK; tabId is optional, leave it default.
+            return true;
+          case Value::TYPE_INTEGER:
+            CHECK(tab_id_value->GetAsInteger(&tab_id_));
+            return true;
+          default:
+            // Boom.
+            return false;
+        }
+      }
+      // Not found; tabId is optional, leave it default.
+      break;
+    }
+
+    case Value::TYPE_NULL:
+      // The tabId might be an optional argument.
+      break;
+
+    default:
+      return 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() {
+  // TODO(dewittj) Implement status tray behavior here.
+  // See http://crbug.com/142450.
+}
+
 // 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;
 }