[Extensions] Un-refcount PermissionSet

PermissionSet represents a set of permissions, and for some reason,
it's been refcounted. There's really no reason to have it, and it
makes everything more costly and difficult to reason about. Remove the
refcounting.

Note: This is part 1 of a 2-part series. This removes the ref-counting.
In a followup, I'll go through and update many of the places that
use const PermissionSet* and convert to const &.

BUG=455414
TBR=thestig@chromium.org (misc chrome files with ptr conversion)
TBR=nasko@chromium.org (extension messages - not actually changing any IPC messages)

Review URL: https://codereview.chromium.org/1349613003

Cr-Commit-Position: refs/heads/master@{#350684}

4 files changed, 174 insertions, 122 deletions

diff --git a/extensions/common/permissions/permission_set.cc b/extensions/common/permissions/permission_set.cc
index 5cd8d05..a3e3f41 100644
--- a/extensions/common/permissions/permission_set.cc
+++ b/extensions/common/permissions/permission_set.cc
@@ -43,8 +43,10 @@ PermissionSet::PermissionSet(
   InitEffectiveHosts();
 }
 
+PermissionSet::~PermissionSet() {}
+
 // static
-scoped_refptr<const PermissionSet> PermissionSet::CreateDifference(
+scoped_ptr<const PermissionSet> PermissionSet::CreateDifference(
     const PermissionSet& set1,
     const PermissionSet& set2) {
   APIPermissionSet apis;
@@ -61,12 +63,12 @@ scoped_refptr<const PermissionSet> PermissionSet::CreateDifference(
   URLPatternSet scriptable_hosts = URLPatternSet::CreateDifference(
       set1.scriptable_hosts(), set2.scriptable_hosts());
 
-  return make_scoped_refptr(new PermissionSet(
-      apis, manifest_permissions, explicit_hosts, scriptable_hosts));
+  return make_scoped_ptr(new PermissionSet(apis, manifest_permissions,
+                                           explicit_hosts, scriptable_hosts));
 }
 
 // static
-scoped_refptr<const PermissionSet> PermissionSet::CreateIntersection(
+scoped_ptr<const PermissionSet> PermissionSet::CreateIntersection(
     const PermissionSet& set1,
     const PermissionSet& set2) {
   APIPermissionSet apis;
@@ -82,12 +84,12 @@ scoped_refptr<const PermissionSet> PermissionSet::CreateIntersection(
   URLPatternSet scriptable_hosts = URLPatternSet::CreateSemanticIntersection(
       set1.scriptable_hosts(), set2.scriptable_hosts());
 
-  return new PermissionSet(apis, manifest_permissions,
-                           explicit_hosts, scriptable_hosts);
+  return make_scoped_ptr(new PermissionSet(apis, manifest_permissions,
+                                           explicit_hosts, scriptable_hosts));
 }
 
 // static
-scoped_refptr<const PermissionSet> PermissionSet::CreateUnion(
+scoped_ptr<const PermissionSet> PermissionSet::CreateUnion(
     const PermissionSet& set1,
     const PermissionSet& set2) {
   APIPermissionSet apis;
@@ -104,8 +106,8 @@ scoped_refptr<const PermissionSet> PermissionSet::CreateUnion(
   URLPatternSet scriptable_hosts = URLPatternSet::CreateUnion(
       set1.scriptable_hosts(), set2.scriptable_hosts());
 
-  return new PermissionSet(apis, manifest_permissions,
-                           explicit_hosts, scriptable_hosts);
+  return make_scoped_ptr(new PermissionSet(apis, manifest_permissions,
+                                           explicit_hosts, scriptable_hosts));
 }
 
 bool PermissionSet::operator==(
@@ -120,6 +122,10 @@ bool PermissionSet::operator!=(const PermissionSet& rhs) const {
   return !(*this == rhs);
 }
 
+scoped_ptr<const PermissionSet> PermissionSet::Clone() const {
+  return make_scoped_ptr(new PermissionSet(*this));
+}
+
 bool PermissionSet::Contains(const PermissionSet& set) const {
   return apis_.Contains(set.apis()) &&
          manifest_permissions_.Contains(set.manifest_permissions()) &&
@@ -218,7 +224,13 @@ bool PermissionSet::HasEffectiveFullAccess() const {
   return false;
 }
 
-PermissionSet::~PermissionSet() {}
+PermissionSet::PermissionSet(const PermissionSet& permissions)
+    : apis_(permissions.apis_),
+      manifest_permissions_(permissions.manifest_permissions_),
+      explicit_hosts_(permissions.explicit_hosts_),
+      scriptable_hosts_(permissions.scriptable_hosts_),
+      effective_hosts_(permissions.effective_hosts_),
+      should_warn_all_hosts_(permissions.should_warn_all_hosts_) {}
 
 void PermissionSet::InitImplicitPermissions() {
   // The downloads permission implies the internal version as well.
diff --git a/extensions/common/permissions/permission_set.h b/extensions/common/permissions/permission_set.h
index a6c62be..4f02dbd 100644
--- a/extensions/common/permissions/permission_set.h
+++ b/extensions/common/permissions/permission_set.h
@@ -25,8 +25,7 @@ namespace extensions {
 // 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();
@@ -39,26 +38,29 @@ class PermissionSet
                 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;
+
   // 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|.
@@ -123,9 +125,9 @@ class PermissionSet
  private:
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest, GetWarningMessages_AudioVideo);
   FRIEND_TEST_ALL_PREFIXES(PermissionsTest, AccessToDevicesMessages);
-  friend class base::RefCountedThreadSafe<PermissionSet>;
 
-  ~PermissionSet();
+  // Deliberate copy constructor for cloning the set.
+  PermissionSet(const PermissionSet& permission_set);
 
   // Adds permissions implied independently of other context.
   void InitImplicitPermissions();
@@ -160,10 +162,11 @@ class PermissionSet
     WARN_ALL_HOSTS,
     DONT_WARN_ALL_HOSTS
   };
-  // Whether or not this permission set includes access to so many origins, we
-  // should treat it as all_hosts for warning purposes.
-  // Lazily initialized (and therefore mutable).
+  // Cache whether this set implies access to all hosts, because it's
+  // non-trivial to compute (lazily initialized).
   mutable ShouldWarnAllHostsType should_warn_all_hosts_;
+
+  DISALLOW_ASSIGN(PermissionSet);
 };
 
 }  // namespace extensions
diff --git a/extensions/common/permissions/permissions_data.cc b/extensions/common/permissions/permissions_data.cc
index 7fdbd2c..6757298 100644
--- a/extensions/common/permissions/permissions_data.cc
+++ b/extensions/common/permissions/permissions_data.cc
@@ -25,19 +25,31 @@ namespace {
 
 PermissionsData::PolicyDelegate* g_policy_delegate = NULL;
 
+class AutoLockOnValidThread {
+ public:
+  AutoLockOnValidThread(base::Lock& lock, base::ThreadChecker* thread_checker)
+      : auto_lock_(lock) {
+    DCHECK(!thread_checker || thread_checker->CalledOnValidThread());
+  }
+
+ private:
+  base::AutoLock auto_lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(AutoLockOnValidThread);
+};
+
 }  // namespace
 
 PermissionsData::PermissionsData(const Extension* extension)
     : extension_id_(extension->id()), manifest_type_(extension->GetType()) {
-  base::AutoLock auto_lock(runtime_lock_);
-  scoped_refptr<const PermissionSet> required_permissions =
+  const PermissionSet* required_permissions =
       PermissionsParser::GetRequiredPermissions(extension);
-  active_permissions_unsafe_ =
+  active_permissions_unsafe_.reset(
       new PermissionSet(required_permissions->apis(),
                         required_permissions->manifest_permissions(),
                         required_permissions->explicit_hosts(),
-                        required_permissions->scriptable_hosts());
-  withheld_permissions_unsafe_ = new PermissionSet();
+                        required_permissions->scriptable_hosts()));
+  withheld_permissions_unsafe_.reset(new PermissionSet());
 }
 
 PermissionsData::~PermissionsData() {
@@ -75,6 +87,7 @@ bool PermissionsData::ScriptsMayRequireActionForExtension(
       permissions->ShouldWarnAllHosts();
 }
 
+// static
 bool PermissionsData::ShouldSkipPermissionWarnings(
     const std::string& extension_id) {
   // See http://b/4946060 for more details.
@@ -121,60 +134,74 @@ bool PermissionsData::IsRestrictedUrl(const GURL& document_url,
   return false;
 }
 
+void PermissionsData::BindToCurrentThread() const {
+  DCHECK(!thread_checker_);
+  thread_checker_.reset(new base::ThreadChecker());
+}
+
 void PermissionsData::SetPermissions(
-    const scoped_refptr<const PermissionSet>& active,
-    const scoped_refptr<const PermissionSet>& withheld) const {
-  base::AutoLock auto_lock(runtime_lock_);
-  active_permissions_unsafe_ = active;
-  withheld_permissions_unsafe_ = withheld;
+    scoped_ptr<const PermissionSet> active,
+    scoped_ptr<const PermissionSet> withheld) const {
+  AutoLockOnValidThread lock(runtime_lock_, thread_checker_.get());
+  active_permissions_unsafe_ = active.Pass();
+  withheld_permissions_unsafe_ = withheld.Pass();
+}
+
+void PermissionsData::SetActivePermissions(
+    scoped_ptr<const PermissionSet> active) const {
+  AutoLockOnValidThread lock(runtime_lock_, thread_checker_.get());
+  active_permissions_unsafe_ = active.Pass();
 }
 
 void PermissionsData::UpdateTabSpecificPermissions(
     int tab_id,
-    scoped_refptr<const PermissionSet> permissions) const {
-  base::AutoLock auto_lock(runtime_lock_);
+    const PermissionSet& permissions) const {
+  AutoLockOnValidThread lock(runtime_lock_, thread_checker_.get());
   CHECK_GE(tab_id, 0);
-  TabPermissionsMap::iterator iter = tab_specific_permissions_.find(tab_id);
-  if (iter == tab_specific_permissions_.end())
-    tab_specific_permissions_[tab_id] = permissions;
-  else
-    iter->second = PermissionSet::CreateUnion(*(iter->second), *permissions);
+  TabPermissionsMap::const_iterator iter =
+      tab_specific_permissions_.find(tab_id);
+  scoped_ptr<const PermissionSet> new_permissions = PermissionSet::CreateUnion(
+      iter == tab_specific_permissions_.end()
+          ? static_cast<const PermissionSet&>(PermissionSet())
+          : *iter->second,
+      permissions);
+  tab_specific_permissions_.set(tab_id, new_permissions.Pass());
 }
 
 void PermissionsData::ClearTabSpecificPermissions(int tab_id) const {
-  base::AutoLock auto_lock(runtime_lock_);
+  AutoLockOnValidThread lock(runtime_lock_, thread_checker_.get());
   CHECK_GE(tab_id, 0);
   tab_specific_permissions_.erase(tab_id);
 }
 
 bool PermissionsData::HasAPIPermission(APIPermission::ID permission) const {
-  return active_permissions()->HasAPIPermission(permission);
+  base::AutoLock auto_lock(runtime_lock_);
+  return active_permissions_unsafe_->HasAPIPermission(permission);
 }
 
 bool PermissionsData::HasAPIPermission(
     const std::string& permission_name) const {
-  return active_permissions()->HasAPIPermission(permission_name);
+  base::AutoLock auto_lock(runtime_lock_);
+  return active_permissions_unsafe_->HasAPIPermission(permission_name);
 }
 
 bool PermissionsData::HasAPIPermissionForTab(
     int tab_id,
     APIPermission::ID permission) const {
-  if (HasAPIPermission(permission))
+  base::AutoLock auto_lock(runtime_lock_);
+  if (active_permissions_unsafe_->HasAPIPermission(permission))
     return true;
 
-  scoped_refptr<const PermissionSet> tab_permissions =
-      GetTabSpecificPermissions(tab_id);
-
-  // Place autolock below the HasAPIPermission() and
-  // GetTabSpecificPermissions(), since each already acquires the lock.
-  base::AutoLock auto_lock(runtime_lock_);
-  return tab_permissions.get() && tab_permissions->HasAPIPermission(permission);
+  const PermissionSet* tab_permissions = GetTabSpecificPermissions(tab_id);
+  return tab_permissions && tab_permissions->HasAPIPermission(permission);
 }
 
 bool PermissionsData::CheckAPIPermissionWithParam(
     APIPermission::ID permission,
     const APIPermission::CheckParam* param) const {
-  return active_permissions()->CheckAPIPermissionWithParam(permission, param);
+  base::AutoLock auto_lock(runtime_lock_);
+  return active_permissions_unsafe_->CheckAPIPermissionWithParam(permission,
+                                                                 param);
 }
 
 URLPatternSet PermissionsData::GetEffectiveHostPermissions() const {
@@ -186,24 +213,28 @@ URLPatternSet PermissionsData::GetEffectiveHostPermissions() const {
 }
 
 bool PermissionsData::HasHostPermission(const GURL& url) const {
-  return active_permissions()->HasExplicitAccessToOrigin(url);
+  base::AutoLock auto_lock(runtime_lock_);
+  return active_permissions_unsafe_->HasExplicitAccessToOrigin(url);
 }
 
 bool PermissionsData::HasEffectiveAccessToAllHosts() const {
-  return active_permissions()->HasEffectiveAccessToAllHosts();
+  base::AutoLock auto_lock(runtime_lock_);
+  return active_permissions_unsafe_->HasEffectiveAccessToAllHosts();
 }
 
 PermissionMessages PermissionsData::GetPermissionMessages() const {
+  base::AutoLock auto_lock(runtime_lock_);
   return PermissionMessageProvider::Get()->GetPermissionMessages(
       PermissionMessageProvider::Get()->GetAllPermissionIDs(
-          active_permissions().get(), manifest_type_));
+          active_permissions_unsafe_.get(), manifest_type_));
 }
 
 bool PermissionsData::HasWithheldImpliedAllHosts() const {
+  base::AutoLock auto_lock(runtime_lock_);
   // Since we currently only withhold all_hosts, it's sufficient to check
   // that either set is not empty.
-  return !withheld_permissions()->explicit_hosts().is_empty() ||
-         !withheld_permissions()->scriptable_hosts().is_empty();
+  return !withheld_permissions_unsafe_->explicit_hosts().is_empty() ||
+         !withheld_permissions_unsafe_->scriptable_hosts().is_empty();
 }
 
 bool PermissionsData::CanAccessPage(const Extension* extension,
@@ -211,13 +242,11 @@ bool PermissionsData::CanAccessPage(const Extension* extension,
                                     int tab_id,
                                     int process_id,
                                     std::string* error) const {
-  AccessType result = CanRunOnPage(extension,
-                                   document_url,
-                                   tab_id,
-                                   process_id,
-                                   active_permissions()->explicit_hosts(),
-                                   withheld_permissions()->explicit_hosts(),
-                                   error);
+  base::AutoLock auto_lock(runtime_lock_);
+  AccessType result =
+      CanRunOnPage(extension, document_url, tab_id, process_id,
+                   active_permissions_unsafe_->explicit_hosts(),
+                   withheld_permissions_unsafe_->explicit_hosts(), error);
   // TODO(rdevlin.cronin) Update callers so that they only need ACCESS_ALLOWED.
   return result == ACCESS_ALLOWED || result == ACCESS_WITHHELD;
 }
@@ -228,13 +257,10 @@ PermissionsData::AccessType PermissionsData::GetPageAccess(
     int tab_id,
     int process_id,
     std::string* error) const {
-  return CanRunOnPage(extension,
-                      document_url,
-                      tab_id,
-                      process_id,
-                      active_permissions()->explicit_hosts(),
-                      withheld_permissions()->explicit_hosts(),
-                      error);
+  base::AutoLock auto_lock(runtime_lock_);
+  return CanRunOnPage(extension, document_url, tab_id, process_id,
+                      active_permissions_unsafe_->explicit_hosts(),
+                      withheld_permissions_unsafe_->explicit_hosts(), error);
 }
 
 bool PermissionsData::CanRunContentScriptOnPage(const Extension* extension,
@@ -242,13 +268,11 @@ bool PermissionsData::CanRunContentScriptOnPage(const Extension* extension,
                                                 int tab_id,
                                                 int process_id,
                                                 std::string* error) const {
-  AccessType result = CanRunOnPage(extension,
-                                   document_url,
-                                   tab_id,
-                                   process_id,
-                                   active_permissions()->scriptable_hosts(),
-                                   withheld_permissions()->scriptable_hosts(),
-                                   error);
+  base::AutoLock auto_lock(runtime_lock_);
+  AccessType result =
+      CanRunOnPage(extension, document_url, tab_id, process_id,
+                   active_permissions_unsafe_->scriptable_hosts(),
+                   withheld_permissions_unsafe_->scriptable_hosts(), error);
   // TODO(rdevlin.cronin) Update callers so that they only need ACCESS_ALLOWED.
   return result == ACCESS_ALLOWED || result == ACCESS_WITHHELD;
 }
@@ -259,13 +283,10 @@ PermissionsData::AccessType PermissionsData::GetContentScriptAccess(
     int tab_id,
     int process_id,
     std::string* error) const {
-  return CanRunOnPage(extension,
-                      document_url,
-                      tab_id,
-                      process_id,
-                      active_permissions()->scriptable_hosts(),
-                      withheld_permissions()->scriptable_hosts(),
-                      error);
+  base::AutoLock auto_lock(runtime_lock_);
+  return CanRunOnPage(extension, document_url, tab_id, process_id,
+                      active_permissions_unsafe_->scriptable_hosts(),
+                      withheld_permissions_unsafe_->scriptable_hosts(), error);
 }
 
 bool PermissionsData::CanCaptureVisiblePage(int tab_id,
@@ -273,13 +294,13 @@ bool PermissionsData::CanCaptureVisiblePage(int tab_id,
   const URLPattern all_urls(URLPattern::SCHEME_ALL,
                             URLPattern::kAllUrlsPattern);
 
-  if (active_permissions()->explicit_hosts().ContainsPattern(all_urls))
+  base::AutoLock auto_lock(runtime_lock_);
+  if (active_permissions_unsafe_->explicit_hosts().ContainsPattern(all_urls))
     return true;
 
   if (tab_id >= 0) {
-    scoped_refptr<const PermissionSet> tab_permissions =
-        GetTabSpecificPermissions(tab_id);
-    if (tab_permissions.get() &&
+    const PermissionSet* tab_permissions = GetTabSpecificPermissions(tab_id);
+    if (tab_permissions &&
         tab_permissions->HasAPIPermission(APIPermission::kTab)) {
       return true;
     }
@@ -293,28 +314,22 @@ bool PermissionsData::CanCaptureVisiblePage(int tab_id,
   return false;
 }
 
-PermissionsData::TabPermissionsMap
-PermissionsData::CopyTabSpecificPermissionsMap() const {
-  base::AutoLock auto_lock(runtime_lock_);
-  return tab_specific_permissions_;
-}
-
-scoped_refptr<const PermissionSet> PermissionsData::GetTabSpecificPermissions(
+const PermissionSet* PermissionsData::GetTabSpecificPermissions(
     int tab_id) const {
-  base::AutoLock auto_lock(runtime_lock_);
   CHECK_GE(tab_id, 0);
+  runtime_lock_.AssertAcquired();
   TabPermissionsMap::const_iterator iter =
       tab_specific_permissions_.find(tab_id);
-  return (iter != tab_specific_permissions_.end()) ? iter->second : NULL;
+  return (iter != tab_specific_permissions_.end()) ? iter->second : nullptr;
 }
 
 bool PermissionsData::HasTabSpecificPermissionToExecuteScript(
     int tab_id,
     const GURL& url) const {
+  runtime_lock_.AssertAcquired();
   if (tab_id >= 0) {
-    scoped_refptr<const PermissionSet> tab_permissions =
-        GetTabSpecificPermissions(tab_id);
-    if (tab_permissions.get() &&
+    const PermissionSet* tab_permissions = GetTabSpecificPermissions(tab_id);
+    if (tab_permissions &&
         tab_permissions->explicit_hosts().MatchesSecurityOrigin(url)) {
       return true;
     }
@@ -330,6 +345,7 @@ PermissionsData::AccessType PermissionsData::CanRunOnPage(
     const URLPatternSet& permitted_url_patterns,
     const URLPatternSet& withheld_url_patterns,
     std::string* error) const {
+  runtime_lock_.AssertAcquired();
   if (g_policy_delegate &&
       !g_policy_delegate->CanExecuteScriptOnPage(
           extension, document_url, tab_id, process_id, error)) {
diff --git a/extensions/common/permissions/permissions_data.h b/extensions/common/permissions/permissions_data.h
index 43be869..d7d5f27 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);
 };