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

components/ntp_snippets/ntp_snippets_service_unittest.cc

 // Copyright 2015 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/ntp_snippets_service.h"
 
 #include <memory>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 225 matching lines @@
  public:
   MOCK_METHOD0(NTPSnippetsServiceLoaded, void());
   MOCK_METHOD0(NTPSnippetsServiceShutdown, void());
   MOCK_METHOD0(NTPSnippetsServiceDisabled, void());
 };
 
 class WaitForDBLoad : public NTPSnippetsServiceObserver {
  public:
   WaitForDBLoad(NTPSnippetsService* service) : service_(service) {
     service_->AddObserver(this);
-    if (!service_->loaded())
+    if (!service_->ready())
       run_loop_.Run();
   }
 
   ~WaitForDBLoad() override {
     service_->RemoveObserver(this);
   }
 
  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();
   }
 
   void NTPSnippetsServiceShutdown() override {}
   void NTPSnippetsServiceDisabled() override {}
 
   NTPSnippetsService* service_;
   base::RunLoop run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(WaitForDBLoad);
@@ skipping 27 matching lines @@
       service_->Shutdown();
 
     // We need to run the message loop after deleting the database, because
     // ProtoDatabaseImpl deletes the actual LevelDB asynchronously on the task
     // runner. Without this, we'd get reports of memory leaks.
     service_.reset();
     base::RunLoop().RunUntilIdle();
   }
 
   void SetUp() override {
+    ResetSyncServiceMock();
     EXPECT_CALL(mock_scheduler(), Schedule(_, _, _, _)).Times(1);
     CreateSnippetsService(/*enabled=*/true);
   }
 
   void CreateSnippetsService(bool enabled) {
     if (service_)
       service_->Shutdown();
 
     scoped_refptr<base::SingleThreadTaskRunner> task_runner(
         base::ThreadTaskRunnerHandle::Get());
@@ skipping 70 matching lines @@
   std::unique_ptr<NTPSnippetsService> service_;
 
   base::ScopedTempDir database_dir_;
 
   DISALLOW_COPY_AND_ASSIGN(NTPSnippetsServiceTest);
 };
 
 class NTPSnippetsServiceWithSyncTest : public NTPSnippetsServiceTest {
  public:
   void SetUp() override {
-    ResetSyncServiceMock();
+    // ResetSyncServiceMock();
     NTPSnippetsServiceTest::SetUp();
   }
 };
 
 class NTPSnippetsServiceDisabledTest : public NTPSnippetsServiceTest {
  public:
   void SetUp() override {
     EXPECT_CALL(mock_scheduler(), Unschedule()).Times(1);
     CreateSnippetsService(/*enabled=*/false);
   }
 };
 
 TEST_F(NTPSnippetsServiceTest, ScheduleIfEnabled) {
   // SetUp() checks that Schedule is called.
 }
 
 TEST_F(NTPSnippetsServiceDisabledTest, Unschedule) {
   // SetUp() checks that Unschedule is called.
 }
 
 TEST_F(NTPSnippetsServiceTest, Full) {
+  EXPECT_TRUE(service()->ready());
   std::string json_str(GetTestJson({GetSnippet()}));
 
   LoadFromJSONString(json_str);
   ASSERT_THAT(service()->snippets(), SizeIs(1));
   const NTPSnippet& snippet = *service()->snippets().front();
 
   EXPECT_EQ(snippet.id(), kSnippetUrl);
   EXPECT_EQ(snippet.title(), kSnippetTitle);
   EXPECT_EQ(snippet.snippet(), kSnippetText);
   EXPECT_EQ(snippet.salient_image_url(), GURL(kSnippetSalientImage));
@@ skipping 412 matching lines @@
   EXPECT_THAT(tester.GetAllSamples("NewTabPage.Snippets.NumArticles"),
               ElementsAre(base::Bucket(/*min=*/0, /*count=*/3),
                           base::Bucket(/*min=*/1, /*count=*/2)));
   // Discarded snippets shouldn't influence NumArticlesFetched.
   EXPECT_THAT(tester.GetAllSamples("NewTabPage.Snippets.NumArticlesFetched"),
               ElementsAre(base::Bucket(/*min=*/0, /*count=*/1),
                           base::Bucket(/*min=*/1, /*count=*/3)));
   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);
   CreateSnippetsService(/*enabled=*/true);
   tester.ExpectTotalCount("NewTabPage.Snippets.NumArticlesFetched", 4);
 }
 
 TEST_F(NTPSnippetsServiceTest, DiscardShouldRespectAllKnownUrls) {
   const std::string creation =
       NTPSnippet::TimeToJsonString(GetDefaultCreationTime());
@@ skipping 18 matching lines @@
   EXPECT_THAT(service()->snippets(), IsEmpty());
 
   // The same article from the AOL domain should now be detected as discarded.
   LoadFromJSONString(GetTestJson({GetSnippetWithUrlAndTimesAndSources(
       source_urls[1], creation, expiry, source_urls, publishers, amp_urls)}));
   ASSERT_THAT(service()->snippets(), IsEmpty());
 }
 
 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.
   ON_CALL(*mock_sync_service(), ConfigurationDone())
       .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) {
   MockServiceObserver mock_observer;
   service()->AddObserver(&mock_observer);
 
   // Simulate user disabled sync.
   ON_CALL(*mock_sync_service(), CanSyncStart()).WillByDefault(Return(false));
   // The service should notify observers it's been disabled and clear the
   // snippets instead of pulling new ones.
   EXPECT_CALL(mock_observer, NTPSnippetsServiceDisabled());
   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);
 }
 
 }  // namespace ntp_snippets