Create a ManifestPermission implementation for Automation.

chrome/common/extensions/manifest_handlers/automation.cc

 // Copyright 2014 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/common/extensions/manifest_handlers/automation.h"
 
-#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/common/extensions/api/manifest_types.h"
 #include "extensions/common/error_utils.h"
+#include "extensions/common/extensions_client.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/permissions/api_permission_set.h"
+#include "extensions/common/permissions/manifest_permission.h"
+#include "extensions/common/permissions/permission_message.h"
+#include "extensions/common/permissions/permission_message_util.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/common/url_pattern.h"
+#include "grit/generated_resources.h"
+#include "ipc/ipc_message.h"
+#include "ipc/ipc_message_utils.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace extensions {
 
 namespace automation_errors {
 const char kErrorDesktopTrueInteractFalse[] =
     "Cannot specify interactive=false if desktop=true is specified; "
     "interactive=false will be ignored.";
 const char kErrorDesktopTrueMatchesSpecified[] =
     "Cannot specify matches for Automation if desktop=true is specified; "
     "matches will be ignored.";
 const char kErrorInvalidMatch[] = "Invalid match pattern '*': *";
 const char kErrorNoMatchesProvided[] = "No valid match patterns provided.";
 }
 
 namespace errors = manifest_errors;
 namespace keys = extensions::manifest_keys;
 using api::manifest_types::Automation;
 
