[Extensions] Un-refcount PermissionSet

extensions/common/permissions/permission_set.h

 // Copyright 2013 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 EXTENSIONS_COMMON_PERMISSIONS_PERMISSION_SET_H_
 #define EXTENSIONS_COMMON_PERMISSIONS_PERMISSION_SET_H_
 
 #include <set>
 #include <string>
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "extensions/common/permissions/api_permission.h"
 #include "extensions/common/permissions/api_permission_set.h"
 #include "extensions/common/permissions/manifest_permission.h"
 #include "extensions/common/permissions/manifest_permission_set.h"
 #include "extensions/common/url_pattern_set.h"
 
 namespace extensions {
 
 // The PermissionSet is an immutable class that encapsulates an
 // extension's permissions. The class exposes set operations for combining and
 // manipulating the permissions.
 // TODO(sashab): PermissionIDSet should be called PermissionSet. Once
 // PermissionMessageProvider::GetCoalescedPermissionMessages() is the only
 // method used for generating permission messages, find the other users of this
 // class and deprecate or rename it as appropriate.
-class PermissionSet
-    : public base::RefCountedThreadSafe<PermissionSet> {
+class PermissionSet {
  public:
   // Creates an empty permission set (e.g. default permissions).
   PermissionSet();
 
   // Creates a new permission set based on the specified data: the API
   // permissions, manifest key permissions, host permissions, and scriptable
   // hosts. The effective hosts of the newly created permission set will be
   // inferred from the given host permissions.
   PermissionSet(const APIPermissionSet& apis,
                 const ManifestPermissionSet& manifest_permissions,
                 const URLPatternSet& explicit_hosts,
                 const URLPatternSet& scriptable_hosts);
+  ~PermissionSet();
 
   // Creates a new permission set equal to |set1| - |set2|.
-  static scoped_refptr<const PermissionSet> CreateDifference(
+  static scoped_ptr<const PermissionSet> CreateDifference(
       const PermissionSet& set1,
       const PermissionSet& set2);
 
   // Creates a new permission set equal to the intersection of |set1| and
   // |set2|.
-  static scoped_refptr<const PermissionSet> CreateIntersection(
+  static scoped_ptr<const PermissionSet> CreateIntersection(
       const PermissionSet& set1,
       const PermissionSet& set2);
 
   // Creates a new permission set equal to the union of |set1| and |set2|.
-  static scoped_refptr<const PermissionSet> CreateUnion(
-      const PermissionSet& set1,
-      const PermissionSet& set2);
+  static scoped_ptr<const PermissionSet> CreateUnion(const PermissionSet& set1,
+                                                     const PermissionSet& set2);
 
   bool operator==(const PermissionSet& rhs) const;
   bool operator!=(const PermissionSet& rhs) const;
 
+  // Returns a copy of this PermissionSet.
+  scoped_ptr<const PermissionSet> Clone() const;

[not at google - send to devlin, 2015/09/22 21:51:26]

It's a bit odd to have a Clone() method but no DISALLOW_COPY_AND_ASSIGN. With
respect to other comment in Clone(), I'd make this DISALLOW_ASSIGN and then add
a private copy constructor.

(or just make it copy and assignable and do away with the Clone(), but I do like
the explicit copy)

[Devlin, 2015/09/23 17:08:59]

Done.

+
   // Returns true if every API or host permission available to |set| is also
   // available to this. In other words, if the API permissions of |set| are a
   // subset of this, and the host permissions in this encompass those in |set|.
   bool Contains(const PermissionSet& set) const;
 
   // Gets the API permissions in this set as a set of strings.
   std::set<std::string> GetAPIsAsStrings() const;
 
   // Returns true if this is an empty set (e.g., the default permission set).
   bool IsEmpty() const;
@@ skipping 44 matching lines @@
 
   const URLPatternSet& effective_hosts() const { return effective_hosts_; }
 
   const URLPatternSet& explicit_hosts() const { return explicit_hosts_; }
 
   const URLPatternSet& scriptable_hosts() const { return scriptable_hosts_; }
 
  private:
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest, GetWarningMessages_AudioVideo);
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest, AccessToDevicesMessages);
-  friend class base::RefCountedThreadSafe<PermissionSet>;
-
-  ~PermissionSet();
 
   // Adds permissions implied independently of other context.
   void InitImplicitPermissions();
 
   // Initializes the effective host permission based on the data in this set.
   void InitEffectiveHosts();
 
   // Initializes |has_access_to_most_hosts_|.
   void InitShouldWarnAllHosts() const;
 
@@ skipping 14 matching lines @@
   URLPatternSet scriptable_hosts_;
 
   // The list of hosts this effectively grants access to.
   URLPatternSet effective_hosts_;
 
   enum ShouldWarnAllHostsType {
     UNINITIALIZED = 0,
     WARN_ALL_HOSTS,
     DONT_WARN_ALL_HOSTS
   };
   // Whether or not this permission set includes access to so many origins, we

[not at google - send to devlin, 2015/09/22 21:51:26]

Unrelated: this comment is wrong. It should just be "Cache whether this set
implies access to all hosts, because it's non-trivial to compute."

[Devlin, 2015/09/23 17:08:59]

Done.

   // should treat it as all_hosts for warning purposes.
   // Lazily initialized (and therefore mutable).
   mutable ShouldWarnAllHostsType should_warn_all_hosts_;
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_COMMON_PERMISSIONS_PERMISSION_SET_H_