[NTP Snippets] Unschedule fetches when the service should be disabled

components/ntp_snippets/ntp_snippets_service_unittest.cc

Index: components/ntp_snippets/ntp_snippets_service_unittest.cc
diff --git a/components/ntp_snippets/ntp_snippets_service_unittest.cc b/components/ntp_snippets/ntp_snippets_service_unittest.cc
index e1b0b7c69d5fabf6fea9b189acd9ed738db389b5..1dbd26c8d35b1caca70f9d19c7366756589c71e8 100644
--- a/components/ntp_snippets/ntp_snippets_service_unittest.cc
+++ b/components/ntp_snippets/ntp_snippets_service_unittest.cc
@@ -243,7 +243,7 @@ class WaitForDBLoad : public NTPSnippetsServiceObserver {
  public:
   WaitForDBLoad(NTPSnippetsService* service) : service_(service) {
     service_->AddObserver(this);
-    if (!service_->loaded())
+    if (!service_->ready())
       run_loop_.Run();
   }
 
@@ -253,7 +253,6 @@ class WaitForDBLoad : public NTPSnippetsServiceObserver {
 
  private:
   void NTPSnippetsServiceLoaded() override {
-    EXPECT_TRUE(service_->loaded());

[Marc Treib, 2016/06/06 08:38:33]

Any reason for removing this line?

[dgn, 2016/06/06 15:44:39]

That was related to the order between the db loading and the state change.
Changed it and reverted here.

[Marc Treib, 2016/06/07 08:55:39]

Acknowledged.

     run_loop_.Quit();
   }
 
@@ -301,6 +300,7 @@ class NTPSnippetsServiceTest : public testing::Test {
   }
 
   void SetUp() override {
+    ResetSyncServiceMock();
     EXPECT_CALL(mock_scheduler(), Schedule(_, _, _, _)).Times(1);
     CreateSnippetsService(/*enabled=*/true);
   }
@@ -391,7 +391,7 @@ class NTPSnippetsServiceTest : public testing::Test {
 class NTPSnippetsServiceWithSyncTest : public NTPSnippetsServiceTest {
  public:
   void SetUp() override {
-    ResetSyncServiceMock();
+    // ResetSyncServiceMock();
     NTPSnippetsServiceTest::SetUp();
   }
 };
@@ -413,6 +413,7 @@ TEST_F(NTPSnippetsServiceDisabledTest, Unschedule) {
 }
 
 TEST_F(NTPSnippetsServiceTest, Full) {
+  EXPECT_TRUE(service()->ready());
   std::string json_str(GetTestJson({GetSnippet()}));
 
   LoadFromJSONString(json_str);
@@ -845,6 +846,7 @@ TEST_F(NTPSnippetsServiceTest, LogNumArticlesHistogram) {
   EXPECT_THAT(
       tester.GetAllSamples("NewTabPage.Snippets.NumArticlesZeroDueToDiscarded"),
       ElementsAre(base::Bucket(/*min=*/1, /*count=*/1)));
+
   // Recreating the service and loading from prefs shouldn't count as fetched
   // articles.
   EXPECT_CALL(mock_scheduler(), Schedule(_, _, _, _)).Times(1);
@@ -883,12 +885,13 @@ TEST_F(NTPSnippetsServiceTest, DiscardShouldRespectAllKnownUrls) {
 
 TEST_F(NTPSnippetsServiceWithSyncTest, SyncStateCompatibility) {
   // The default test setup has a compatible sync state.
-  EXPECT_FALSE(service()->IsSyncStateIncompatible());
+  EXPECT_EQ(DisabledReason::NONE, service()->GetDisabledReason());
 
   // History sync disabled.
   ON_CALL(*mock_sync_service(), GetActiveDataTypes())
       .WillByDefault(Return(syncer::ModelTypeSet()));
-  EXPECT_TRUE(service()->IsSyncStateIncompatible());
+  EXPECT_EQ(DisabledReason::HISTORY_SYNC_DISABLED,
+            service()->GetDisabledReason());
   ResetSyncServiceMock();
 
   // Not done loading.
@@ -896,17 +899,20 @@ TEST_F(NTPSnippetsServiceWithSyncTest, SyncStateCompatibility) {
       .WillByDefault(Return(false));
   ON_CALL(*mock_sync_service(), GetActiveDataTypes())
       .WillByDefault(Return(syncer::ModelTypeSet()));
-  EXPECT_FALSE(service()->IsSyncStateIncompatible());
+  EXPECT_EQ(DisabledReason::HISTORY_SYNC_STATE_UNKNOWN,
+            service()->GetDisabledReason());
   ResetSyncServiceMock();
 
   // Sync disabled.
   ON_CALL(*mock_sync_service(), CanSyncStart()).WillByDefault(Return(false));
-  EXPECT_TRUE(service()->IsSyncStateIncompatible());
+  EXPECT_EQ(DisabledReason::HISTORY_SYNC_DISABLED,
+            service()->GetDisabledReason());
   ResetSyncServiceMock();
 
   // No service.
   service()->sync_service_ = nullptr;
-  EXPECT_TRUE(service()->IsSyncStateIncompatible());
+  EXPECT_EQ(DisabledReason::HISTORY_SYNC_DISABLED,
+            service()->GetDisabledReason());
 }
 
 TEST_F(NTPSnippetsServiceWithSyncTest, HistorySyncStateChanges) {
@@ -921,14 +927,17 @@ TEST_F(NTPSnippetsServiceWithSyncTest, HistorySyncStateChanges) {
   SetUpFetchResponse(GetTestJson({GetSnippet()}));
   service()->OnStateChanged();
   base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(NTPSnippetsService::State::DISABLED, service()->state_);
   EXPECT_THAT(service()->snippets(), IsEmpty()); // No fetch should be made.
 
   // Simulate user sign in.
   ResetSyncServiceMock();
   // The service should be ready again and load snippets.
+  EXPECT_CALL(mock_scheduler(), Schedule(_, _, _, _)).Times(1);
   SetUpFetchResponse(GetTestJson({GetSnippet()}));
   service()->OnStateChanged();
   base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(NTPSnippetsService::State::READY, service()->state_);
   EXPECT_FALSE(service()->snippets().empty());
 
   service()->RemoveObserver(&mock_observer);