OBSOLETE REVIEW. See http://codereview.chromium.org/10382149/

chrome/browser/extensions/installed_loader.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/installed_loader.h"
 
 #include "base/file_path.h"
 #include "base/metrics/histogram.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
@@ skipping 51 matching lines @@
 
 InstalledLoader::~InstalledLoader() {
 }
 
 void InstalledLoader::Load(const ExtensionInfo& info, bool write_to_prefs) {
   std::string error;
   scoped_refptr<const Extension> extension(NULL);
   // An explicit check against policy is required to behave correctly during
   // startup.  This is because extensions that were previously OK might have
   // been blacklisted in policy while Chrome was not running.
-  if (!extension_prefs_->IsExtensionAllowedByPolicy(info.extension_id,
-                                                    info.extension_location)) {
+  // This isn't a new extension installation, so it's OK if some extension
+  // management policy delegates aren't yet registered with the
+  // ExtensionService. Admin policy is handled by the ExtensionPrefs class,
+  // which is created before the ExtensionService itself, so we have what we
+  // need here.
+  if (!extension_service_->extension_management_policy()->
+      UserMayInstall(info.extension_id, info.extension_location, std::string(),

[Aaron Boodman, 2012/05/17 22:41:06]

Maybe the name of this method should be UserMayRun() or UserMayLoad(), since
it's not really limited to installation.

+      NULL)) {
+    // The error message from UserMayInstall() is not well suited to this UI.

[Aaron Boodman, 2012/05/17 22:41:06]

Why?

[Pam (message me for reviews), 2012/05/18 20:15:15]

It is typically IDS_EXTENSION_CANT_INSTALL_POLICY_BLACKLIST, which contains the
extension ID. That's pretty ugly to have appear in user UI (as opposed to log
messages), but I'm willing to use it if you think it's good.

     error = errors::kDisabledByPolicy;
   } else if (info.extension_manifest.get()) {
     extension = Extension::Create(

[Pam (message me for reviews), 2012/05/18 20:15:15]

Is it permissible to Create the extension before checking the admin policy? It
looks safe, and doing that allows UserMayLoad() to take a full Extension*, which
may be useful to some providers.

         info.extension_path,
         info.extension_location,
         *info.extension_manifest,
         GetCreationFlags(&info),
         &error);
   } else {
     error = errors::kManifestUnreadable;
   }
 
   // Once installed, non-unpacked extensions cannot change their IDs (e.g., by
@@ skipping 208 matching lines @@
   if (extension_prefs_->AllowFileAccess(info->extension_id))
     flags |= Extension::ALLOW_FILE_ACCESS;
   if (extension_prefs_->IsFromWebStore(info->extension_id))
     flags |= Extension::FROM_WEBSTORE;
   if (extension_prefs_->IsFromBookmark(info->extension_id))
     flags |= Extension::FROM_BOOKMARK;
   return flags;
 }
 
 }  // namespace extensions