Cleanup: give ChromeAppSorting a BrowserContext

extensions/browser/extensions_browser_client.h

 // 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.
 
 #ifndef EXTENSIONS_BROWSER_EXTENSIONS_BROWSER_CLIENT_H_
 #define EXTENSIONS_BROWSER_EXTENSIONS_BROWSER_CLIENT_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 143 matching lines @@
   // once each time the extensions system is loaded per browser_context. The
   // implementation may wish to use the BrowserContext to record the current
   // version for later comparison.
   virtual bool DidVersionUpdate(content::BrowserContext* context) = 0;
 
   // Permits an external protocol handler to be launched. See
   // ExternalProtocolHandler::PermitLaunchUrl() in Chrome.
   virtual void PermitExternalProtocolHandler() = 0;
 
   // Creates a new AppSorting instance.
-  virtual scoped_ptr<AppSorting> CreateAppSorting() = 0;
+  virtual scoped_ptr<AppSorting> CreateAppSorting(

[not at google - send to devlin, 2015/07/23 15:35:11]

Even more cleanup:

This stuff would all be much simpler if there were an "app_sorting()" method on
ExtensionSystem, which ExtensionSystemImpl overrides to return a
ChromeAppSorting, and everything else returns a NullAppSorting/nullptr (and why
are both acceptable? eh whatever).

I.e. shift ownership of AppSorting from ExtensionPrefs to ExtensionSystem.

That would let you delete AppSorting::SetExtensionScopedPrefs, and instead
ChromeAppSorting could just retrieve the ExtensionPrefs instance from the
BrowserContext it's given.

It would also let you simply delete ExtensionsBrowserClient::CreateAppSorting
rather than updating it.

[Marc Treib, 2015/07/24 11:38:34]

I started on this, but then realized it creates a dependency cycle:
ExtensionSystem depends on ExtensionPrefs, but ExtensionPrefs needs an
AppSorting and so now will depend on ExtensionSystem. That could probably be
worked around with an ExtensionPrefs::set_app_sorting or something like that,
but then we won't really win much by the whole thing. WDYT?

[not at google - send to devlin, 2015/07/24 14:43:53]

That's pretty typical, usually all these things depend on in their constructor
is a Profile/BrowserContext. They fetch the ExtensionPrefs/AppSorting from that,
sometimes via an ExtensionSystem, when needed.

+      content::BrowserContext* context) = 0;
 
   // Return true if the system is run in forced app mode.
   virtual bool IsRunningInForcedAppMode() = 0;
 
   // Returns the embedder's ApiActivityMonitor for |context|. Returns NULL if
   // the embedder does not monitor extension API activity.
   virtual ApiActivityMonitor* GetApiActivityMonitor(
       content::BrowserContext* context) = 0;
 
   // Returns the factory that provides an ExtensionSystem to be returned from
@@ skipping 54 matching lines @@
   // Returns the single instance of |this|.
   static ExtensionsBrowserClient* Get();
 
   // Initialize the single instance.
   static void Set(ExtensionsBrowserClient* client);
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_EXTENSIONS_BROWSER_CLIENT_H_