Prepare declarativeContent API for new script injection feature. Added Javascript types and functio…

chrome/browser/extensions/api/declarative_content/content_action.cc

Index: chrome/browser/extensions/api/declarative_content/content_action.cc
diff --git a/chrome/browser/extensions/api/declarative_content/content_action.cc b/chrome/browser/extensions/api/declarative_content/content_action.cc
index 751f8d53383c946c633b1ef23066a414675e8554..175ab12b75a339022d603924c48f55deae282f63 100644
--- a/chrome/browser/extensions/api/declarative_content/content_action.cc
+++ b/chrome/browser/extensions/api/declarative_content/content_action.cc
@@ -29,6 +29,9 @@ const char kInvalidInstanceTypeError[] =
     "An action has an invalid instanceType: %s";
 const char kNoPageAction[] =
     "Can't use declarativeContent.ShowPageAction without a page action";
+const char kMissingParameter[] = "Missing parameter is required: %s";
+const char kInvalidTypeOfParameter[] = "Attribute '%s' has an invalid type. "
+                                       "Expected type: %s";
 
 #define INPUT_FORMAT_VALIDATE(test) do { \
     if (!(test)) { \
@@ -94,6 +97,132 @@ class ShowPageAction : public ContentAction {
   DISALLOW_COPY_AND_ASSIGN(ShowPageAction);
 };
 
+// Action that injects a content script.
+class RequestContentScript : public ContentAction {
+ public:
+  RequestContentScript(const std::vector<std::string>& css_file_names,
+                       const std::vector<std::string>& js_file_names,
+                       const bool& all_frames,

[Jeffrey Yasskin, 2014/07/15 20:44:59]

You can pass bools by value.

[Mark Dittmer, 2014/07/16 12:26:13]

Done.

[not at google - send to devlin, 2014/07/16 16:51:14]

there's no point having them as "const" when they're passed by value.

[Mark Dittmer, 2014/07/16 19:40:45]

Done.

+                       const bool& match_about_blank)
+      : css_file_names_(css_file_names),
+        js_file_names_(js_file_names),
+        all_frames_(all_frames),
+        match_about_blank_(match_about_blank) {}
+
+  static scoped_refptr<ContentAction> Create(const Extension* extension,
+                                             const base::DictionaryValue* dict,
+                                             std::string* error,
+                                             bool* bad_message);
+  // friend scoped_refptr<ContentAction> RequestContentScript::Create(

[Jeffrey Yasskin, 2014/07/15 20:44:59]

Remember to delete this before submitting.

[Mark Dittmer, 2014/07/16 12:26:13]

Done. Thanks!

+  //     const Extension* extension,
+  //     const base::DictionaryValue* dict,
+  //     std::string* error,
+  //     bool* bad_message);
+
+  // Implementation of ContentAction:
+  virtual Type GetType() const OVERRIDE {
+    return ACTION_REQUEST_CONTENT_SCRIPT;
+  }
+
+  virtual void Apply(const std::string& extension_id,
+                     const base::Time& extension_install_time,
+                     ApplyInfo* apply_info) const OVERRIDE {
+    // TODO(markdittmer): Invoke UserScriptMaster declarative script loader:
+    // load new user script.
+  }
+
+  virtual void Revert(const std::string& extension_id,
+                      const base::Time& extension_install_time,
+                      ApplyInfo* apply_info) const OVERRIDE {
+    // TODO(markdittmer): Invoke UserScriptMaster declarative script loader:
+    // do not load user script if Apply() runs again on the same page.
+  }
+
+ private:
+  std::vector<std::string> css_file_names_;
+  std::vector<std::string> js_file_names_;
+  bool all_frames_;
+  bool match_about_blank_;
+
+  virtual ~RequestContentScript() {}

[Jeffrey Yasskin, 2014/07/15 20:44:59]

The destructor would go first in this private section
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Declar...),
but you can probably just remove it.

[Mark Dittmer, 2014/07/16 12:26:13]

Moved it to the beginning of the section. Without it, I get "error:
[chromium-style] Classes that are ref-counted should have explicit destructors
that are declared protected or private."

+
+  DISALLOW_COPY_AND_ASSIGN(RequestContentScript);
+};
+
+// Helper for getting JS collections into C++.
+static bool AppendJSStringsToCPPStrings(const base::ListValue& append_strings,
+                                        std::vector<std::string>* append_to) {
+  for (base::ListValue::const_iterator it = append_strings.begin();
+       it != append_strings.end(); ++it) {
+    std::string value;
+    if ((*it)->GetAsString(&value)) {
+      append_to->push_back(value);
+    } else {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+// static
+scoped_refptr<ContentAction> RequestContentScript::Create(
+    const Extension* extension,
+    const base::DictionaryValue* dict,
+    std::string* error,
+    bool* bad_message) {
+  std::vector<std::string> css_file_names;
+  std::vector<std::string> js_file_names;
+  bool all_frames(false);

[Jeffrey Yasskin, 2014/07/15 20:44:59]

We usually initialize simple values with = instead of ()s.

[Mark Dittmer, 2014/07/16 12:26:13]

Done.

+  bool match_about_blank(false);
+  const base::ListValue* list_value;
+
+  if (!dict->HasKey(keys::kCss) && !dict->HasKey(keys::kJs)) {
+    *error = base::StringPrintf(kMissingParameter, "css or js");
+    return scoped_refptr<ContentAction>();
+  }
+
+  if (dict->HasKey(keys::kCss) && dict->GetList(keys::kCss, &list_value)) {
+    AppendJSStringsToCPPStrings(*list_value, &css_file_names);

[Jeffrey Yasskin, 2014/07/15 20:44:59]

You should use the return value of this function, in order to catch invalid list
element types.

[Mark Dittmer, 2014/07/16 12:26:13]

Done.

+  } else {
+    *error = base::StringPrintf(kInvalidTypeOfParameter,

[Jeffrey Yasskin, 2014/07/15 20:44:59]

This looks like it's going to give an error if someone has "js" but not "css",
which doesn't seem correct. Please add some tests to content_action_unittest.cc
to check this function.

[Mark Dittmer, 2014/07/16 12:26:13]

Done.

+                                keys::kCss,
+                                "Array of strings");
+    return scoped_refptr<ContentAction>();
+  }
+
+  if (dict->HasKey(keys::kJs) && dict->GetList(keys::kJs, &list_value)) {
+    AppendJSStringsToCPPStrings(*list_value, &js_file_names);
+  } else {
+    *error = base::StringPrintf(kInvalidTypeOfParameter,
+                                keys::kJs,
+                                "Array of strings");
+    return scoped_refptr<ContentAction>();
+  }
+
+  if (dict->HasKey(keys::kAllFrames) &&
+      !dict->GetBoolean(keys::kAllFrames, &all_frames)) {
+    *error = base::StringPrintf(kInvalidTypeOfParameter,
+                                keys::kAllFrames,
+                                "Boolean");
+    return scoped_refptr<ContentAction>();
+  }
+
+  if (dict->HasKey(keys::kMatchAboutBlank) &&
+      !dict->GetBoolean(keys::kMatchAboutBlank, &match_about_blank)) {
+    *error = base::StringPrintf(kInvalidTypeOfParameter,
+                                keys::kMatchAboutBlank,
+                                "Boolean");
+    return scoped_refptr<ContentAction>();
+  }
+
+  return scoped_refptr<ContentAction>(new RequestContentScript(
+      css_file_names,
+      js_file_names,
+      all_frames,
+      match_about_blank));
+}
+
 struct ContentActionFactory {
   // Factory methods for ContentAction instances. |extension| is the extension
   // for which the action is being created. |dict| contains the json dictionary
@@ -113,6 +242,8 @@ struct ContentActionFactory {
   ContentActionFactory() {
     factory_methods[keys::kShowPageAction] =
         &ShowPageAction::Create;
+    factory_methods[keys::kRequestContentScript] =
+        &RequestContentScript::Create;
   }
 };