Add ContentSuggestionsService

chrome/browser/ntp_snippets/content_suggestions_service_factory.h

Index: chrome/browser/ntp_snippets/content_suggestions_service_factory.h
diff --git a/chrome/browser/ntp_snippets/content_suggestions_service_factory.h b/chrome/browser/ntp_snippets/content_suggestions_service_factory.h
new file mode 100644
index 0000000000000000000000000000000000000000..e09f157f7e513c0d1b13a7b6df56c3e250cd1f56
--- /dev/null
+++ b/chrome/browser/ntp_snippets/content_suggestions_service_factory.h
@@ -0,0 +1,44 @@
+// Copyright 2016 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 CHROME_BROWSER_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_FACTORY_H_
+#define CHROME_BROWSER_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_FACTORY_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+class Profile;
+
+namespace base {
+template <typename T>
+struct DefaultSingletonTraits;

[tschumann, 2016/06/28 11:47:17]

never forward declare templates. It makes them pretty much impossible to change
(think like someone wants to add an optional parameter).

[Marc Treib, 2016/06/28 12:05:19]

I generally agree, though this particular thing is very common with these
factory classes. That said, I'm also fine with just including the thing.

[tschumann, 2016/06/28 12:32:40]

I'm not feeling strongly, but leaning towards not forward-declaring. Primarily
because there might be less common types where we probably shouldn't forward
declare them and cases should be rare in general.
I'm also happy with an exception of commonly used templates. Your call ;-)

[Philipp Keck, 2016/06/28 14:18:57]

The existing code is not consistent here (compare NTPSnippetsServiceFactory to
SuggestionsServiceFactory). I'm just going to leave it as is, please let me know
if you want me to change it.

+}  // namespace base
+
+namespace ntp_snippets {
+class ContentSuggestionsService;

[tschumann, 2016/06/28 11:47:17]

I don't think this forward declaration is necessary. the only time when I
encountered files you don't want to include in headers were pretty fat proto
files (but proto.h also helps with this issue now).

Mark: Is there some general practice on Chrome around forward declarations?

[Marc Treib, 2016/06/28 12:05:19]

Yes. As per https://www.chromium.org/developers/coding-style:
"Unlike the Google style guide, Chromium style prefers forward declarations to
#includes where possible."

[tschumann, 2016/06/28 12:32:40]

I see. Well it's probably because you compile a lot locally -- header includes
are reasonably cheap when compiling on forge.
Thanks for the explanation -- this clarifies a lot, including why "empty" in
headers is considered bad!

[Philipp Keck, 2016/06/28 14:18:57]

Acknowledged.

+}  // namespace ntp_snippets
+
+class ContentSuggestionsServiceFactory
+    : public BrowserContextKeyedServiceFactory {
+ public:
+  static ContentSuggestionsServiceFactory* GetInstance();
+  static ntp_snippets::ContentSuggestionsService* GetForProfile(
+      Profile* profile);
+
+ private:
+  friend struct base::DefaultSingletonTraits<ContentSuggestionsServiceFactory>;

[tschumann, 2016/06/28 11:47:17]

nit: friends are usually a pretty deep implementation detail, why I usually move
them to the bottom right before the DISALLOW_XXX macro.

AFAIK, there's no rule about where to put them though.

[Marc Treib, 2016/06/28 12:05:19]

This is the common style in Chrome AFAIK.
There *is* a rule that DISALLOW_COPY_AND_ASSIGN goes last.

[tschumann, 2016/06/28 12:32:40]

Ok, cool. If there's a common style, we should just apply it everywhere (as this
is a pretty minor detail).

[Philipp Keck, 2016/06/28 14:18:57]

Acknowledged, see above.

+
+  ContentSuggestionsServiceFactory();
+  ~ContentSuggestionsServiceFactory() override;
+
+  // BrowserStateKeyedServiceFactory implementation.
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+
+  DISALLOW_COPY_AND_ASSIGN(ContentSuggestionsServiceFactory);
+};
+
+#endif  // CHROME_BROWSER_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_FACTORY_H_