Refactor the various ways to control what users can do to extensions.

chrome/browser/extensions/extension_management_api.cc

Index: chrome/browser/extensions/extension_management_api.cc
===================================================================
--- chrome/browser/extensions/extension_management_api.cc	(revision 138227)
+++ chrome/browser/extensions/extension_management_api.cc	(working copy)
@@ -17,6 +17,7 @@
 #include "chrome/browser/extensions/extension_event_router.h"
 #include "chrome/browser/extensions/extension_management_api_constants.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
@@ -59,6 +60,10 @@
   bool enabled = service->IsExtensionEnabled(extension.id());
   extension.GetBasicInfo(enabled, info);
 
+  info->SetBoolean(keys::kMayDisableKey,

[Pam (message me for reviews), 2012/05/22 12:51:07]

It's part of the ExtensionInfo returned by the API. It was originally set here,
then it moved into Extension::GetBasicInfo(), and now it has to move back out
since an
extension doesn't know about ManagementPolicy.

[Aaron Boodman, 2012/05/22 15:56:11]

No what I meant was: is there a client that needs to know whether an extension
may be disabled?

Also, as to wrapping, I meant that wrapping at the arrow (either before or
after) looks strange to me, and I'd prefer wrapping at the open paren or storing
a pointer if possible.

That said, looking at this code, you use this pattern everywhere, and the lesser
of two evils is probably to just do it all on one virtual line, wrapped *after*
the arrow (sorry).

[Pam (message me for reviews), 2012/05/23 15:00:58]

Sorry, I could have been clearer. It's a documented field in the ExtensionInfo
object returned by the public chrome.management.get() extension API.
In some places there wasn't a good open paren to wrap at (wrapping between empty
() looks ludicrous to me), but I'll change them to store pointers.

+    ExtensionSystem::Get(service->profile())->management_policy()
+        ->UserMayModifyStatus(&extension, NULL));
+
   info->SetBoolean(keys::kIsAppKey, extension.is_app());
 
   if (!enabled) {
@@ -366,9 +371,10 @@
     return false;
   }
 
-  if (!Extension::UserMayDisable(extension->location())) {
+  if (!ExtensionSystem::Get(profile())->management_policy()
+      ->UserMayModifyStatus(extension, NULL)) {
     error_ = ExtensionErrorUtils::FormatErrorMessage(
-        keys::kUserCantDisableError, extension_id_);
+        keys::kUserCantModifyError, extension_id_);

[Pam (message me for reviews), 2012/05/22 12:51:07]

Excellent, I'd love to consolidate them.

     return false;
   }
 
@@ -421,12 +427,11 @@
     return false;
   }
 
-  ExtensionPrefs* prefs = service()->extension_prefs();
-
-  if (!Extension::UserMayDisable(
-      prefs->GetInstalledExtensionInfo(extension_id)->extension_location)) {
+  const Extension* extension = service()->GetExtensionById(extension_id, true);
+  if (!ExtensionSystem::Get(profile())->management_policy()
+      ->UserMayModifyStatus(extension, NULL)) {
     error_ = ExtensionErrorUtils::FormatErrorMessage(
-        keys::kUserCantDisableError, extension_id);
+        keys::kUserCantModifyError, extension_id);
     return false;
   }