[Remote suggestions] Do not fetch before EULA accepted

components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h

Index: components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h
diff --git a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h
index 32f1964134162f5453d195bbcda7f90b8d1fffba..477bdb49353449252b2d643c3de6981e20e4382d 100644
--- a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h
+++ b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h
@@ -18,6 +18,7 @@
 #include "components/ntp_snippets/remote/remote_suggestions_provider.h"
 #include "components/ntp_snippets/remote/remote_suggestions_scheduler.h"
 #include "components/ntp_snippets/remote/request_throttler.h"
+#include "components/web_resource/eula_accepted_notifier.h"
 
 class PrefRegistrySimple;
 class PrefService;
@@ -57,14 +58,16 @@ class UserClassifier;
 // Scheduler interface. crbug.com/695447
 class SchedulingRemoteSuggestionsProvider final
     : public RemoteSuggestionsProvider,
-      public RemoteSuggestionsScheduler {
+      public RemoteSuggestionsScheduler,
+      public web_resource::EulaAcceptedNotifier::Observer {

[tschumann, 2017/03/21 20:13:15]

as you know, I'm trying to avoid multiple inheritance.
In this case, we can actually quite nicely use aggregation as the observer is
using the RemoteSuggestionsScheduler's public interface.

So why not have a small class like EulaState with a constructor taking the
local_state_prefs and remote suggestions scheduler. EulaState would also
encapsulate the notifier (so no doubt if it's null or not)
The SchedulingRemoteSuggestionsProvider's constructor can instantiate build it
and simply ask that state for all info it needs. WDYT?

[jkrcal, 2017/03/22 09:36:04]

Done. WDYT?

[tschumann, 2017/03/22 09:47:01]

Thanks! The cyclic dependency is still a bit of a smell but I don't see and easy
way to improve this.

[jkrcal, 2017/03/22 16:48:01]

Acknowledged.

  public:
   SchedulingRemoteSuggestionsProvider(
-      Observer* observer,
+      ContentSuggestionsProvider::Observer* observer,
       std::unique_ptr<RemoteSuggestionsProvider> provider,
       PersistentScheduler* persistent_scheduler,
       const UserClassifier* user_classifier,
-      PrefService* pref_service,
+      PrefService* profile_prefs,
+      PrefService* local_state_prefs,
       std::unique_ptr<base::Clock> clock);
 
   ~SchedulingRemoteSuggestionsProvider() override;
@@ -109,6 +112,9 @@ class SchedulingRemoteSuggestionsProvider final
       const DismissedSuggestionsCallback& callback) override;
   void ClearDismissedSuggestionsForDebugging(Category category) override;
 
+  // EulaAcceptedNotifier::Observer implementation.
+  void OnEulaAccepted() override;
+
  private:
   // Abstract description of the fetching schedule.
   struct FetchingSchedule {
@@ -198,7 +204,10 @@ class SchedulingRemoteSuggestionsProvider final
   RequestThrottler request_throttler_active_ntp_user_;
   RequestThrottler request_throttler_active_suggestions_consumer_;
 
-  PrefService* pref_service_;
+  // We should not fetch in background before EULA gets accepted.
+  std::unique_ptr<web_resource::EulaAcceptedNotifier> eula_notifier_;
+
+  PrefService* profile_prefs_;
   std::unique_ptr<base::Clock> clock_;
   std::set<SchedulingRemoteSuggestionsProvider::TriggerType> enabled_triggers_;