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

extensions/browser/extension_prefs.h

Index: extensions/browser/extension_prefs.h
diff --git a/extensions/browser/extension_prefs.h b/extensions/browser/extension_prefs.h
index 2e9e84035c11d6c7e5af4238005698d4ef944688..b135ea09f640fdd771f61333cb52feb5f44077b1 100644
--- a/extensions/browser/extension_prefs.h
+++ b/extensions/browser/extension_prefs.h
@@ -511,11 +511,6 @@ class ExtensionPrefs : public ExtensionScopedPrefs, public KeyedService {
   // 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.
@@ -570,6 +565,9 @@ class ExtensionPrefs : public ExtensionScopedPrefs, public KeyedService {
                  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();