[Extensions] Un-refcount PermissionSet

extensions/common/permissions/permissions_data.h

Index: extensions/common/permissions/permissions_data.h
diff --git a/extensions/common/permissions/permissions_data.h b/extensions/common/permissions/permissions_data.h
index 43be869029d784ff470ae8db08d9ae3ce46fcd1c..d7d5f27f4cdc3064cf1b60ba45b968fe2a936457 100644
--- a/extensions/common/permissions/permissions_data.h
+++ b/extensions/common/permissions/permissions_data.h
@@ -9,9 +9,11 @@
 #include <string>
 #include <vector>
 
+#include "base/containers/scoped_ptr_map.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
 #include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/permissions/api_permission.h"
 #include "extensions/common/permissions/permission_message.h"
@@ -25,6 +27,14 @@ class URLPatternSet;
 
 // A container for the permissions state of an extension, including active,
 // withheld, and tab-specific permissions.
+// Thread-Safety: Since this is an object on the Extension object, *some* thread
+// safety is provided. All utility functions for checking if a permission is
+// present or an operation is allowed are thread-safe. However, permissions can
+// only be set (or updated) on the thread to which this object is bound.
+// Permissions may be accessed synchronously on that same thread.
+// Accessing on an improper thread will DCHECK().
+// This is necessary to prevent a scenario in which one thread will access
+// permissions while another thread changes them.
 class PermissionsData {
  public:
   // The possible types of access for a given frame.
@@ -35,7 +45,8 @@ class PermissionsData {
                      // the given page.
   };
 
-  using TabPermissionsMap = std::map<int, scoped_refptr<const PermissionSet>>;
+  using TabPermissionsMap =
+      base::ScopedPtrMap<int, scoped_ptr<const PermissionSet>>;
 
   // Delegate class to allow different contexts (e.g. browser vs renderer) to
   // have control over policy decisions.
@@ -80,16 +91,22 @@ class PermissionsData {
                               const Extension* extension,
                               std::string* error);
 
+  // Locks the permissions data to the current thread. We don't do this on
+  // construction, since extensions are initialized across multiple threads.
+  void BindToCurrentThread() const;
+
   // Sets the runtime permissions of the given |extension| to |active| and
   // |withheld|.
-  void SetPermissions(const scoped_refptr<const PermissionSet>& active,
-                      const scoped_refptr<const PermissionSet>& withheld) const;
+  void SetPermissions(scoped_ptr<const PermissionSet> active,
+                      scoped_ptr<const PermissionSet> withheld) const;
+
+  // Sets the active permissions, leaving withheld the same.
+  void SetActivePermissions(scoped_ptr<const PermissionSet> active) const;
 
   // Updates the tab-specific permissions of |tab_id| to include those from
   // |permissions|.
-  void UpdateTabSpecificPermissions(
-      int tab_id,
-      scoped_refptr<const PermissionSet> permissions) const;
+  void UpdateTabSpecificPermissions(int tab_id,
+                                    const PermissionSet& permissions) const;
 
   // Clears the tab-specific permissions of |tab_id|.
   void ClearTabSpecificPermissions(int tab_id) const;
@@ -182,24 +199,24 @@ class PermissionsData {
   // page itself.
   bool CanCaptureVisiblePage(int tab_id, std::string* error) const;
 
-  // Returns the tab permissions map.
-  TabPermissionsMap CopyTabSpecificPermissionsMap() const;
+  const TabPermissionsMap& tab_specific_permissions() const {
+    DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
+    return tab_specific_permissions_;
+  }
 
-  scoped_refptr<const PermissionSet> active_permissions() const {
-    // We lock so that we can't also be setting the permissions while returning.
-    base::AutoLock auto_lock(runtime_lock_);
-    return active_permissions_unsafe_;
+  const PermissionSet* active_permissions() const {
+    DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
+    return active_permissions_unsafe_.get();
   }
 
-  scoped_refptr<const PermissionSet> withheld_permissions() const {
-    // We lock so that we can't also be setting the permissions while returning.
-    base::AutoLock auto_lock(runtime_lock_);
-    return withheld_permissions_unsafe_;
+  const PermissionSet* withheld_permissions() const {
+    DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
+    return withheld_permissions_unsafe_.get();
   }
 
 #if defined(UNIT_TEST)
-  scoped_refptr<const PermissionSet> GetTabSpecificPermissionsForTesting(
-      int tab_id) const {
+  const PermissionSet* GetTabSpecificPermissionsForTesting(int tab_id) const {
+    base::AutoLock auto_lock(runtime_lock_);
     return GetTabSpecificPermissions(tab_id);
   }
 #endif
@@ -207,19 +224,21 @@ class PermissionsData {
  private:
   // Gets the tab-specific host permissions of |tab_id|, or NULL if there
   // aren't any.
-  scoped_refptr<const PermissionSet> GetTabSpecificPermissions(
-      int tab_id) const;
+  // Must be called with |runtime_lock_| acquired.
+  const PermissionSet* GetTabSpecificPermissions(int tab_id) const;
 
   // Returns true if the |extension| has tab-specific permission to operate on
   // the tab specified by |tab_id| with the given |url|.
   // Note that if this returns false, it doesn't mean the extension can't run on
   // the given tab, only that it does not have tab-specific permission to do so.
+  // Must be called with |runtime_lock_| acquired.
   bool HasTabSpecificPermissionToExecuteScript(int tab_id,
                                                const GURL& url) const;
 
   // Returns whether or not the extension is permitted to run on the given page,
   // checking against |permitted_url_patterns| in addition to blocking special
   // sites (like the webstore or chrome:// urls).
+  // Must be called with |runtime_lock_| acquired.
   AccessType CanRunOnPage(const Extension* extension,
                           const GURL& document_url,
                           int tab_id,
@@ -241,17 +260,19 @@ class PermissionsData {
   // Unsafe indicates that we must lock anytime this is directly accessed.
   // Unless you need to change |active_permissions_unsafe_|, use the (safe)
   // active_permissions() accessor.
-  mutable scoped_refptr<const PermissionSet> active_permissions_unsafe_;
+  mutable scoped_ptr<const PermissionSet> active_permissions_unsafe_;
 
   // The permissions the extension requested, but was not granted due because
   // they are too powerful. This includes things like all_hosts.
   // Unsafe indicates that we must lock anytime this is directly accessed.
   // Unless you need to change |withheld_permissions_unsafe_|, use the (safe)
   // withheld_permissions() accessor.
-  mutable scoped_refptr<const PermissionSet> withheld_permissions_unsafe_;
+  mutable scoped_ptr<const PermissionSet> withheld_permissions_unsafe_;
 
   mutable TabPermissionsMap tab_specific_permissions_;
 
+  mutable scoped_ptr<base::ThreadChecker> thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(PermissionsData);
 };