+class AutomationManifestPermission : public ManifestPermission {
+ public:
+  explicit AutomationManifestPermission(
+      scoped_ptr<const AutomationInfo> automation_info)
+      : automation_info_(automation_info.Pass()) {}
+
+  // extensions::ManifestPermission overrides.
+  virtual std::string name() const OVERRIDE { return keys::kAutomation; }
+
+  virtual std::string id() const OVERRIDE { return keys::kAutomation; }
+
+  virtual bool HasMessages() const OVERRIDE { return GetMessages().size() > 0; }
+
+  virtual PermissionMessages GetMessages() const OVERRIDE {

[Devlin, 2014/07/18 22:38:51]

We should probably not have these all in-lined...

[aboxhall, 2014/07/19 22:08:21]

This is the .cc file, so they're not inlined, unless I've misunderstood your
comment.

[Devlin, 2014/07/21 19:45:35]

Inlining has nothing to do with .h vs .cc file - it's when the method is
declared and defined in the same piece of code.

[aboxhall, 2014/07/21 22:44:10]

I see. But given these are virtual methods and never called directly on this
type, what's the issue with defining them here?

[Devlin, 2014/07/21 23:43:52]

More than anything else... style.  In theory, inlining methods actually doesn't
matter much, because the compiler should be smart enough to say "no".  But we
want to be consistent, and we (shouldn't) be inlining large methods like this
(though, obviously, we aren't always consistent - particularly with testing).

[aboxhall, 2014/07/22 17:00:09]

Done, although please note that this style is used very frequently throughout
the codebase, not just in tests.

[Devlin, 2014/07/22 17:36:02]

Thanks for humoring me. :)

+    PermissionMessages messages;
+    if (automation_info_->desktop) {
+      messages.push_back(PermissionMessage(
+          PermissionMessage::kFullAccess,
+          l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS)));
+    } else if (automation_info_->matches.MatchesAllURLs()) {
+      messages.push_back(PermissionMessage(
+          PermissionMessage::kHostsAll,
+          l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_ALL_HOSTS)));
+    } else {
+      URLPatternSet regular_hosts;
+      std::set<PermissionMessage> message_set;
+      ExtensionsClient::Get()->FilterHostPermissions(
+          automation_info_->matches, &regular_hosts, &message_set);
+      messages.insert(messages.end(), message_set.begin(), message_set.end());
+
+      std::set<std::string> hosts =
+          permission_message_util::GetDistinctHosts(regular_hosts, true, true);
+      if (!hosts.empty())
+        messages.push_back(permission_message_util::CreateFromHostList(hosts));
+    }
+
+    return messages;
+  }
+
+  virtual bool FromValue(const base::Value* value) OVERRIDE {
+    base::string16 error;
+    automation_info_.reset(
+        AutomationInfo::FromValue(*value, NULL /* install_warnings */, &error)
+            .release());
+    return error.empty();
+  }
+
+  virtual scoped_ptr<base::Value> ToValue() const OVERRIDE {
+    return AutomationInfo::ToValue(*automation_info_).Pass();
+  }
+
+  virtual ManifestPermission* Clone() const OVERRIDE {

[not at google - send to devlin, 2014/07/18 16:07:22]

(comment made even later)

and you could implement Clone() as "return Union(this)".

[Devlin, 2014/07/18 22:38:51]

Still gonna need Clone() for line 243, so I'm not sure I'd worry about it (since
Union() will also be slower).

[aboxhall, 2014/07/19 22:08:21]

This is moot now, since Clone() is implemented in the superclass, but there's
really not that much in it performance-wise actually: an extra cast and some
boolean operations. For a very infrequently used method, I think that's totally
fine.

+    scoped_ptr<const AutomationInfo> info(
+        AutomationInfo::Clone(*automation_info_.get()));
+    return new AutomationManifestPermission(info.Pass());
+  }
+
+  virtual ManifestPermission* Diff(
+      const ManifestPermission* rhs) const OVERRIDE {
+    const AutomationManifestPermission* other =
+        static_cast<const AutomationManifestPermission*>(rhs);
+
+    bool desktop =
+        automation_info_->desktop && !other->automation_info_->desktop;
+    bool interact =
+        automation_info_->interact && !other->automation_info_->interact;
+    URLPatternSet matches;
+    URLPatternSet::CreateDifference(
+        automation_info_->matches, other->automation_info_->matches, &matches);
+    scoped_ptr<const AutomationInfo> info(
+        new AutomationInfo(desktop, matches, interact));
+    return new AutomationManifestPermission(info.Pass());

[Devlin, 2014/07/18 22:38:51]

nit: inline |info| (here and below)

[aboxhall, 2014/07/19 22:08:21]

Done.

+  }
+
+  virtual ManifestPermission* Union(
+      const ManifestPermission* rhs) const OVERRIDE {
+    const AutomationManifestPermission* other =
+        static_cast<const AutomationManifestPermission*>(rhs);
+
+    bool desktop =
+        automation_info_->desktop || other->automation_info_->desktop;
+    bool interact =
+        automation_info_->interact || other->automation_info_->interact;
+    URLPatternSet matches;
+    URLPatternSet::CreateUnion(
+        automation_info_->matches, other->automation_info_->matches, &matches);
+    scoped_ptr<const AutomationInfo> info(
+        new AutomationInfo(desktop, matches, interact));
+    return new AutomationManifestPermission(info.Pass());
+  }
+
+  virtual ManifestPermission* Intersect(
+      const ManifestPermission* rhs) const OVERRIDE {
+    const AutomationManifestPermission* other =
+        static_cast<const AutomationManifestPermission*>(rhs);
+
+    bool desktop =
+        automation_info_->desktop && other->automation_info_->desktop;
+    bool interact =
+        automation_info_->interact && other->automation_info_->interact;
+    URLPatternSet matches;
+    URLPatternSet::CreateIntersection(
+        automation_info_->matches, other->automation_info_->matches, &matches);
+    scoped_ptr<const AutomationInfo> info(
+        new AutomationInfo(desktop, matches, interact));
+    return new AutomationManifestPermission(info.Pass());
+  }
+
+  virtual bool Contains(const ManifestPermission* rhs) const OVERRIDE {
+    const AutomationManifestPermission* other =
+        static_cast<const AutomationManifestPermission*>(rhs);
+
+    bool contains_desktop =
+        automation_info_->desktop || !other->automation_info_->desktop;
+    bool contains_interact =
+        automation_info_->interact || !other->automation_info_->interact;
+    bool contains_matches =
+        automation_info_->matches.Contains(other->automation_info_->matches);
+
+    return contains_desktop && contains_interact && contains_matches;
+  }
+
+  virtual bool Equal(const ManifestPermission* rhs) const OVERRIDE {

[not at google - send to devlin, 2014/07/18 16:07:22]

(writing this comment before below)

and you could write this as Value::Equals(ToValue(), rhs->ToValue())

[Devlin, 2014/07/18 22:38:50]

I don't think that's a great solution - it's a ton of extra work for what it
does.

If you're just worried about line count, what about
return *automation_info_ ==
       *(static_cast<const
AutomationManifestPermission*>(rhs))->automation_info_;

[aboxhall, 2014/07/19 22:08:21]

Moot: this is now in the superclass.

+    const AutomationManifestPermission* other =
+        static_cast<const AutomationManifestPermission*>(rhs);
+
+    bool same_desktop =
+        automation_info_->desktop == other->automation_info_->desktop;
+    bool same_interact =
+        automation_info_->interact == other->automation_info_->interact;
+    bool same_matches =
+        automation_info_->matches == other->automation_info_->matches;
+
+    return same_desktop && same_interact && same_matches;
+  }
+
+  virtual void Write(IPC::Message* m) const OVERRIDE {
+    base::ListValue singleton;

[Devlin, 2014/07/18 22:38:50]

not sure "singleton" is the right name for this...

[aboxhall, 2014/07/19 22:08:21]

Moot, now in superclass.

FWIW, this was intended to echo things like
http://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#singleton...,
rather than the GoF Singleton pattern.

+    base::Value* value = ToValue().release();
+    singleton.Append(value);

[Devlin, 2014/07/18 22:38:50]

nit: inline value.

[aboxhall, 2014/07/19 22:08:21]

Moot :)

