Implement ComplexFeature to support permission features with multiple rules.

chrome/common/extensions/features/feature.h

 // 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.
 
 #ifndef CHROME_COMMON_EXTENSIONS_FEATURES_FEATURE_H_
 #define CHROME_COMMON_EXTENSIONS_FEATURES_FEATURE_H_
 
 #include <set>
 #include <string>
 
@@ skipping 65 matching lines @@
     friend class Feature;
 
     // Instances should be created via Feature::CreateAvailability.
     Availability(AvailabilityResult result, const std::string& message)
         : result_(result), message_(message) { }
 
     const AvailabilityResult result_;
     const std::string message_;
   };
 
-  Feature();
-  Feature(const Feature& other);
-  virtual ~Feature();
+  Feature() {}

[not at google - send to devlin, 2012/12/14 19:10:42]

If the constructor is just this then it can be left out

[justinlin, 2012/12/14 21:07:52]

Done.

+  virtual ~Feature() {}

[not at google - send to devlin, 2012/12/14 19:10:42]

IIRC virtual constructors in non-pure-virtual classes shouldn't be inlined, and
there are those CreateAvailability methods. If you can make those pure-virtual
(or move them someplace else) then that'd be cool, but nbd if not. I just don't
think this can be inline-virtual.

[justinlin, 2012/12/14 21:07:52]

Done. Right, put it in the cc for now.
I moved the CreateAvailability functions to SimpleFeature, maybe that makes more
sense since they're more like helpers.

The get/set_name are in this class, so it won't be pure virtual anyway.

 
   // Gets the current channel as seen by the Feature system.
   static chrome::VersionInfo::Channel GetCurrentChannel();
 
   // Sets the current channel as seen by the Feature system. In the browser
   // process this should be chrome::VersionInfo::GetChannel(), and in the
   // renderer this will need to come from an IPC.
   static void SetCurrentChannel(chrome::VersionInfo::Channel channel);
 
   // Gets the default channel as seen by the Feature system.
@@ skipping 18 matching lines @@
 
   const std::string& name() const { return name_; }
   void set_name(const std::string& name) { name_ = name; }
 
   // Gets the platform the code is currently running on.
   static Platform GetCurrentPlatform();
 
   // Gets the Feature::Location value for the specified Extension::Location.
   static Location ConvertLocation(Extension::Location extension_location);
 
-  std::set<std::string>* whitelist() { return &whitelist_; }
-  std::set<Extension::Type>* extension_types() { return &extension_types_; }
-  std::set<Context>* contexts() { return &contexts_; }
-
-  Location location() const { return location_; }
-  void set_location(Location location) { location_ = location; }
-
-  Platform platform() const { return platform_; }
-  void set_platform(Platform platform) { platform_ = platform; }
-
-  int min_manifest_version() const { return min_manifest_version_; }
-  void set_min_manifest_version(int min_manifest_version) {
-    min_manifest_version_ = min_manifest_version;
-  }
-
-  int max_manifest_version() const { return max_manifest_version_; }
-  void set_max_manifest_version(int max_manifest_version) {
-    max_manifest_version_ = max_manifest_version;
-  }
-
-  // Parses the JSON representation of a feature into the fields of this object.
-  // Unspecified values in the JSON are not modified in the object. This allows
-  // us to implement inheritance by parsing one value after another.
-  void Parse(const DictionaryValue* value);
-
-  // Returns true if the feature contains the same values as another.
-  bool Equals(const Feature& other) const;
+  virtual std::set<Context>* contexts() = 0;

[not at google - send to devlin, 2012/12/14 19:10:42]

Generally speaking you can't guarantee that virtual methods have trivial
implementations, which is what calling this contexts() rather than GetContexts()
implies. One reasonable exception is for when things are virtual for testing.

But stepping back: why does this need to be on the interface for Feature?

[justinlin, 2012/12/14 21:07:52]

extension_api.cc needs this method in a couple places (3) where it doesn't look
like it's safe to case to SimpleFeature. Renamed. I think this maybe can be
removed eventually by somebody who fully understands the logic in extension_api.
I didn't want to change it and risk breaking something yet.

 
   // Returns true if the feature is available to be parsed into a new extension
   // manifest.
   Availability IsAvailableToManifest(const std::string& extension_id,
                                      Extension::Type type,
                                      Location location,
                                      int manifest_version) const {
     return IsAvailableToManifest(extension_id, type, location, manifest_version,
                                  GetCurrentPlatform());
   }
-  Availability IsAvailableToManifest(const std::string& extension_id,
-                                     Extension::Type type,
-                                     Location location,
-                                     int manifest_version,
-                                     Platform platform) const;
+  virtual Availability IsAvailableToManifest(const std::string& extension_id,
+                                             Extension::Type type,
+                                             Location location,
+                                             int manifest_version,
+                                             Platform platform) const = 0;
 
   // Returns true if the feature is available to be used in the specified
   // extension and context.
   Availability IsAvailableToContext(const Extension* extension,
                                     Context context) const {
     return IsAvailableToContext(extension, context, GetCurrentPlatform());
   }
   virtual Availability IsAvailableToContext(const Extension* extension,
                                             Context context,
-                                            Platform platform) const;
+                                            Platform platform) const = 0;
 
  protected:
   Availability CreateAvailability(AvailabilityResult result) const;
   Availability CreateAvailability(AvailabilityResult result,
                                   Extension::Type type) const;
 
- private:
-  std::string GetAvailabilityMessage(
-      AvailabilityResult result, Extension::Type type) const;
+  virtual std::string GetAvailabilityMessage(
+      AvailabilityResult result, Extension::Type type) const = 0;
 
   std::string name_;
-
-  // For clarify and consistency, we handle the default value of each of these
-  // members the same way: it matches everything. It is up to the higher level
-  // code that reads Features out of static data to validate that data and set
-  // sensible defaults.
-  std::set<std::string> whitelist_;
-  std::set<Extension::Type> extension_types_;
-  std::set<Context> contexts_;
-  Location location_;  // we only care about component/not-component now
-  Platform platform_;  // we only care about chromeos/not-chromeos now
-  int min_manifest_version_;
-  int max_manifest_version_;
-  chrome::VersionInfo::Channel channel_;
 };
 
 }  // namespace extensions
 
 #endif  // CHROME_COMMON_EXTENSIONS_FEATURES_FEATURE_H_