Cleanup: Remove some deprecated ExtensionService usage.

chrome/browser/extensions/extension_util.cc

 // Copyright 2013 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_util.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ skipping 16 matching lines @@
 #include "extensions/common/manifest.h"
 #include "extensions/common/manifest_handlers/incognito_info.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "grit/theme_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 
 namespace extensions {
 namespace util {
 
 namespace {
+
 // The entry into the ExtensionPrefs for allowing an extension to script on
 // all urls without explicit permission.
 const char kExtensionAllowedOnAllUrlsPrefName[] =
     "extension_can_script_all_urls";
 
 // Returns true if |extension_id| for an external component extension should
 // always be enabled in incognito windows.
 bool IsWhitelistedForIncognito(const std::string& extension_id) {
   static const char* kExtensionWhitelist[] = {
     "D5736E4B5CF695CB93A2FB57E4FDC6E5AFAB6FE2",  // http://crbug.com/312900
     "D57DE394F36DC1C3220E7604C575D29C51A6C495",  // http://crbug.com/319444
     "3F65507A3B39259B38C8173C6FFA3D12DF64CCE9"   // http://crbug.com/371562
   };
 
   return extensions::SimpleFeature::IsIdInList(
       extension_id,
       std::set<std::string>(
           kExtensionWhitelist,
           kExtensionWhitelist + arraysize(kExtensionWhitelist)));
 }
+
+// Returns |extension_id|. See note below.
+std::string ReloadExtensionIfEnabled(const std::string& extension_id,
+                                     content::BrowserContext* context) {
+  ExtensionRegistry* registry = ExtensionRegistry::Get(context);
+  bool extension_is_enabled =
+      registry->enabled_extensions().Contains(extension_id);
+
+  if (extension_is_enabled)
+    return extension_id;

[Devlin, 2014/09/04 20:50:00]

This line means that this is "ReloadExtensionIfNotEnabled", which won't work as
this function is used.

[Lei Zhang, 2014/09/05 06:07:45]

I meant if (!extension_is_enabled) ...

+
+  // When we reload the extension the ID may be invalidated if we've passed it
+  // by const ref everywhere. Make a copy to be safe. http://crbug.com/103762
+  std::string id = extension_id;
+  ExtensionService* service =
+      ExtensionSystem::Get(context)->extension_service();
+  CHECK(service);
+  service->ReloadExtension(id);
+  return id;
+}
+
 }  // namespace
 
 bool IsIncognitoEnabled(const std::string& extension_id,
                         content::BrowserContext* context) {
   const Extension* extension = ExtensionRegistry::Get(context)->
       GetExtensionById(extension_id, ExtensionRegistry::ENABLED);
   if (extension) {
     if (!extension->can_be_incognito_enabled())
       return false;
     // If this is an existing component extension we always allow it to
     // work in incognito mode.
     if (extension->location() == Manifest::COMPONENT)
       return true;
     if (extension->location() == Manifest::EXTERNAL_COMPONENT &&
         IsWhitelistedForIncognito(extension_id)) {
       return true;
     }
   }
 
   return ExtensionPrefs::Get(context)->IsIncognitoEnabled(extension_id);
 }
 
 void SetIsIncognitoEnabled(const std::string& extension_id,
                            content::BrowserContext* context,
                            bool enabled) {
-  ExtensionService* service =
-      ExtensionSystem::Get(context)->extension_service();
-  CHECK(service);
-  const Extension* extension = service->GetInstalledExtension(extension_id);
+  ExtensionRegistry* registry = ExtensionRegistry::Get(context);
+  const Extension* extension =
+      registry->enabled_extensions().GetByID(extension_id);

[Devlin, 2014/09/04 20:50:00]

I think this should probably be registry->GetExtensionById(EVERYTHING) (so that
it includes disabled/etc extensions).

[Lei Zhang, 2014/09/05 06:07:45]

Bleh, I looked up the GetExtensionById() conversion.

 
   if (extension) {
     if (!extension->can_be_incognito_enabled())
       return;
 
     if (extension->location() == Manifest::COMPONENT) {
       // This shouldn't be called for component extensions unless it is called
       // by sync, for syncable component extensions.
       // See http://crbug.com/112290 and associated CLs for the sordid history.
       DCHECK(sync_helper::IsSyncable(extension));
 
       // If we are here, make sure the we aren't trying to change the value.
-      DCHECK_EQ(enabled, IsIncognitoEnabled(extension_id, service->profile()));
+      DCHECK_EQ(enabled, IsIncognitoEnabled(extension_id, context));
       return;
     }
   }
 
-  ExtensionPrefs* extension_prefs = ExtensionPrefs::Get(service->profile());
+  ExtensionPrefs* extension_prefs = ExtensionPrefs::Get(context);
   // Broadcast unloaded and loaded events to update browser state. Only bother
   // if the value changed and the extension is actually enabled, since there is
   // no UI otherwise.
   bool old_enabled = extension_prefs->IsIncognitoEnabled(extension_id);
   if (enabled == old_enabled)
     return;
 
   extension_prefs->SetIsIncognitoEnabled(extension_id, enabled);
 
-  bool extension_is_enabled = service->extensions()->Contains(extension_id);
-
-  // When we reload the extension the ID may be invalidated if we've passed it
-  // by const ref everywhere. Make a copy to be safe.
-  std::string id = extension_id;
-  if (extension_is_enabled)
-    service->ReloadExtension(id);
+  std::string id = ReloadExtensionIfEnabled(extension_id, context);
 
   // Reloading the extension invalidates the |extension| pointer.
-  extension = service->GetInstalledExtension(id);
+  extension = registry->GetExtensionById(id, ExtensionRegistry::EVERYTHING);

[Devlin, 2014/09/04 20:50:00]

Maybe have ReloadExtensionIfEnabled return a valid pointer to the extension,
instead?

[Lei Zhang, 2014/09/05 06:07:45]

In the SetAllowFileAccess() use case, the returned extension pointer would go
unused. That's more wasted work.

[Devlin, 2014/09/05 15:38:09]

Fair.  I think what would be best would actually be for ExtensionService to
return a pointer to the reloaded extension, and then it wouldn't be any extra
work.  But, that's obviously a bigger change, so don't worry about it.

   if (extension) {
-    ExtensionSyncService::Get(service->profile())->
-        SyncExtensionChangeIfNeeded(*extension);
+    Profile* profile = Profile::FromBrowserContext(context);
+    ExtensionSyncService::Get(profile)->SyncExtensionChangeIfNeeded(*extension);
   }
 }
 
 bool CanCrossIncognito(const Extension* extension,
                        content::BrowserContext* context) {
   // We allow the extension to see events and data from another profile iff it
   // uses "spanning" behavior and it has incognito access. "split" mode
   // extensions only see events for a matching profile.
   CHECK(extension);
   return IsIncognitoEnabled(extension->id(), context) &&
@@ skipping 14 matching lines @@
 bool AllowFileAccess(const std::string& extension_id,
                      content::BrowserContext* context) {
   return CommandLine::ForCurrentProcess()->HasSwitch(
              switches::kDisableExtensionsFileAccessCheck) ||
          ExtensionPrefs::Get(context)->AllowFileAccess(extension_id);
 }
 
 void SetAllowFileAccess(const std::string& extension_id,
                         content::BrowserContext* context,
                         bool allow) {
-  ExtensionService* service =
-      ExtensionSystem::Get(context)->extension_service();
-  CHECK(service);
-
   // Reload to update browser state. Only bother if the value changed and the
   // extension is actually enabled, since there is no UI otherwise.
   if (allow == AllowFileAccess(extension_id, context))
     return;
 
   ExtensionPrefs::Get(context)->SetAllowFileAccess(extension_id, allow);
 
-  bool extension_is_enabled = service->extensions()->Contains(extension_id);
-  if (extension_is_enabled)
-    service->ReloadExtension(extension_id);
+  ReloadExtensionIfEnabled(extension_id, context);
 }
 
 bool AllowedScriptingOnAllUrls(const std::string& extension_id,
                                content::BrowserContext* context) {
   bool allowed = false;
   return ExtensionPrefs::Get(context)->ReadPrefAsBoolean(
              extension_id,
              kExtensionAllowedOnAllUrlsPrefName,
              &allowed) &&
          allowed;
@@ skipping 139 matching lines @@
       IDR_EXTENSION_DEFAULT_ICON);
 }
 
 bool IsStreamlinedHostedAppsEnabled() {
   return CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableStreamlinedHostedApps);
 }
 
 }  // namespace util
 }  // namespace extensions