Move ownership of AppSorting from ExtensionPrefs to ExtensionSystem

extensions/browser/app_window/app_window_geometry_cache_unittest.cc

 // 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.
 
 #include "extensions/browser/app_window/app_window_geometry_cache.h"
 
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/mock_pref_change_callback.h"
 #include "base/prefs/pref_service_factory.h"
@@ skipping 77 matching lines @@
   user_prefs::PrefRegistrySyncable* pref_registry =
       new user_prefs::PrefRegistrySyncable;
   // Prefs should be registered before the PrefService is created.
   ExtensionPrefs::RegisterProfilePrefs(pref_registry);
   pref_service_ = factory.Create(pref_registry).Pass();
 
   extension_prefs_.reset(ExtensionPrefs::Create(
       pref_service_.get(),
       browser_context()->GetPath().AppendASCII("Extensions"),
       extension_pref_value_map_.get(),
-      scoped_ptr<AppSorting>(new NullAppSorting),
       false /* extensions_disabled */,
       std::vector<ExtensionPrefsObserver*>()));
+  scoped_ptr<AppSorting> app_sorting(new NullAppSorting);
+  extension_prefs_->set_app_sorting(app_sorting.get());

[not at google - send to devlin, 2015/07/29 18:22:03]

... would making TestExtensionSystem use a NullAppSorting solve all of these,
and let you remove set_app_sorting?

[Marc Treib, 2015/07/30 14:04:12]

..and instead have ExtensionPrefs use
|ExtensionSystem::Get(context)->app_sorting()| instead to get the AppSorting?
Probably possible, yes, but not trivial - right now ExtensionPrefs doesn't have
a BrowserContext, and giving it one is non-trivial, since not all unit tests
even set up a profile.

[not at google - send to devlin, 2015/07/30 18:21:00]

Ah yeah. I saw ExtensionPrefs::Get(BrowserContext*) and figured there was
already a dependency. That is the function that everybody does call, so plugging
it through to actually a member variable of ExtensionPrefs is conceptually
straightforward?

Side note, why does ExtensionPrefs extend KeyedService?
BrowserContextKeyedService I could understand, but what is the point in
extending just KeyedService.

[Marc Treib, 2015/07/31 08:17:49]

Yes, the problem is just with tests that use an ExtensionPrefs, but not a
profile. Those will need special massaging to make this work.
BrowserContextKeyedService doesn't exist (anymore), I guess it's just been
renamed to KeyedService since it doesn't really relate to a BrowserContext?
BrowserContextKeyedService*Factory* exists though, and returns a KeyedService,
see
https://code.google.com/p/chromium/codesearch#chromium/src/components/keyed_s...

 
   cache_.reset(
       new AppWindowGeometryCache(browser_context(), extension_prefs_.get()));
   cache_->SetSyncDelayForTests(0);
 }
 
 void AppWindowGeometryCacheTest::TearDown() {
   cache_.reset();
   extension_prefs_.reset();
   pref_service_.reset();
@@ skipping 310 matching lines @@
   // The first added window should no longer have cached geometry.
   EXPECT_FALSE(cache_->GetGeometry(extension_id, "window_0", NULL, NULL, NULL));
   // All other windows should still exist.
   for (size_t i = 1; i < AppWindowGeometryCache::kMaxCachedWindows + 1; ++i) {
     std::string window_id = "window_" + base::IntToString(i);
     EXPECT_TRUE(cache_->GetGeometry(extension_id, window_id, NULL, NULL, NULL));
   }
 }
 
 }  // namespace extensions