[NTP Snippets] Refactor background scheduling for remote suggestions

components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc

Index: components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc
diff --git a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8257fb3e0f2fca969091e27c8819bca12e893139
--- /dev/null
+++ b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc
@@ -0,0 +1,286 @@
+// 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.
+
+#include "components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h"
+
+#include <memory>
+#include <set>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/command_line.h"
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
+#include "components/ntp_snippets/features.h"
+#include "components/ntp_snippets/ntp_snippets_constants.h"
+#include "components/ntp_snippets/pref_names.h"
+#include "components/ntp_snippets/remote/persistent_scheduler.h"
+#include "components/ntp_snippets/remote/remote_suggestions_provider.h"
+#include "components/ntp_snippets/remote/test_utils.h"
+#include "components/ntp_snippets/user_classifier.h"
+#include "components/prefs/testing_pref_service.h"
+#include "components/variations/variations_params_manager.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::ElementsAre;
+using testing::Eq;
+using testing::InSequence;
+using testing::Invoke;
+using testing::IsEmpty;
+using testing::Mock;
+using testing::MockFunction;
+using testing::NiceMock;
+using testing::Not;
+using testing::SaveArg;
+using testing::SizeIs;
+using testing::StartsWith;
+using testing::WithArgs;
+using testing::_;
+
+namespace ntp_snippets {
+
+namespace {
+
+const char* kDifferentWiFiInterval = "2";
+
+class MockPersistentScheduler : public PersistentScheduler {
+ public:
+  MOCK_METHOD2(Schedule,
+               bool(base::TimeDelta period_wifi,
+                    base::TimeDelta period_fallback));
+  MOCK_METHOD0(Unschedule, bool());
+};
+
+class MockRemoteSuggestionsProvider : public RemoteSuggestionsProvider {
+ public:
+  MOCK_METHOD2(Schedule,
+               bool(base::TimeDelta period_wifi,
+                    base::TimeDelta period_fallback));
+  MOCK_METHOD0(Unschedule, bool());
+};
+
+}  // namespace
+
+class SchedulingRemoteSuggestionsProviderTest : public ::testing::Test {
+ public:
+  SchedulingRemoteSuggestionsProviderTest()
+      : user_classifier_(/*pref_service=*/nullptr) {
+    SchedulingRemoteSuggestionsProvider::RegisterProfilePrefs(
+        utils_.pref_service()->registry());
+  }
+
+  std::unique_ptr<SchedulingRemoteSuggestionsProvider> MakeProvider {
+    auto provider = base::MakeUnique<MockRemoteSuggestionsProvider>();
+    provider_ = provider_.get();
+
+    auto scheduling_provider =
+        base::MakeUnique<SchedulingRemoteSuggestionsProvider>(
+            /*observer=*/nullptr, /*category_factory=*/nullptr,
+            std::move(provider), mock_persistent_scheduler(), &user_classifier_,
+            utils_.pref_service());
+    return scheduler;
+  }
+
+ protected:
+  MockPersistentScheduler* mock_persistent_scheduler() {
+    return &persistent_scheduler_;
+  }
+
+ private:
+  test::RemoteSuggestionsTestUtils utils_;
+  UserClassifier user_classifier_;
+  NiceMock<MockPersistentScheduler> persistent_scheduler_;
+  NiceMock<MockUpdater> updater_;
+  MockRemoteSuggestionsProvider* provider_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsSchedulerTest);
+};
+
+
+
+TEST_F(RemoteSuggestionsProviderTest, DontRescheduleAfterFailedFetch) {
+  // We should get two |Schedule| calls: The first when initialization
+  // completes, the second one after the automatic (since the service doesn't
+  // have any data yet) fetch finishes.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _)).Times(2);
+  auto service = MakeSnippetsService();
+
+  // A failed fetch should NOT trigger another |Schedule|.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _)).Times(0);
+  LoadFromJSONString(service.get(), GetTestJson({GetInvalidSnippet()}));
+}
+
+TEST_F(RemoteSuggestionsProviderTest, RescheduleOnStateChange) {
+  {
+    InSequence s;
+    // Initial startup.
+    EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _)).Times(2);
+    // Service gets disabled.
+    EXPECT_CALL(mock_persistent_scheduler(), Unschedule());
+    // Service gets enabled again.
+    EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _)).Times(2);
+  }
+  auto service = MakeSnippetsService();
+  ASSERT_TRUE(service->ready());
+
+  service->OnStatusChanged(RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN,
+                           RemoteSuggestionsStatus::EXPLICITLY_DISABLED);
+  ASSERT_FALSE(service->ready());
+  base::RunLoop().RunUntilIdle();
+
+  service->OnStatusChanged(RemoteSuggestionsStatus::EXPLICITLY_DISABLED,
+                           RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT);
+  ASSERT_TRUE(service->ready());
+  base::RunLoop().RunUntilIdle();
+}
+
+

