Added new fileBrowserPrivate and fileHandler extension APIs

chrome/common/extensions/extension.cc

Index: chrome/common/extensions/extension.cc
===================================================================
--- chrome/common/extensions/extension.cc	(revision 81212)
+++ chrome/common/extensions/extension.cc	(working copy)
@@ -32,6 +32,7 @@
 #include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/extensions/extension_sidebar_defaults.h"
 #include "chrome/common/extensions/extension_sidebar_utils.h"
+#include "chrome/common/extensions/file_browser_action.h"
 #include "chrome/common/extensions/user_script.h"
 #include "chrome/common/url_constants.h"
 #include "googleurl/src/url_util.h"
@@ -879,6 +880,93 @@
   return result.release();
 }
 
+Extension::FileBrowserActionList* Extension::LoadFileBrowserActions(
+    const ListValue* extension_actions, std::string* error) {
+  scoped_ptr<FileBrowserActionList> result(
+      new FileBrowserActionList());
+  for (ListValue::const_iterator iter = extension_actions->begin();
+       iter != extension_actions->end();
+       ++iter) {
+    if (!(*iter)->IsType(Value::TYPE_DICTIONARY)) {
+      *error = errors::kInvalidFileBrowserAction;
+      return false;
+    }
+    scoped_ptr<FileBrowserAction> action(
+        LoadFileBrowserAction(
+            reinterpret_cast<DictionaryValue*>(*iter), error));
+    if (!action.get())
+      return NULL;  // Failed to parse file browser action definition.
+    result->push_back(linked_ptr<FileBrowserAction>(action.release()));
+  }
+  return result.release();
+}
+
+FileBrowserAction* Extension::LoadFileBrowserAction(
+    const DictionaryValue* file_browser_actions, std::string* error) {

[Aaron Boodman, 2011/04/12 22:47:21]

s/file_browser_actions/file_browser_action/

[zel, 2011/04/13 17:49:55]

Done.

+  scoped_ptr<FileBrowserAction> result(
+      new FileBrowserAction());
+  result->set_extension_id(id());
+
+  std::string id;
+  // Read the file action |id| (mandatory).
+  if (!file_browser_actions->HasKey(keys::kPageActionId) ||

[Aaron Boodman, 2011/04/12 22:47:21]

Can you name this property "name" instead? (We don't use the old support for
page action IDs anymore, so there's no precedent, and I think name is better).

[Aaron Boodman, 2011/04/12 22:48:20]

Whoops, I didn't mean to include this comment. I thought about it some more and
worried "name" might get confused with "title", so id is fine.

+      !file_browser_actions->GetString(keys::kPageActionId, &id)) {
+    *error = errors::kInvalidPageActionId;

[Aaron Boodman, 2011/04/12 22:47:21]

Incorrect error message.

[zel, 2011/04/13 17:49:55]

This message is used for generic "id is missing" errors all across the code. We
should probably rename the variable? 

+    return NULL;
+  }
+  result->set_id(id);
+
+  // Read the page action title from |default_title| (mandatory).
+  std::string title;
+  if (!file_browser_actions->HasKey(keys::kPageActionDefaultTitle) ||

[Aaron Boodman, 2011/04/12 22:47:21]

It's a bit weird to reuse these constants (pageActionDefaultTitle), but OK.

I do like the idea of using "default_title" and "default_icon", even if we don't
have any way to change them currently, because if we do add that, it will make
sense.

[zel, 2011/04/13 17:49:55]

Done.

+      !file_browser_actions->GetString(keys::kPageActionDefaultTitle, &title)) {
+    *error = errors::kInvalidPageActionDefaultTitle;
+    return NULL;
+  }
+  result->set_default_title(title);
+
+  // Initialize file filters (mandatory).
+  ListValue* list_value;

[Aaron Boodman, 2011/04/12 22:47:21]

= NULL

[zel, 2011/04/13 17:49:55]

Done.

+  if (!file_browser_actions->HasKey(keys::kFileFilters) ||
+      !file_browser_actions->GetList(keys::kFileFilters, &list_value) ||
+      list_value->empty()) {
+    *error = errors::kInvalidFileFiltersList;
+    return false;
+  }
+  for (size_t i = 0; i < list_value->GetSize(); ++i) {
+    std::string filter;
+    if (!list_value->GetString(i, &filter)) {
+      *error = ExtensionErrorUtils::FormatErrorMessage(
+          errors::kInvalidFileFilterValue, base::IntToString(i));
+      return false;
+    }
+    URLPattern pattern(URLPattern::SCHEME_FILESYSTEM);
+    // TODO(skerner):  Consider enabling strict pattern parsing
+    // if this extension's location indicates that it is under development.
+    if (URLPattern::PARSE_SUCCESS != pattern.Parse(filter,
+                                                   URLPattern::PARSE_LENIENT)) {

[Aaron Boodman, 2011/04/12 22:47:21]

Use PARSE_STRICT and remove the TODO(skerner). The other one was only lenient
for legacy reasons.

[zel, 2011/04/13 17:49:55]

Done.

+      *error = ExtensionErrorUtils::FormatErrorMessage(
+          errors::kInvalidURLPatternError, filter);
+      return false;
+    }
+    result->AddPattern(pattern);
+  }
+
+  std::string default_icon;
+  // Read the page action |default_icon| (optional).

[Aaron Boodman, 2011/04/12 22:47:21]

paste-o ('page action')

[zel, 2011/04/13 17:49:55]

Done.

+  if (file_browser_actions->HasKey(keys::kPageActionDefaultIcon)) {
+    if (!file_browser_actions->GetString(keys::kPageActionDefaultIcon,
+                                     &default_icon) ||

[Aaron Boodman, 2011/04/12 22:47:21]

intent+=4

[zel, 2011/04/13 17:49:55]

Done.

+        default_icon.empty()) {
+      *error = errors::kInvalidPageActionIconPath;
+      return NULL;
+    }
+    result->set_default_icon_path(default_icon);
+  }
+
+  return result.release();
+}
+
 ExtensionSidebarDefaults* Extension::LoadExtensionSidebarDefaults(
     const DictionaryValue* extension_sidebar, std::string* error) {
   scoped_ptr<ExtensionSidebarDefaults> result(new ExtensionSidebarDefaults());
@@ -1910,6 +1998,21 @@
       return false;  // Failed to parse browser action definition.
   }
 
+  // Initialize browser action (optional).

[asargent_no_longer_on_chrome, 2011/04/12 21:09:38]

nit: comment should be "Initialize file browser actions"

[zel, 2011/04/13 17:49:55]

Done.

+  if (source.HasKey(keys::kFileBrowserActions)) {
+    ListValue* file_browser_actions_value = NULL;
+    if (!source.GetList(keys::kFileBrowserActions,
+                              &file_browser_actions_value)) {
+      *error = errors::kInvalidFileBrowserAction;
+      return false;
+    }
+
+    file_browser_actions_.reset(
+        LoadFileBrowserActions(file_browser_actions_value, error));
+    if (!file_browser_actions_.get())
+      return false;  // Failed to parse browser action definition.

[asargent_no_longer_on_chrome, 2011/04/12 21:09:38]

same naming nit on comment here

[zel, 2011/04/13 17:49:55]

Done.

+  }
+
   // Load App settings.
   if (!LoadIsApp(manifest_value_.get(), error) ||
       !LoadExtent(manifest_value_.get(), keys::kWebURLs,
@@ -1976,7 +2079,12 @@
       // Only COMPONENT extensions can use private APIs.
       // TODO(asargent) - We want a more general purpose mechanism for this,
       // and better error messages. (http://crbug.com/54013)
-      if (!IsComponentOnlyPermission(permission_str)) {
+      if (!IsComponentOnlyPermission(permission_str)
+#ifndef NDEBUG
+           && !CommandLine::ForCurrentProcess()->HasSwitch(

[Aaron Boodman, 2011/04/12 22:47:21]

Is this testing code? Can we remove it?

[zel, 2011/04/13 17:49:55]

This is super useful for us while debugging component extensions. It let us
rerun them as regular extensions without rebuilding chrome binary.

+                 switches::kExposePrivateExtensionApi)
+#endif
+          ) {
         continue;
       }
 
@@ -2573,7 +2681,12 @@
 }
 
 bool Extension::CanExecuteScriptEverywhere() const {
-  if (location() == Extension::COMPONENT)
+  if (location() == Extension::COMPONENT
+#ifndef NDEBUG
+      || CommandLine::ForCurrentProcess()->HasSwitch(

[Aaron Boodman, 2011/04/12 22:47:21]

Is this testing code? Can we remove it?

[zel, 2011/04/13 17:49:55]

see comments above

+          switches::kExposePrivateExtensionApi)
+#endif
+      )
     return true;
 
   ScriptingWhitelist* whitelist =