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

chrome/browser/extensions/extension_system.cc

Index: chrome/browser/extensions/extension_system.cc
===================================================================
--- chrome/browser/extensions/extension_system.cc	(revision 138227)
+++ chrome/browser/extensions/extension_system.cc	(working copy)
@@ -26,6 +26,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system_factory.h"
 #include "chrome/browser/extensions/lazy_background_task_queue.h"
+#include "chrome/browser/extensions/management_policy.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/extensions/user_script_master.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -84,6 +85,13 @@
   extension_prefs_->Init(extensions_disabled);
 }
 
+void ExtensionSystemImpl::Shared::RegisterManagementPolicyProviders() {
+  DCHECK(extension_prefs_.get());
+  management_policy_->RegisterProvider(extension_prefs_.get());
+  DCHECK(extension_service_.get());
+  management_policy_->RegisterProvider(extension_service_.get());

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

This circular dependency between ManagementPolicy and ExtensionService is weird
and would be great to get rid of.

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

I'm not sure I follow. Moving the ManagementPolicy over to the ExtensionSystem
got rid of the circular dependency. ExtensionSystem and ExtensionService still
cross-reference each other, but that's not new.

ExtensionSystem owns a ManagementPolicy, and also owns an ExtensionService and
an ExtensionPrefs. Both the ExtensionService and the ExtensionPrefs are
Providers for the ManagementPolicy. It's a little complicated, but not too bad.

+}
+
 void ExtensionSystemImpl::Shared::InitInfoMap() {
   // The ExtensionInfoMap needs to be created before the
   // ExtensionProcessManager.
@@ -120,6 +128,11 @@
       autoupdate_enabled,
       extensions_enabled));
 
+  // The ManagementPolicy must be created and its providers registered before
+  // the ExtensionService tries to load any extensions.
+  management_policy_.reset(new extensions::ManagementPolicy);

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

You could just create the management policy in the constructor. It doesn't even
need to be a scoped pointer.

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

Because we don't care about leaks on shutdown? Or it should be on the stack? Or
am I missing something? Why are all the other components, the
extension_event_router_ and extension_service_ and so on, scoped pointers, and
not created until here?

[Aaron Boodman, 2012/05/24 08:11:23]

I meant that it could be a plain old member. I don't know why no other pieces do
this... it's fine the way you have it.

+  RegisterManagementPolicyProviders();
+
   extension_service_->component_loader()->AddDefaultComponentExtensions();
   if (command_line->HasSwitch(switches::kLoadComponentExtension)) {
     CommandLine::StringType path_list = command_line->GetSwitchValueNative(
@@ -189,6 +202,10 @@
   return extension_service_.get();
 }
 
+extensions::ManagementPolicy* ExtensionSystemImpl::Shared::management_policy() {
+  return management_policy_.get();
+}
+
 UserScriptMaster* ExtensionSystemImpl::Shared::user_script_master() {
   return user_script_master_.get();
 }
@@ -261,6 +278,10 @@
   return shared_->extension_service();
 }
 
+extensions::ManagementPolicy* ExtensionSystemImpl::management_policy() {
+  return shared_->management_policy();
+}
+
 UserScriptMaster* ExtensionSystemImpl::user_script_master() {
   return shared_->user_script_master();
 }