[NTP Snippets] Always show the bookmarks section, but at the end if it's empty

components/ntp_snippets/content_suggestions_service_unittest.cc

 // 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/content_suggestions_service.h"
 
 #include <memory>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 160 matching lines @@
   providers() {
     return service()->providers_by_category_;
   }
 
   CategoryFactory* category_factory() { return service()->category_factory(); }
 
   Category FromKnownCategory(KnownCategories known_category) {
     return service()->category_factory()->FromKnownCategory(known_category);
   }
 
+  Category FromRemoteCategory(int remote_category) {
+    return service()->category_factory()->FromRemoteCategory(remote_category);
+  }
+
   MockProvider* RegisterProvider(Category provided_category) {
     return RegisterProvider(std::vector<Category>({provided_category}));
   }
 
   MockProvider* RegisterProvider(
       const std::vector<Category>& provided_categories) {
     std::unique_ptr<MockProvider> provider = base::MakeUnique<MockProvider>(
         service(), category_factory(), provided_categories);
     MockProvider* result = provider.get();
     service()->RegisterProvider(std::move(provider));
@@ skipping 24 matching lines @@
   void SetUp() override {
     CreateContentSuggestionsService(ContentSuggestionsService::State::DISABLED);
   }
 };
 
 TEST_F(ContentSuggestionsServiceTest, ShouldRegisterProviders) {
   EXPECT_THAT(service()->state(),
               Eq(ContentSuggestionsService::State::ENABLED));
   Category articles_category = FromKnownCategory(KnownCategories::ARTICLES);
   Category offline_pages_category =
-      FromKnownCategory(KnownCategories::BOOKMARKS);
+      FromKnownCategory(KnownCategories::DOWNLOADS);
   ASSERT_THAT(providers(), IsEmpty());
   EXPECT_THAT(service()->GetCategories(), IsEmpty());
   EXPECT_THAT(service()->GetCategoryStatus(articles_category),
               Eq(CategoryStatus::NOT_PROVIDED));
   EXPECT_THAT(service()->GetCategoryStatus(offline_pages_category),
               Eq(CategoryStatus::NOT_PROVIDED));
 
   MockProvider* provider1 = RegisterProvider(articles_category);
   provider1->FireCategoryStatusChangedWithCurrentStatus(articles_category);
   EXPECT_THAT(providers().count(offline_pages_category), Eq(0ul));
@@ skipping 17 matching lines @@
               ElementsAre(offline_pages_category, articles_category));
   EXPECT_THAT(service()->GetCategoryStatus(articles_category),
               Eq(CategoryStatus::AVAILABLE));
   EXPECT_THAT(service()->GetCategoryStatus(offline_pages_category),
               Eq(CategoryStatus::AVAILABLE));
 }
 
 TEST_F(ContentSuggestionsServiceDisabledTest, ShouldDoNothingWhenDisabled) {
   Category articles_category = FromKnownCategory(KnownCategories::ARTICLES);
   Category offline_pages_category =
-      FromKnownCategory(KnownCategories::BOOKMARKS);
+      FromKnownCategory(KnownCategories::DOWNLOADS);
   EXPECT_THAT(service()->state(),
               Eq(ContentSuggestionsService::State::DISABLED));
   EXPECT_THAT(providers(), IsEmpty());
   EXPECT_THAT(service()->GetCategoryStatus(articles_category),
               Eq(CategoryStatus::ALL_SUGGESTIONS_EXPLICITLY_DISABLED));
   EXPECT_THAT(service()->GetCategoryStatus(offline_pages_category),
               Eq(CategoryStatus::ALL_SUGGESTIONS_EXPLICITLY_DISABLED));
   EXPECT_THAT(service()->GetCategories(), IsEmpty());
   EXPECT_THAT(service()->GetSuggestionsForCategory(articles_category),
               IsEmpty());
 }
 
 TEST_F(ContentSuggestionsServiceTest, ShouldRedirectFetchSuggestionImage) {
   Category articles_category = FromKnownCategory(KnownCategories::ARTICLES);
   Category offline_pages_category =
-      FromKnownCategory(KnownCategories::BOOKMARKS);
+      FromKnownCategory(KnownCategories::DOWNLOADS);
   MockProvider* provider1 = RegisterProvider(articles_category);
   MockProvider* provider2 = RegisterProvider(offline_pages_category);
 
   provider1->FireSuggestionsChanged(articles_category, {1});
   std::string suggestion_id = CreateSuggestion(1).id();
 
   EXPECT_CALL(*provider1, FetchSuggestionImage(suggestion_id, _));
   EXPECT_CALL(*provider2, FetchSuggestionImage(_, _)).Times(0);
   service()->FetchSuggestionImage(
       suggestion_id, base::Bind(&ContentSuggestionsServiceTest::OnImageFetched,
@@ skipping 12 matching lines @@
       .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   service()->FetchSuggestionImage(
       suggestion_id, base::Bind(&ContentSuggestionsServiceTest::OnImageFetched,
                                 base::Unretained(this)));
   run_loop.Run();
 }
 
 TEST_F(ContentSuggestionsServiceTest, ShouldRedirectDismissSuggestion) {
   Category articles_category = FromKnownCategory(KnownCategories::ARTICLES);
   Category offline_pages_category =
-      FromKnownCategory(KnownCategories::BOOKMARKS);
+      FromKnownCategory(KnownCategories::DOWNLOADS);
   MockProvider* provider1 = RegisterProvider(articles_category);
   MockProvider* provider2 = RegisterProvider(offline_pages_category);
 
   provider2->FireSuggestionsChanged(offline_pages_category, {11});
   std::string suggestion_id = CreateSuggestion(11).id();
 
   EXPECT_CALL(*provider1, DismissSuggestion(_)).Times(0);
   EXPECT_CALL(*provider2, DismissSuggestion(suggestion_id));
   service()->DismissSuggestion(suggestion_id);
 }
@@ skipping 23 matching lines @@
   provider->FireSuggestionInvalidated(articles_category, unknown_id);
   ExpectThatSuggestionsAre(articles_category, {11, 13});
   Mock::VerifyAndClearExpectations(&observer);
 
   service()->RemoveObserver(&observer);
 }
 
 TEST_F(ContentSuggestionsServiceTest, ShouldForwardSuggestions) {
   Category articles_category = FromKnownCategory(KnownCategories::ARTICLES);
   Category offline_pages_category =
-      FromKnownCategory(KnownCategories::BOOKMARKS);
+      FromKnownCategory(KnownCategories::DOWNLOADS);
 
   // Create and register providers
   MockProvider* provider1 = RegisterProvider(articles_category);
   provider1->FireCategoryStatusChangedWithCurrentStatus(articles_category);
   MockProvider* provider2 = RegisterProvider(offline_pages_category);
   provider2->FireCategoryStatusChangedWithCurrentStatus(offline_pages_category);
   ASSERT_THAT(providers().count(articles_category), Eq(1ul));
   EXPECT_THAT(providers().at(articles_category), Eq(provider1));
   ASSERT_THAT(providers().count(offline_pages_category), Eq(1ul));
   EXPECT_THAT(providers().at(offline_pages_category), Eq(provider2));
@@ skipping 45 matching lines @@
 
   // Shutdown the service
   EXPECT_CALL(observer, ContentSuggestionsServiceShutdown());
   service()->Shutdown();
   service()->RemoveObserver(&observer);
   // The service will receive two Shutdown() calls.
 }
 
 TEST_F(ContentSuggestionsServiceTest,
        ShouldNotReturnCategoryInfoForNonexistentCategory) {
-  Category category = FromKnownCategory(KnownCategories::BOOKMARKS);
+  Category category = FromKnownCategory(KnownCategories::DOWNLOADS);
   base::Optional<CategoryInfo> result = service()->GetCategoryInfo(category);
   EXPECT_FALSE(result.has_value());
 }
 
 TEST_F(ContentSuggestionsServiceTest, ShouldReturnCategoryInfo) {
-  Category category = FromKnownCategory(KnownCategories::BOOKMARKS);
+  Category category = FromKnownCategory(KnownCategories::DOWNLOADS);
   MockProvider* provider = RegisterProvider(category);
   provider->FireCategoryStatusChangedWithCurrentStatus(category);
   base::Optional<CategoryInfo> result = service()->GetCategoryInfo(category);
   ASSERT_TRUE(result.has_value());
   CategoryInfo expected = provider->GetCategoryInfo(category);
   const CategoryInfo& actual = result.value();
   EXPECT_THAT(expected.title(), Eq(actual.title()));
   EXPECT_THAT(expected.card_layout(), Eq(actual.card_layout()));
   EXPECT_THAT(expected.has_more_button(), Eq(actual.has_more_button()));
 }
 
 TEST_F(ContentSuggestionsServiceTest,
        ShouldRegisterNewCategoryOnNewSuggestions) {
-  Category category = FromKnownCategory(KnownCategories::BOOKMARKS);
+  Category category = FromKnownCategory(KnownCategories::DOWNLOADS);
   MockProvider* provider = RegisterProvider(category);
   provider->FireCategoryStatusChangedWithCurrentStatus(category);
   MockServiceObserver observer;
   service()->AddObserver(&observer);
 
   // Provider starts providing |new_category| without calling
   // |OnCategoryStatusChanged|. This is supported for now until further
   // reconsideration.
   Category new_category = FromKnownCategory(KnownCategories::ARTICLES);
   provider->SetProvidedCategories(
@@ skipping 12 matching lines @@
   ASSERT_THAT(providers().count(new_category), Eq(1ul));
   EXPECT_THAT(providers().at(new_category), Eq(provider));
   EXPECT_THAT(service()->GetCategoryStatus(new_category),
               Eq(CategoryStatus::AVAILABLE));
 
   service()->RemoveObserver(&observer);
 }
 
 TEST_F(ContentSuggestionsServiceTest,
        ShouldRegisterNewCategoryOnCategoryStatusChanged) {
-  Category category = FromKnownCategory(KnownCategories::BOOKMARKS);
+  Category category = FromKnownCategory(KnownCategories::DOWNLOADS);
   MockProvider* provider = RegisterProvider(category);
   provider->FireCategoryStatusChangedWithCurrentStatus(category);
   MockServiceObserver observer;
   service()->AddObserver(&observer);
 
   // Provider starts providing |new_category| and calls
   // |OnCategoryStatusChanged|, but the category is not yet available.
   Category new_category = FromKnownCategory(KnownCategories::ARTICLES);
   provider->SetProvidedCategories(
       std::vector<Category>({category, new_category}));
   EXPECT_CALL(observer, OnCategoryStatusChanged(new_category,
                                                 CategoryStatus::INITIALIZING));
   provider->FireCategoryStatusChanged(new_category,
                                       CategoryStatus::INITIALIZING);
 
   ASSERT_THAT(providers().count(new_category), Eq(1ul));
   EXPECT_THAT(providers().at(new_category), Eq(provider));
   ExpectThatSuggestionsAre(new_category, std::vector<int>());
   EXPECT_THAT(service()->GetCategoryStatus(new_category),
               Eq(CategoryStatus::INITIALIZING));
   EXPECT_THAT(service()->GetCategories(),
               Eq(std::vector<Category>({category, new_category})));
 
   service()->RemoveObserver(&observer);
 }
 
 TEST_F(ContentSuggestionsServiceTest, ShouldRemoveCategoryWhenNotProvided) {
-  Category category = FromKnownCategory(KnownCategories::BOOKMARKS);
+  Category category = FromKnownCategory(KnownCategories::DOWNLOADS);
   MockProvider* provider = RegisterProvider(category);
   MockServiceObserver observer;
   service()->AddObserver(&observer);
 
   provider->FireSuggestionsChanged(category, {1, 2});
   ExpectThatSuggestionsAre(category, {1, 2});
 
   EXPECT_CALL(observer,
               OnCategoryStatusChanged(category, CategoryStatus::NOT_PROVIDED));
   provider->FireCategoryStatusChanged(category, CategoryStatus::NOT_PROVIDED);
 
   EXPECT_THAT(service()->GetCategoryStatus(category),
               Eq(CategoryStatus::NOT_PROVIDED));
   EXPECT_TRUE(service()->GetCategories().empty());
   ExpectThatSuggestionsAre(category, std::vector<int>());
 
   service()->RemoveObserver(&observer);
 }
 
+// This tests the temporary special-casing of the bookmarks section: If it is
+// empty, it should appear at the end; see crbug.com/640568.
+TEST_F(ContentSuggestionsServiceTest, ShouldPutBookmarksAtEndIfEmpty) {
+  // Register a bookmarks provider and an arbitrary remote provider.
+  Category bookmarks = FromKnownCategory(KnownCategories::BOOKMARKS);
+  MockProvider* bookmarks_provider = RegisterProvider(bookmarks);
+  bookmarks_provider->FireCategoryStatusChangedWithCurrentStatus(bookmarks);
+  Category remote = FromRemoteCategory(123);
+  MockProvider* remote_provider = RegisterProvider(remote);
+  remote_provider->FireCategoryStatusChangedWithCurrentStatus(remote);
+
+  // By default, the bookmarks category is empty, so it should be at the end.
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(remote, bookmarks));
+
+  // Add two bookmark suggestions; now bookmarks should be in the front.
+  bookmarks_provider->FireSuggestionsChanged(bookmarks, {1, 2});
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(bookmarks, remote));
+  // Dismiss the first suggestion; bookmarks should stay in the front.
+  service()->DismissSuggestion(CreateSuggestion(1).id());
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(bookmarks, remote));
+  // Dismiss the second suggestion; now bookmarks should go back to the end.
+  service()->DismissSuggestion(CreateSuggestion(2).id());
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(remote, bookmarks));
+
+  // Same thing, but invalidate instead of dismissing.
+  bookmarks_provider->FireSuggestionsChanged(bookmarks, {1, 2});
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(bookmarks, remote));
+  bookmarks_provider->FireSuggestionInvalidated(bookmarks,
+                                                CreateSuggestion(1).id());
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(bookmarks, remote));
+  bookmarks_provider->FireSuggestionInvalidated(bookmarks,
+                                                CreateSuggestion(2).id());
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(remote, bookmarks));
+
+  // Same thing, but now the bookmarks category updates "naturally".
+  bookmarks_provider->FireSuggestionsChanged(bookmarks, {1, 2});
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(bookmarks, remote));
+  bookmarks_provider->FireSuggestionsChanged(bookmarks, {1});
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(bookmarks, remote));
+  bookmarks_provider->FireSuggestionsChanged(bookmarks, std::vector<int>());
+  EXPECT_THAT(service()->GetCategories(), ElementsAre(remote, bookmarks));
+}
+
 }  // namespace ntp_snippets