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

chrome/browser/extensions/extension_system.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_system.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/string_tokenizer.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/component_loader.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/extensions/api/alarms/alarm_manager.h"
 #include "chrome/browser/extensions/api/declarative/rules_registry_service.h"
 #include "chrome/browser/extensions/extension_devtools_manager.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_event_router.h"
 #include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/extensions/extension_message_service.h"
 #include "chrome/browser/extensions/extension_navigation_observer.h"
 #include "chrome/browser/extensions/extension_pref_store.h"
 #include "chrome/browser/extensions/extension_pref_value_map.h"
 #include "chrome/browser/extensions/extension_pref_value_map_factory.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #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"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 #include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/feature.h"
@@ skipping 38 matching lines @@
   bool extensions_disabled =
       profile_->GetPrefs()->GetBoolean(prefs::kDisableExtensions) ||
       CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableExtensions);
   extension_prefs_.reset(new ExtensionPrefs(
       profile_->GetPrefs(),
       profile_->GetPath().AppendASCII(ExtensionService::kInstallDirectoryName),
       ExtensionPrefValueMapFactory::GetForProfile(profile_)));
   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.
   extension_info_map_ = new ExtensionInfoMap();
 }
 
 void ExtensionSystemImpl::Shared::Init(bool extensions_enabled) {
   const CommandLine* command_line = CommandLine::ForCurrentProcess();
 
   lazy_background_task_queue_.reset(new extensions::LazyBackgroundTaskQueue(
@@ skipping 16 matching lines @@
     autoupdate_enabled = !command_line->HasSwitch(switches::kGuestSession);
 #endif
   extension_service_.reset(new ExtensionService(
       profile_,
       CommandLine::ForCurrentProcess(),
       profile_->GetPath().AppendASCII(ExtensionService::kInstallDirectoryName),
       extension_prefs_.get(),
       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(
         switches::kLoadComponentExtension);
     StringTokenizerT<CommandLine::StringType,
         CommandLine::StringType::const_iterator> t(path_list,
                                                    FILE_PATH_LITERAL(","));
     while (t.GetNext()) {
       // Load the component extension manifest synchronously.
       // Blocking the UI thread is acceptable here since
@@ skipping 49 matching lines @@
   }
 
   rules_registry_service_.reset(new extensions::RulesRegistryService(profile_));
   rules_registry_service_->RegisterDefaultRulesRegistries();
 }
 
 ExtensionService* ExtensionSystemImpl::Shared::extension_service() {
   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();
 }
 
 ExtensionInfoMap* ExtensionSystemImpl::Shared::info_map() {
   return extension_info_map_.get();
 }
 
 extensions::LazyBackgroundTaskQueue*
 ExtensionSystemImpl::Shared::lazy_background_task_queue() {
@@ skipping 52 matching lines @@
   extension_process_manager_.reset(ExtensionProcessManager::Create(profile_));
   alarm_manager_.reset(new extensions::AlarmManager(profile_));
 
   shared_->Init(extensions_enabled);
 }
 
 ExtensionService* ExtensionSystemImpl::extension_service() {
   return shared_->extension_service();
 }
 
+extensions::ManagementPolicy* ExtensionSystemImpl::management_policy() {
+  return shared_->management_policy();
+}
+
 UserScriptMaster* ExtensionSystemImpl::user_script_master() {
   return shared_->user_script_master();
 }
 
 ExtensionDevToolsManager* ExtensionSystemImpl::devtools_manager() {
   // TODO(mpcomplete): in incognito, figure out whether we should
   // return the original profile's version.
   return extension_devtools_manager_.get();
 }
 
@@ skipping 44 matching lines @@
 }
 
 void ExtensionSystemImpl::UnregisterExtensionWithRequestContexts(
     const std::string& extension_id,
     const extension_misc::UnloadedExtensionReason reason) {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&ExtensionInfoMap::RemoveExtension, info_map(),
                  extension_id, reason));
 }