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

chrome/browser/extensions/api/declarative_content/content_action.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/declarative_content/content_action.h"
 
 #include <map>
 
 #include "base/lazy_instance.h"
 #include "base/strings/stringprintf.h"
@@ skipping 11 matching lines @@
 namespace extensions {
 
 namespace keys = declarative_content_constants;
 
 namespace {
 // Error messages.
 const char kInvalidInstanceTypeError[] =
     "An action has an invalid instanceType: %s";
 const char kNoPageAction[] =
     "Can't use declarativeContent.ShowPageAction without a page action";
+const char kInvalidTypeOfParamter[] = "Attribute '%s' has an invalid type";

[Jeffrey Yasskin, 2014/07/15 17:12:59]

sp: "Paramter"

It's also more user-friendly to say what the type needs to be. This may mean you
need more strings.

[Mark Dittmer, 2014/07/15 18:32:42]

Done.

 
 #define INPUT_FORMAT_VALIDATE(test) do { \
     if (!(test)) { \
       *bad_message = true; \
       return scoped_refptr<ContentAction>(NULL); \
     } \
   } while (0)
 
 //
 // The following are concrete actions.
@@ skipping 45 matching lines @@
             ->GetExtensionById(extension_id, ExtensionRegistry::EVERYTHING);
     if (!extension)
       return NULL;
     return ExtensionActionManager::Get(profile)->GetPageAction(*extension);
   }
   virtual ~ShowPageAction() {}
 
   DISALLOW_COPY_AND_ASSIGN(ShowPageAction);
 };
 
+// Action that injects a content script.
+class RequestContentScript : public ContentAction {
+ public:
+  RequestContentScript() {}

[Jeffrey Yasskin, 2014/07/15 17:12:59]

Give this constructor parameters matching the values that need to be stored,
instead of side-loading them after constructing the object.

[Mark Dittmer, 2014/07/15 18:32:43]

Done.

+
+  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 17:12:59]

You don't need to friend members of the same class.

[Mark Dittmer, 2014/07/15 18:32:43]

I hit a compiler error without this. I think the friendship is needed because
Create is static.

+      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 in the future.

[Jeffrey Yasskin, 2014/07/15 17:13:00]

s/in the future/if Apply() runs again on the same page/

[Mark Dittmer, 2014/07/15 18:32:43]

Done.

+  }
+
+ private:
+  virtual ~RequestContentScript() {}
+  DISALLOW_COPY_AND_ASSIGN(RequestContentScript);

[Jeffrey Yasskin, 2014/07/15 17:13:00]

This goes last in the class:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Declar...

[Mark Dittmer, 2014/07/15 18:32:42]

Done.

+
+  std::vector<std::string> css_file_names_;
+  std::vector<std::string> js_file_names_;
+  bool all_frames_;
+  bool match_about_blank_;
+};
+
+// Helper for getting JS collections into C++.
+bool AppendJSStringsToCPPStrings(const base::ListValue& append_strings,

[Jeffrey Yasskin, 2014/07/15 17:12:59]

This should be static to be sure it doesn't collide with a helper in another .cc
file.

[Mark Dittmer, 2014/07/15 18:32:42]

Done. I'm curious, doesn't living in an anonymous namespace (which it does) also
achieve this?

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

Ah, yes it does. I'd lost the anonymous namespace around nearly everything in
the file.

+                                 std::vector<std::string>& append_to) {

[Jeffrey Yasskin, 2014/07/15 17:12:59]

Chrome doesn't use non-const reference arguments:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Refere...

[Mark Dittmer, 2014/07/15 18:32:42]

Done.

+  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) {
+  RequestContentScript* request_content_script = new RequestContentScript();
+
+  for (base::DictionaryValue::Iterator iter(*dict);
+       !iter.IsAtEnd(); iter.Advance()) {
+    const std::string& attribute_name = iter.key();
+    const base::Value& attribute_value = iter.value();
+    if (attribute_name == "css" || attribute_name == "js") {

[Jeffrey Yasskin, 2014/07/15 17:12:59]

I would probably look up the attributes I want instead of iterating all
attributes, but this isn't terrible.

[Mark Dittmer, 2014/07/15 18:32:42]

Done. Switched to looking up individual members. This didn't account for
instanceType anyway, and if we add more metadata in the future, we shouldn't
have to update a "skip-property list" in this loop.

+      std::vector<std::string>& string_list = (attribute_name == "css") ?
+          request_content_script->css_file_names_ :
+          request_content_script->js_file_names_;
+      const base::ListValue* list_value = NULL;
+      if (attribute_value.GetAsList(&list_value)) {
+        AppendJSStringsToCPPStrings(*list_value, string_list);
+      } else {
+        *error = base::StringPrintf(kInvalidTypeOfParamter,
+                                    attribute_name.c_str());
+        break;
+      }
+    } else if (attribute_name == "allFrames" ||
+               attribute_name == "matchAboutBlank") {
+      bool& bool_ref = (attribute_name == "allFrames") ?
+          request_content_script->all_frames_ :
+          request_content_script->match_about_blank_;
+      bool bool_read;
+      if (attribute_value.GetAsBoolean(&bool_read)) {
+        bool_ref = bool_read;
+      } else {
+        *error = base::StringPrintf(kInvalidTypeOfParamter,
+                                    attribute_name.c_str());
+        break;
+      }
+    }
+  }
+
+  if (error->empty()) {
+    delete request_content_script;

[Jeffrey Yasskin, 2014/07/15 17:12:59]

Try never to have to write "delete foo" at the end of a function: it makes the
function more fragile. Instead, capture the pointer into a scoped_refptr or
scoped_ptr as soon as you create it.

[Mark Dittmer, 2014/07/15 18:32:42]

Done.

+    return scoped_refptr<ContentAction>();
+  } else {
+    return scoped_refptr<ContentAction>(request_content_script);
+  }
+}
+
 struct ContentActionFactory {
   // Factory methods for ContentAction instances. |extension| is the extension
   // for which the action is being created. |dict| contains the json dictionary
   // that describes the action. |error| is used to return error messages in case
   // the extension passed an action that was syntactically correct but
   // semantically incorrect. |bad_message| is set to true in case |dict| does
   // not confirm to the validated JSON specification.
   typedef scoped_refptr<ContentAction>(*FactoryMethod)(
       const Extension* /* extension */,
       const base::DictionaryValue* /* dict */,
       std::string* /* error */,
       bool* /* bad_message */);
   // Maps the name of a declarativeContent action type to the factory
   // function creating it.
   std::map<std::string, FactoryMethod> factory_methods;
 
   ContentActionFactory() {
     factory_methods[keys::kShowPageAction] =
         &ShowPageAction::Create;
+    factory_methods[keys::kRequestContentScript] =
+        &RequestContentScript::Create;
   }
 };
 
 base::LazyInstance<ContentActionFactory>::Leaky
     g_content_action_factory = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 //
 // ContentAction
@@ skipping 24 matching lines @@
       factory_method_iter = factory.factory_methods.find(instance_type);
   if (factory_method_iter != factory.factory_methods.end())
     return (*factory_method_iter->second)(
         extension, action_dict, error, bad_message);
 
   *error = base::StringPrintf(kInvalidInstanceTypeError, instance_type.c_str());
   return scoped_refptr<ContentAction>();
 }
 
 }  // namespace extensions