Cleanup extension permissions module.

chrome/browser/extensions/extension_permissions_api.cc

Index: chrome/browser/extensions/extension_permissions_api.cc
diff --git a/chrome/browser/extensions/extension_permissions_api.cc b/chrome/browser/extensions/extension_permissions_api.cc
index b6ce2939315e7bbff88858319c7465b0a52bfb21..9ec4a4c7bd8fa2ce69d3cc93a9801d6c05122a02 100644
--- a/chrome/browser/extensions/extension_permissions_api.cc
+++ b/chrome/browser/extensions/extension_permissions_api.cc
@@ -14,8 +14,8 @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_error_utils.h"
 #include "chrome/common/extensions/extension_messages.h"
-#include "chrome/common/extensions/extension_permission_set.h"
 #include "chrome/common/extensions/url_pattern_set.h"
+#include "content/browser/renderer_host/render_process_host.h"
 #include "content/public/browser/notification_service.h"
 #include "googleurl/src/gurl.h"
 
@@ -135,13 +135,13 @@ bool UnpackPermissionsFromValue(DictionaryValue* value,
 
 } // namespace
 
-ExtensionPermissionsManager::ExtensionPermissionsManager(
+ExtensionPermissionsUpdater::ExtensionPermissionsUpdater(
     ExtensionService* extension_service)
     : extension_service_(extension_service) {}
 
-ExtensionPermissionsManager::~ExtensionPermissionsManager() {}
+ExtensionPermissionsUpdater::~ExtensionPermissionsUpdater() {}
 
-void ExtensionPermissionsManager::AddPermissions(
+void ExtensionPermissionsUpdater::AddPermissions(
     const Extension* extension, const ExtensionPermissionSet* permissions) {
   scoped_refptr<const ExtensionPermissionSet> existing(
       extension->GetActivePermissions());
@@ -158,7 +158,7 @@ void ExtensionPermissionsManager::AddPermissions(
   NotifyPermissionsUpdated(ADDED, extension, added.get());
 }
 
-void ExtensionPermissionsManager::RemovePermissions(
+void ExtensionPermissionsUpdater::RemovePermissions(
     const Extension* extension, const ExtensionPermissionSet* permissions) {
   scoped_refptr<const ExtensionPermissionSet> existing(
       extension->GetActivePermissions());
@@ -175,7 +175,7 @@ void ExtensionPermissionsManager::RemovePermissions(
   NotifyPermissionsUpdated(REMOVED, extension, removed.get());
 }
 
-void ExtensionPermissionsManager::DispatchEvent(
+void ExtensionPermissionsUpdater::DispatchEvent(
     const std::string& extension_id,
     const char* event_name,
     const ExtensionPermissionSet* changed_permissions) {
@@ -190,7 +190,7 @@ void ExtensionPermissionsManager::DispatchEvent(
   }
 }
 
-void ExtensionPermissionsManager::NotifyPermissionsUpdated(
+void ExtensionPermissionsUpdater::NotifyPermissionsUpdated(
     EventType event_type,
     const Extension* extension,
     const ExtensionPermissionSet* changed) {
@@ -270,8 +270,6 @@ bool RemovePermissionsFunction::RunImpl() {
   CHECK(permissions.get());
 
   const Extension* extension = GetExtension();
-  ExtensionPermissionsManager* perms_manager =
-      profile()->GetExtensionService()->permissions_manager();
   ExtensionPermissionsInfo* info = ExtensionPermissionsInfo::GetInstance();
 
   // Make sure they're only trying to remove permissions supported by this API.
@@ -296,7 +294,8 @@ bool RemovePermissionsFunction::RunImpl() {
     return false;
   }
 
-  perms_manager->RemovePermissions(extension, permissions.get());
+  ExtensionPermissionsUpdater perms_updater(profile()->GetExtensionService());
+  perms_updater.RemovePermissions(extension, permissions.get());
   result_.reset(Value::CreateBooleanValue(true));
   return true;
 }
@@ -331,10 +330,7 @@ bool RequestPermissionsFunction::RunImpl() {
     return false;
   CHECK(requested_permissions_.get());
 
-  extension_ = GetExtension();
   ExtensionPermissionsInfo* info = ExtensionPermissionsInfo::GetInstance();
-  ExtensionPermissionsManager* perms_manager =
-      profile()->GetExtensionService()->permissions_manager();
   ExtensionPrefs* prefs = profile()->GetExtensionService()->extension_prefs();
 
   // Make sure they're only requesting permissions supported by this API.
@@ -350,7 +346,7 @@ bool RequestPermissionsFunction::RunImpl() {
   }
 
   // The requested permissions must be defined as optional in the manifest.
-  if (!extension_->optional_permission_set()->Contains(
+  if (!GetExtension()->optional_permission_set()->Contains(
           *requested_permissions_)) {
     error_ = kNotInOptionalPermissionsError;
     result_.reset(Value::CreateBooleanValue(false));
@@ -360,9 +356,10 @@ bool RequestPermissionsFunction::RunImpl() {
   // We don't need to prompt the user if the requested permissions are a subset
   // of the granted permissions set.
   const ExtensionPermissionSet* granted =
-      prefs->GetGrantedPermissions(extension_->id());
+      prefs->GetGrantedPermissions(GetExtension()->id());
   if (granted && granted->Contains(*requested_permissions_)) {
-    perms_manager->AddPermissions(extension_, requested_permissions_.get());
+    ExtensionPermissionsUpdater perms_updater(profile()->GetExtensionService());
+    perms_updater.AddPermissions(GetExtension(), requested_permissions_.get());
     result_.reset(Value::CreateBooleanValue(true));
     SendResponse(true);
     return true;
@@ -372,8 +369,7 @@ bool RequestPermissionsFunction::RunImpl() {
   requested_permissions_ = ExtensionPermissionSet::CreateDifference(
       requested_permissions_.get(), granted);
 
-  // Balanced with Release() in InstallUIProceed() and InstallUIAbort().
-  AddRef();
+  AddRef();  // Balanced in InstallUIProceed() / InstallUIAbort().
 
   // We don't need to show the prompt if there are no new warnings, or if
   // we're skipping the confirmation UI. All extension types but INTERNAL
@@ -388,30 +384,25 @@ bool RequestPermissionsFunction::RunImpl() {
     CHECK_EQ(DO_NOT_SKIP, auto_confirm_for_tests);
     install_ui_.reset(new ExtensionInstallUI(profile()));
     install_ui_->ConfirmPermissions(
-        this, extension_, requested_permissions_.get());
+        this, GetExtension(), requested_permissions_.get());
   }
 
   return true;
 }
 
 void RequestPermissionsFunction::InstallUIProceed() {
-  ExtensionPermissionsManager* perms_manager =
-      profile()->GetExtensionService()->permissions_manager();
+  ExtensionPermissionsUpdater perms_updater(profile()->GetExtensionService());
+  perms_updater.AddPermissions(GetExtension(), requested_permissions_.get());

[Aaron Boodman, 2011/11/14 19:26:05]

I can't remember - if an extension is unloaded, does GetExtension() start
returning NULL? I think it might. In that case, you'll need to NULL-check this.

[jstritar, 2011/12/12 22:28:03]

The ExtensionFunction has a scoped_refptr to the Extension, so I think it should
stay around.

 
-  install_ui_.reset();
   result_.reset(Value::CreateBooleanValue(true));
-  perms_manager->AddPermissions(extension_, requested_permissions_.get());
-
   SendResponse(true);
 
-  Release();
+  Release();  // Balanced in RunImpl().
 }
 
 void RequestPermissionsFunction::InstallUIAbort(bool user_initiated) {
-  install_ui_.reset();
   result_.reset(Value::CreateBooleanValue(false));
-  requested_permissions_ = NULL;
-
   SendResponse(true);
-  Release();
+
+  Release();  // Balanced in RunImpl().
 }