[Remote suggestions] Refactor the scheduler

components/ntp_snippets/content_suggestions_service.h

 // 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 COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_H_
 #define COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_H_
 
 #include <map>
 #include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/observer_list.h"
 #include "base/optional.h"
 #include "base/scoped_observer.h"
 #include "base/time/time.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/history/core/browser/history_service_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/ntp_snippets/callbacks.h"
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/category_rankers/category_ranker.h"
 #include "components/ntp_snippets/category_status.h"
 #include "components/ntp_snippets/content_suggestions_provider.h"
+#include "components/ntp_snippets/remote/remote_suggestions_scheduler.h"
 #include "components/ntp_snippets/user_classifier.h"
 #include "components/signin/core/browser/signin_manager.h"
 
 class PrefService;
 class PrefRegistrySimple;
 
 namespace ntp_snippets {
 
 class RemoteSuggestionsProvider;
-class RemoteSuggestionsScheduler;
 
 // Retrieves suggestions from a number of ContentSuggestionsProviders and serves
 // them grouped into categories. There can be at most one provider per category.
 class ContentSuggestionsService : public KeyedService,
                                   public ContentSuggestionsProvider::Observer,
                                   public SigninManagerBase::Observer,
                                   public history::HistoryServiceObserver {
  public:
   class Observer {
    public:
@@ skipping 32 matching lines @@
 
    protected:
     virtual ~Observer() = default;
   };
 
   enum State {
     ENABLED,
     DISABLED,
   };
 
-  ContentSuggestionsService(State state,
-                            SigninManagerBase* signin_manager,
-                            history::HistoryService* history_service,
-                            PrefService* pref_service,
-                            std::unique_ptr<CategoryRanker> category_ranker);
+  ContentSuggestionsService(
+      State state,
+      SigninManagerBase* signin_manager,         // Can be nullptr in unittests.
+      history::HistoryService* history_service,  // Can be nullptr in unittests.
+      PrefService* pref_service,
+      std::unique_ptr<CategoryRanker> category_ranker,
+      std::unique_ptr<UserClassifier>
+          user_classifier,  // Can be nullptr in unittests.

[Marc Treib, 2017/03/27 14:27:21]

Would "Can be null in tests." make it fit on the previous line? ;)

[jkrcal, 2017/03/29 10:55:57]

Removed, anyway.

+      std::unique_ptr<RemoteSuggestionsScheduler>
+          remote_suggestions_scheduler  // Can be nullptr in unittests.
+      );
   ~ContentSuggestionsService() override;
 
   // Inherited from KeyedService.
   void Shutdown() override;
 
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   State state() { return state_; }
 
   // Gets all categories for which a provider is registered. The categories may
@@ skipping 110 matching lines @@
   // TODO(jkrcal) Move the getter into the scheduler interface (the setter is
   // then not needed any more). crbug.com/695447
   void set_remote_suggestions_provider(
       RemoteSuggestionsProvider* remote_suggestions_provider) {
     remote_suggestions_provider_ = remote_suggestions_provider;
   }
   RemoteSuggestionsProvider* remote_suggestions_provider_for_debugging() {
     return remote_suggestions_provider_;
   }
 
-  // The reference to RemoteSuggestionsScheduler should only be set by the
-  // factory. The interface is suited for informing about external events that
-  // have influence on scheduling remote fetches.
-  void set_remote_suggestions_scheduler(
-      ntp_snippets::RemoteSuggestionsScheduler* remote_suggestions_scheduler) {
-    remote_suggestions_scheduler_ = remote_suggestions_scheduler;
-  }
+  // The interface is suited for informing about external events that have
+  // influence on scheduling remote fetches. Can be nullptr in tests.
   RemoteSuggestionsScheduler* remote_suggestions_scheduler() {
-    return remote_suggestions_scheduler_;
+    return remote_suggestions_scheduler_.get();
   }
 
-  UserClassifier* user_classifier() { return &user_classifier_; }
+  // Can be nullptr in tests.
+  // TODO(jkrcal): The getter is only used from the bridge and from
+  // snippets-internals. Can we get rid of it with the metrics refactoring?
+  UserClassifier* user_classifier() { return user_classifier_.get(); }
+
   CategoryRanker* category_ranker() { return category_ranker_.get(); }
 
  private:
   friend class ContentSuggestionsServiceTest;
 
   // Implementation of ContentSuggestionsProvider::Observer.
   void OnNewSuggestions(ContentSuggestionsProvider* provider,
                         Category category,
                         std::vector<ContentSuggestion> suggestions) override;
   void OnCategoryStatusChanged(ContentSuggestionsProvider* provider,
@@ skipping 87 matching lines @@
 
   base::ObserverList<Observer> observers_;
 
   const std::vector<ContentSuggestion> no_suggestions_;
 
   // Keep a direct reference to this special provider to redirect debugging
   // calls to it. If the RemoteSuggestionsProvider is loaded, it is also present
   // in |providers_|, otherwise this is a nullptr.
   RemoteSuggestionsProvider* remote_suggestions_provider_;
 
-  // Interface for informing about external events that have influence on
-  // scheduling remote fetches. Not owned.
-  RemoteSuggestionsScheduler* remote_suggestions_scheduler_;
-
   PrefService* pref_service_;
 
-  UserClassifier user_classifier_;
+  // Interface for informing about external events that have influence on
+  // scheduling remote fetches.
+  std::unique_ptr<RemoteSuggestionsScheduler> remote_suggestions_scheduler_;
+
+  // Classifies the user on the basis of long-term user interactions.
+  std::unique_ptr<UserClassifier> user_classifier_;
 
   // Provides order for categories.
   std::unique_ptr<CategoryRanker> category_ranker_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentSuggestionsService);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_H_