+    IPC::WriteParam(m, singleton);
+  }
+
+  virtual bool Read(const IPC::Message* m, PickleIterator* iter) OVERRIDE {

[not at google - send to devlin, 2014/07/18 16:07:22]

It makes a lot of sense to implement Read/Write in terms of To/FromValue just in
general. Could you do that? Make this the default ManifestPermission Read/Write
implementation? Smaller code size (once we update the other ManifestPermission
subclasses to use it), less code added here...

[Devlin, 2014/07/18 22:38:51]

+1

[aboxhall, 2014/07/19 22:08:21]

Done.

+    base::ListValue singleton;
+    if (!IPC::ReadParam(m, iter, &singleton))
+      return false;
+    if (singleton.GetSize() != 1)
+      return false;
+    base::Value* value = NULL;
+    if (!singleton.Get(0, &value))
+      return false;
+    return FromValue(value);
+  }
+
+  virtual void Log(std::string* log) const OVERRIDE {

[not at google - send to devlin, 2014/07/18 16:07:22]

Likewise.

and it may make sense to implement Log using JSONWriter rather than IPC, since
it's a bit weird that you'd get a list for logging, and who knows what IPC
logging actually ends up doing.

(and note that I can't find ManifestPermission::Log actually being called
anywhere, but I suppose codesearch might just be behaving funny).

[aboxhall, 2014/07/19 22:08:21]

Done.

+    base::ListValue singleton;
+    scoped_ptr<base::Value> value(ToValue());
+    singleton.Append(value.get());
+    IPC::LogParam(singleton, log);
+  }
+
+ private:
+  scoped_ptr<const AutomationInfo> automation_info_;
+};
+
 AutomationHandler::AutomationHandler() {
 }
 
 AutomationHandler::~AutomationHandler() {
 }
 
 bool AutomationHandler::Parse(Extension* extension, base::string16* error) {
   const base::Value* automation = NULL;
   CHECK(extension->manifest()->Get(keys::kAutomation, &automation));
   std::vector<InstallWarning> install_warnings;
   scoped_ptr<AutomationInfo> info =
       AutomationInfo::FromValue(*automation, &install_warnings, error);
   if (!error->empty())
     return false;
 
   extension->AddInstallWarnings(install_warnings);
 
   if (!info)
     return true;
 
   extension->SetManifestData(keys::kAutomation, info.release());
   return true;
 }
 
 const std::vector<std::string> AutomationHandler::Keys() const {
   return SingleKey(keys::kAutomation);
 }
 
+ManifestPermission* AutomationHandler::CreatePermission() {
+  scoped_ptr<const AutomationInfo> info(new AutomationInfo);
+  return new AutomationManifestPermission(info.Pass());
+}
+
+ManifestPermission* AutomationHandler::CreateInitialRequiredPermission(
+    const Extension* extension) {
+  const AutomationInfo* info = AutomationInfo::Get(extension);
+  if (info)
+    return new AutomationManifestPermission(AutomationInfo::Clone(*info));
+  return NULL;
+}
+
 // static
 const AutomationInfo* AutomationInfo::Get(const Extension* extension) {
   return static_cast<AutomationInfo*>(
       extension->GetManifestData(keys::kAutomation));
 }
 
 // static
