Topsites become keyedService based.

chrome/browser/history/top_sites_factory.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.

[sdefresne, 2015/01/08 10:41:01]

nit: you'll only land this is 2015, so please use 2015 in new files.

[Jitu( very slow this week), 2015/01/12 11:30:07]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/history/top_sites_factory.h"
+
+#include "chrome/browser/history/top_sites_impl.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_constants.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+
+using namespace history;

[sdefresne, 2015/01/08 10:41:01]

nit: we prefer not to use "using namespace" unless it improves the clarity. I
don't think it is required here (especially since you mostly use fully qualified
types history::TopSites, ...).

[Bernhard Bauer, 2015/01/08 10:45:14]

Good catch! The Google C++ style guide is actually even stricter about it:

"You may not use a using-directive to make all names from a namespace
available."
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Namespaces).

[Jitu( very slow this week), 2015/01/12 11:30:07]

Done.

+
+// static
+scoped_refptr<history::TopSites> TopSitesFactory::GetForProfile(
+    Profile* profile) {
+  return static_cast<history::TopSites*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true).get());
+}
+
+// static
+scoped_refptr<history::TopSites> TopSitesFactory::GetForProfileIfExists(
+    Profile* profile) {
+  return static_cast<history::TopSites*>(
+      GetInstance()->GetServiceForBrowserContext(profile, false).get());
+}
+
+// static
+TopSitesFactory* TopSitesFactory::GetInstance() {
+  return Singleton<TopSitesFactory>::get();
+}
+
+scoped_refptr<RefcountedKeyedService> TopSitesFactory::BuildServiceInstanceFor(
+    content::BrowserContext* profile) const {

[sdefresne, 2015/01/08 10:41:01]

nit: s/profile/context/

[Jitu( very slow this week), 2015/01/12 11:30:07]

Done.

+  TopSitesImpl* top_sites = new TopSitesImpl(static_cast<Profile*>(profile));
+  top_sites->Init(profile->GetPath().Append(chrome::kTopSitesFilename));
+  return make_scoped_refptr<history::TopSites>(top_sites);

[Bernhard Bauer, 2015/01/08 10:22:05]

Does a simple make_scoped_refptr() work? If it doesn't, you can just call the
scoped_refptr constructor instead of the make_scoped_refptr method, as that is
shorter.

[Jitu( very slow this week), 2015/01/12 11:30:07]

I think
return make_scoped_refptr(top_sites) work here.

Are you suggesting to do change like this

scoped_refptr<TopSitesImpl> top_sites =  make_scoped_refptr(new
TopSitesImpl(static_cast<Profile*>(profile));

[Bernhard Bauer, 2015/01/12 14:32:23]

That would be a possible alternative, but what you did is exactly what I was
refering to.

+}
+
+TopSitesFactory::TopSitesFactory()
+    : RefcountedBrowserContextKeyedServiceFactory(
+          "TopSitesService",

[sdefresne, 2015/01/08 10:41:01]

nit: the value passed there is usually the name of the returned class, thus use
"TopSites" here.

[Jitu( very slow this week), 2015/01/12 11:30:07]

Done.

[Jitu( very slow this week), 2015/01/12 11:30:07]

Done.

+          BrowserContextDependencyManager::GetInstance()) {
+}
+
+TopSitesFactory::~TopSitesFactory() {
+}