app_sorting() calls should go via ExtensionSystem intsead of prefs.

extensions/browser/extension_prefs.h

 // Copyright 2014 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_EXTENSION_PREFS_H_
 #define EXTENSIONS_BROWSER_EXTENSION_PREFS_H_
 
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 493 matching lines @@
   // history is cleared.
   void ClearLastLaunchTimes();
 
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
   bool extensions_disabled() const { return extensions_disabled_; }
 
   // The underlying PrefService.
   PrefService* pref_service() const { return prefs_; }
 
-  // The underlying AppSorting.
-  // TODO(treib,kalman): This should be private, and all callers should go
-  // through the ExtensionSystem instead.
-  AppSorting* app_sorting() const;
-
   // Schedules garbage collection of an extension's on-disk data on the next
   // start of this ExtensionService. Applies only to extensions with isolated
   // storage.
   void SetNeedsStorageGarbageCollection(bool value);
   bool NeedsStorageGarbageCollection() const;
 
   // Used by AppWindowGeometryCache to persist its cache. These methods
   // should not be called directly.
   const base::DictionaryValue* GetGeometryCache(
         const std::string& extension_id) const;
@@ skipping 34 matching lines @@
 
   // See the Create methods.
   ExtensionPrefs(content::BrowserContext* browser_context,
                  PrefService* prefs,
                  const base::FilePath& root_dir,
                  ExtensionPrefValueMap* extension_pref_value_map,
                  scoped_ptr<TimeProvider> time_provider,
                  bool extensions_disabled,
                  const std::vector<ExtensionPrefsObserver*>& early_observers);
 
+  // The underlying AppSorting.
+  AppSorting* app_sorting() const;

[Dan Beam, 2015/11/12 22:37:21]

this probably shouldn't be const if AppSorting* is mutable

[benwells, 2015/11/12 23:33:49]

I think it is debatable and rests on whether the ExtensionSystem conceptually
owns the AppSorting object. Whichever way its done, it should match
pref_service() above.

Given this is a refactoring (and reducing the visibility of the method from
public to private) I think it is acceptable to leave it as is.

[Dan Beam, 2015/11/17 00:28:48]

ok

+
   // Converts absolute paths in the pref to paths relative to the
   // install_directory_.
   void MakePathsRelative();
 
   // Converts internal relative paths to be absolute. Used for export to
   // consumers who expect full paths.
   void MakePathsAbsolute(base::DictionaryValue* dict);
 
   // Helper function used by GetInstalledExtensionInfo() and
   // GetDelayedInstallInfo() to construct an ExtensionInfo from the provided
@@ skipping 118 matching lines @@
   bool extensions_disabled_;
 
   base::ObserverList<ExtensionPrefsObserver> observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionPrefs);
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_EXTENSION_PREFS_H_