+scoped_ptr<AutomationInfo> AutomationInfo::FromValue(const base::Value& value) {
+  std::vector<InstallWarning> install_warnings;
+  base::string16 error;
+  return AutomationInfo::FromValue(value, &install_warnings, &error);
+}
+
+// static
 scoped_ptr<AutomationInfo> AutomationInfo::FromValue(
     const base::Value& value,
     std::vector<InstallWarning>* install_warnings,
     base::string16* error) {
   scoped_ptr<Automation> automation = Automation::FromValue(value, error);
   if (!automation)
     return scoped_ptr<AutomationInfo>();
 
   if (automation->as_boolean) {
     if (*automation->as_boolean)
       return make_scoped_ptr(new AutomationInfo());
     return scoped_ptr<AutomationInfo>();
   }
-  const Automation::Object& automation_object = *automation->as_object;
+  Automation::Object& automation_object = *automation->as_object;
 
   bool desktop = false;
   bool interact = false;
   if (automation_object.desktop && *automation_object.desktop) {
     desktop = true;
     interact = true;
     if (automation_object.interact && !*automation_object.interact) {
       // TODO(aboxhall): Do we want to allow this?
       install_warnings->push_back(
           InstallWarning(automation_errors::kErrorDesktopTrueInteractFalse));
     }
   } else if (automation_object.interact && *automation_object.interact) {
     interact = true;
   }
 
   URLPatternSet matches;
   bool specified_matches = false;
   if (automation_object.matches) {
     if (desktop) {
       install_warnings->push_back(
           InstallWarning(automation_errors::kErrorDesktopTrueMatchesSpecified));
     } else {
       specified_matches = true;
+
       for (std::vector<std::string>::iterator it =
                automation_object.matches->begin();
            it != automation_object.matches->end();
            ++it) {
         // TODO(aboxhall): Refactor common logic from content_scripts_handler,
         // manifest_url_handler and user_script.cc into a single location and
         // re-use here.
         URLPattern pattern(URLPattern::SCHEME_ALL &
                            ~URLPattern::SCHEME_CHROMEUI);
         URLPattern::ParseResult parse_result = pattern.Parse(*it);
+
         if (parse_result != URLPattern::PARSE_SUCCESS) {
           install_warnings->push_back(
               InstallWarning(ErrorUtils::FormatErrorMessage(
                   automation_errors::kErrorInvalidMatch,
                   *it,
                   URLPattern::GetParseResultString(parse_result))));
           continue;
         }
 
         matches.AddPattern(pattern);
       }
     }
   }
-  if (specified_matches && matches.is_empty())
+  if (specified_matches && matches.is_empty()) {
     install_warnings->push_back(
         InstallWarning(automation_errors::kErrorNoMatchesProvided));
+  }
 
-  return make_scoped_ptr(
-      new AutomationInfo(desktop, matches, interact, specified_matches));
+  return make_scoped_ptr(new AutomationInfo(desktop, matches, interact));
 }
 
-AutomationInfo::AutomationInfo()
-    : desktop(false), interact(false), specified_matches(false) {
+// static
+scoped_ptr<base::Value> AutomationInfo::ToValue(const AutomationInfo& info) {
+  return AsManifestType(info)->ToValue().Pass();
 }
+
+// static
+scoped_ptr<Automation> AutomationInfo::AsManifestType(
+    const AutomationInfo& info) {
+  Automation* automation = new Automation;

[Devlin, 2014/07/18 22:38:50]

Why not make this a scoped_ptr from the start?  It's being returned as one
everywhere...

[aboxhall, 2014/07/19 22:08:21]

Good point, done.

+  if (!info.desktop && !info.interact && info.matches.size() == 0) {
+    automation->as_boolean.reset(new bool(true));
+    return make_scoped_ptr(automation).Pass();
+  }
+
+  Automation::Object* as_object = new Automation::Object;
+  as_object->desktop.reset(new bool(info.desktop));
+  as_object->interact.reset(new bool(info.interact));
+  if (info.matches.size() > 0) {
+    as_object->matches.reset(info.matches.ToStringVector().release());
+  }
+  automation->as_object.reset(as_object);
+  return make_scoped_ptr(automation).Pass();
+}
+
+// static
+scoped_ptr<const AutomationInfo> AutomationInfo::Clone(
+    const AutomationInfo& info) {
+  const AutomationInfo* result =
+      new AutomationInfo(info.desktop, info.matches, info.interact);
+  return make_scoped_ptr(result);
+}
+
+AutomationInfo::AutomationInfo() : desktop(false), interact(false) {
+}
+
 AutomationInfo::AutomationInfo(bool desktop,
-                               const URLPatternSet& matches,
-                               bool interact,
-                               bool specified_matches)
-    : desktop(desktop),
-      matches(matches),
-      interact(interact),
-      specified_matches(specified_matches) {
+                               const URLPatternSet matches,
+                               bool interact)
+    : desktop(desktop), matches(matches), interact(interact) {
 }
 
 AutomationInfo::~AutomationInfo() {
 }
 
 }  // namespace extensions