[tschumann, 2016/12/19 11:07:19]

drop consecutive blank lines.
same in line 106/107

[jkrcal, 2016/12/20 16:39:47]

Done.

+
+
+TEST_F(RemoteSuggestionsSchedulerTest, SchedulesOnStart) {
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  EXPECT_CALL(mock_persistent_scheduler(), Unschedule()).Times(0);
+
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, UnschedulesAfterBeingScheduled) {
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+
+  Mock::VerifyAndClearExpectations(mock_persistent_scheduler());
+  EXPECT_CALL(mock_persistent_scheduler(), Unschedule());
+  scheduler->Unschedule();
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, DoesNotUnscheduleOnShutdown) {
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  EXPECT_CALL(mock_persistent_scheduler(), Unschedule()).Times(0);
+
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+
+  // The scheduler will go out of scope and gets destroyed.
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, UnschedulesOnlyOnce) {
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+
+  // Unschedule for the first time.
+  Mock::VerifyAndClearExpectations(mock_persistent_scheduler());
+  EXPECT_CALL(mock_persistent_scheduler(), Unschedule());
+  scheduler->Unschedule();
+
+  // When unscheduling again, we should not get any |Schedule| calls:
+  // The tasks are already unscheduled, so no need to do it again.
+  Mock::VerifyAndClearExpectations(mock_persistent_scheduler());
+  EXPECT_CALL(mock_persistent_scheduler(), Unschedule()).Times(0);
+  scheduler->Unschedule();
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, SchedulesOnlyOnce) {
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+
+  // When scheduling again, we should not get any |Schedule| calls:
+  // The tasks are already scheduled with the correct intervals, so no need to
+  // do it again.
+  Mock::VerifyAndClearExpectations(mock_persistent_scheduler());
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _)).Times(0);
+  scheduler->Schedule();
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, ReschedulesWhenWifiParamChanges) {
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+
+  // UserClassifier defaults to UserClass::ACTIVE_NTP_USER if PrefService is
+  // null. Change the wifi interval for this class.
+  variations::testing::VariationParamsManager params_manager(
+      ntp_snippets::kStudyName,
+      {{"fetching_interval_hours-wifi-active_ntp_user",
+        kDifferentWiFiInterval}},
+      {kArticleSuggestionsFeature.name});
+
+  Mock::VerifyAndClearExpectations(mock_persistent_scheduler());
+  // After the change of a paramter, it should reschedule.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  scheduler->Schedule();
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, ReschedulesWhenFallbackParamChanges) {
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+
+  // UserClassifier defaults to UserClass::ACTIVE_NTP_USER if PrefService is
+  // null. Change the wifi interval for this class.
+  variations::testing::VariationParamsManager params_manager(
+      ntp_snippets::kStudyName,
+      {{"fetching_interval_hours-fallback-active_ntp_user",
+        kDifferentWiFiInterval}},
+      {kArticleSuggestionsFeature.name});
+
+  Mock::VerifyAndClearExpectations(mock_persistent_scheduler());
+  // After the change of a paramter, it should reschedule.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  scheduler->Schedule();
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, HandlesForcedReschedules) {
+  {
+    InSequence s;
+    // The initial scheduling.
+    EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+    // Forced rescheduling.
+    EXPECT_CALL(mock_persistent_scheduler(), Unschedule());
+    EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  }
+
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+  // Forced reschedule
+  scheduler->Unschedule();
+  scheduler->Schedule();
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, ReschedulesAfterSuccessfulUpdate) {
+  // The initial scheduling.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+  Mock::VerifyAndClearExpectations(mock_persistent_scheduler());
+
+  // Simulate a successful update.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  scheduler->PerformHardUpdate();
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, DoesNotRescheduleAfterFailedUpdate) {
+  // The initial scheduling.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+  Mock::VerifyAndClearExpectations(mock_persistent_scheduler());
+
+  // Simulate a succesfull update.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _)).Times(0);
+  scheduler->PerformHardUpdate();
+  // Failed -> do not call OnSuccessfulUpdate().
+}
+
+TEST_F(RemoteSuggestionsSchedulerTest, CallsUpdater) {
+  // The initial scheduling.
+  EXPECT_CALL(mock_persistent_scheduler(), Schedule(_, _));
+  auto scheduler = MakeScheduler(/*enabled=*/true);
+
+  // Simulate a succesfull update.
+  EXPECT_CALL(updater(), UpdateRemoteSuggestionsBySchedule());
+  scheduler->PerformHardUpdate();
+}
+
+}  // namespace ntp_snippets