Add a unit test for the BookmarkSuggestionsProvider.

This is primarily to verify behavior fixed in https://codereview.chromium.org/2616633002/ but also adds tests for more fundamental behavior of the BookmarkSuggestionsProvider.

This CL also moves ContentSuggestionsProvider test doubles out of specific unit tests and into separate targets so that we can reuse them.

BUG=674178

Review-Url: https://codereview.chromium.org/2618243004
Cr-Commit-Position: refs/heads/master@{#443260}

components/ntp_snippets/BUILD.gn

@@ -129,6 +129,7 @@ source_set("unit_tests") {
   testonly = true
   sources = [
     "bookmarks/bookmark_last_visit_utils_unittest.cc",
+    "bookmarks/bookmark_suggestions_provider_unittest.cc",
     "category_rankers/click_based_category_ranker_unittest.cc",
     "category_rankers/constant_category_ranker_unittest.cc",
     "category_unittest.cc",
@@ -184,6 +185,10 @@ source_set("test_support") {
     "category_rankers/fake_category_ranker.h",
     "category_rankers/mock_category_ranker.cc",
     "category_rankers/mock_category_ranker.h",
+    "fake_content_suggestions_provider_observer.cc",
+    "fake_content_suggestions_provider_observer.h",
+    "mock_content_suggestions_provider.cc",
+    "mock_content_suggestions_provider.h",
     "mock_content_suggestions_provider_observer.cc",
     "mock_content_suggestions_provider_observer.h",
     "offline_pages/offline_pages_test_utils.cc",
@@ -196,5 +201,6 @@ source_set("test_support") {
     "//components/offline_pages/core",
     "//components/offline_pages/core:test_support",
     "//testing/gmock",
+    "//testing/gtest",
   ]
 }

components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc

@@ -305,7 +305,6 @@ void BookmarkSuggestionsProvider::FetchBookmarksInternal() {
   } else {
     end_of_list_last_visit_date_ = suggestions.back().publish_date();
   }
-
   observer()->OnNewSuggestions(this, provided_category_,
                                std::move(suggestions));
 }

components/ntp_snippets/bookmarks/bookmark_suggestions_provider_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/bookmarks/bookmark_suggestions_provider.h"
+
+#include <memory>
+#include <string>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "components/bookmarks/browser/bookmark_model.h"
+#include "components/bookmarks/browser/bookmark_node.h"
+#include "components/bookmarks/test/test_bookmark_client.h"
+#include "components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h"
+#include "components/ntp_snippets/category.h"
+#include "components/ntp_snippets/mock_content_suggestions_provider_observer.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace ntp_snippets {
+
+namespace {
+
+using ::testing::StrictMock;
+using ::testing::_;
+using ::testing::Eq;
+using ::testing::IsEmpty;
+using ::testing::Property;
+using ::testing::UnorderedElementsAre;
+
+class BookmarkSuggestionsProviderTest : public ::testing::Test {
+ public:
+  BookmarkSuggestionsProviderTest()
+      : model_(bookmarks::TestBookmarkClient::CreateModel()) {
+    EXPECT_CALL(observer_, OnNewSuggestions(_, Category::FromKnownCategory(
+                                                   KnownCategories::BOOKMARKS),
+                                            IsEmpty()))
+        .RetiresOnSaturation();
+    EXPECT_CALL(observer_,
+                OnCategoryStatusChanged(
+                    _, Category::FromKnownCategory(KnownCategories::BOOKMARKS),
+                    CategoryStatus::AVAILABLE_LOADING))
+        .RetiresOnSaturation();
+    EXPECT_CALL(observer_,
+                OnCategoryStatusChanged(
+                    _, Category::FromKnownCategory(KnownCategories::BOOKMARKS),
+                    CategoryStatus::AVAILABLE))
+        .RetiresOnSaturation();
+    BookmarkSuggestionsProvider::RegisterProfilePrefs(test_prefs_.registry());
+    provider_ = base::MakeUnique<BookmarkSuggestionsProvider>(
+        &observer_, model_.get(), &test_prefs_);
+  }
+
+ protected:
+  std::unique_ptr<bookmarks::BookmarkModel> model_;
+  StrictMock<MockContentSuggestionsProviderObserver> observer_;
+  TestingPrefServiceSimple test_prefs_;
+  std::unique_ptr<BookmarkSuggestionsProvider> provider_;
+};
+
+TEST_F(BookmarkSuggestionsProviderTest,
+       ShouldProvideBookmarkSuggestions) {
+  GURL url("http://my-new-bookmarked.url");
+  // Note, this update to the model does not trigger OnNewSuggestions() on the
+  // observer as the provider realizes no new nodes were added.
+  // don't have new data.
+  model_->AddURL(model_->bookmark_bar_node(), 0,
+                 base::ASCIIToUTF16("cool page's title"), url);
+
+  // Once we provided the last-visited meta information, an update with the
+  // suggestion containing the bookmark should follow.
+  EXPECT_CALL(
+      observer_,
+      OnNewSuggestions(
+          _, Category::FromKnownCategory(KnownCategories::BOOKMARKS),
+          UnorderedElementsAre(Property(&ContentSuggestion::url, GURL(url)))));
+  UpdateBookmarkOnURLVisitedInMainFrame(model_.get(), url,
+                                        /*is_mobile_platform=*/true);
+}
+
+TEST_F(BookmarkSuggestionsProviderTest,
+       ShouldEnsureToBeClearedBookmarksDontAppearAfterClear) {
+  // Set up the provider with 2 entries: one dismissed and one active.
+
+  // Add one bookmark (the one to be not dismissed) -- this will trigger a
+  // notification.
+  GURL active_bookmark("http://my-active-bookmarked.url");
+  EXPECT_CALL(observer_,
+              OnNewSuggestions(
+                  _, Category::FromKnownCategory(KnownCategories::BOOKMARKS),
+                  UnorderedElementsAre(Property(&ContentSuggestion::url,
+                                                GURL(active_bookmark)))));
+  model_->AddURL(model_->bookmark_bar_node(), 0,
+                 base::ASCIIToUTF16("cool page's title"), active_bookmark);
+  UpdateBookmarkOnURLVisitedInMainFrame(model_.get(), active_bookmark,
+                                        /*is_mobile_platform=*/true);
+
+  // Add the other bookmark -- this will trigger another notification. Then
+  // marks it was dismissed.
+  GURL dismissed_bookmark("http://my-dismissed-bookmark.url");
+  EXPECT_CALL(
+      observer_,
+      OnNewSuggestions(
+          _, Category::FromKnownCategory(KnownCategories::BOOKMARKS),
+          UnorderedElementsAre(
+              Property(&ContentSuggestion::url, GURL(active_bookmark)),
+              Property(&ContentSuggestion::url, GURL(dismissed_bookmark)))));
+  const bookmarks::BookmarkNode* dismissed_node = model_->AddURL(
+      model_->bookmark_bar_node(), 1, base::ASCIIToUTF16("cool page's title"),
+      dismissed_bookmark);
+  UpdateBookmarkOnURLVisitedInMainFrame(model_.get(), dismissed_bookmark,
+                                        /*is_mobile_platform=*/true);
+  // According to the ContentSugestionsProvider contract, solely dismissing an
+  // item should not result in another OnNewSuggestions() call.
+  static_cast<ContentSuggestionsProvider*>(provider_.get())
+      ->DismissSuggestion(ContentSuggestion::ID(
+          Category::FromKnownCategory(KnownCategories::BOOKMARKS),
+          dismissed_bookmark.spec()));
+  EXPECT_THAT(IsDismissedFromNTPForBookmark(*dismissed_node), Eq(true));
+
+  // Clear history and make sure the suggestions actually get removed.
+  EXPECT_CALL(observer_, OnNewSuggestions(_, Category::FromKnownCategory(
+                                                 KnownCategories::BOOKMARKS),
+                                          IsEmpty()));
+  static_cast<ContentSuggestionsProvider*>(provider_.get())
+      ->ClearHistory(base::Time(), base::Time::Max(),
+                     base::Bind([] (const GURL& url) { return true; }));
+
+  // Verify the dismissed marker is gone.
+  EXPECT_THAT(IsDismissedFromNTPForBookmark(*dismissed_node), Eq(false));
+}
+
+// TODO(tschumann): There are plenty of test cases missing. Most importantly:
+// -- Remove a bookmark from the model
+// -- verifying handling of threshold time
+// -- dealing with fetches before the model is loaded.
+
+}  // namespace
+}  // namespace ntp_snippets
+

components/ntp_snippets/fake_content_suggestions_provider_observer.cc

+// Copyright 2017 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/fake_content_suggestions_provider_observer.h"
+
+#include <utility>
+
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ntp_snippets {
+
+using testing::Eq;
+using testing::Not;
+
+FakeContentSuggestionsProviderObserver::
+    FakeContentSuggestionsProviderObserver() = default;
+
+FakeContentSuggestionsProviderObserver::
+    ~FakeContentSuggestionsProviderObserver() = default;
+
+void FakeContentSuggestionsProviderObserver::OnNewSuggestions(
+    ContentSuggestionsProvider* provider,
+    Category category,
+    std::vector<ContentSuggestion> suggestions) {
+  suggestions_[category] = std::move(suggestions);
+}
+
+void FakeContentSuggestionsProviderObserver::OnCategoryStatusChanged(
+    ContentSuggestionsProvider* provider,
+    Category category,
+    CategoryStatus new_status) {
+  statuses_[category] = new_status;
+}
+
+void FakeContentSuggestionsProviderObserver::OnSuggestionInvalidated(
+    ContentSuggestionsProvider* provider,
+    const ContentSuggestion::ID& suggestion_id) {
+  FAIL() << "not implemented.";
+}
+
+const std::map<Category, CategoryStatus, Category::CompareByID>&
+FakeContentSuggestionsProviderObserver::statuses() const {
+  return statuses_;
+}
+
+CategoryStatus FakeContentSuggestionsProviderObserver::StatusForCategory(
+    Category category) const {
+  auto it = statuses_.find(category);
+  EXPECT_THAT(it, Not(Eq(statuses_.end())));
+  return it->second;
+}
+
+const std::vector<ContentSuggestion>&
+FakeContentSuggestionsProviderObserver::SuggestionsForCategory(
+    Category category) {
+  return suggestions_[category];
+}
+
+}  // namespace ntp_snippets

components/ntp_snippets/fake_content_suggestions_provider_observer.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_NTP_SNIPPETS_FAKE_CONTENT_SUGGESTIONS_PROVIDER_OBSERVER_H_
+#define COMPONENTS_NTP_SNIPPETS_FAKE_CONTENT_SUGGESTIONS_PROVIDER_OBSERVER_H_
+
+#include <map>
+#include <vector>
+
+#include "base/macros.h"
+#include "components/ntp_snippets/category.h"
+#include "components/ntp_snippets/content_suggestion.h"
+#include "components/ntp_snippets/content_suggestions_provider.h"
+
+namespace ntp_snippets {
+
+class FakeContentSuggestionsProviderObserver
+    : public ContentSuggestionsProvider::Observer {
+ public:
+  FakeContentSuggestionsProviderObserver();
+  ~FakeContentSuggestionsProviderObserver();
+
+  void OnNewSuggestions(ContentSuggestionsProvider* provider,
+                        Category category,
+                        std::vector<ContentSuggestion> suggestions) override;
+
+  void OnCategoryStatusChanged(ContentSuggestionsProvider* provider,
+                               Category category,
+                               CategoryStatus new_status) override;
+
+  void OnSuggestionInvalidated(
+      ContentSuggestionsProvider* provider,
+      const ContentSuggestion::ID& suggestion_id) override;
+
+  const std::map<Category, CategoryStatus, Category::CompareByID>& statuses()
+      const;
+
+  CategoryStatus StatusForCategory(Category category) const;
+
+  const std::vector<ContentSuggestion>& SuggestionsForCategory(
+      Category category);
+
+ private:
+  std::map<Category, CategoryStatus, Category::CompareByID> statuses_;
+  std::map<Category, std::vector<ContentSuggestion>, Category::CompareByID>
+      suggestions_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeContentSuggestionsProviderObserver);
+};
+
+}  // namespace ntp_snippets
+
+#endif  // COMPONENTS_NTP_SNIPPETS_FAKE_CONTENT_SUGGESTIONS_PROVIDER_OBSERVER_H_

components/ntp_snippets/mock_content_suggestions_provider.cc

+// Copyright 2017 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/mock_content_suggestions_provider.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/strings/utf_string_conversions.h"
+
+namespace ntp_snippets {
+
+MockContentSuggestionsProvider::MockContentSuggestionsProvider(
+    Observer* observer,
+    const std::vector<Category>& provided_categories)
+    : ContentSuggestionsProvider(observer) {
+  SetProvidedCategories(provided_categories);
+}
+
+MockContentSuggestionsProvider::~MockContentSuggestionsProvider() {}
+
+void MockContentSuggestionsProvider::SetProvidedCategories(
+    const std::vector<Category>& provided_categories) {
+  statuses_.clear();
+  provided_categories_ = provided_categories;
+  for (Category category : provided_categories) {
+    statuses_[category.id()] = CategoryStatus::AVAILABLE;
+  }
+}
+
+CategoryStatus MockContentSuggestionsProvider::GetCategoryStatus(
+    Category category) {
+  return statuses_[category.id()];
+}
+
+CategoryInfo MockContentSuggestionsProvider::GetCategoryInfo(
+    Category category) {
+  return CategoryInfo(base::ASCIIToUTF16("Section title"),
+                      ContentSuggestionsCardLayout::FULL_CARD, true, false,
+                      true, false,
+                      base::ASCIIToUTF16("No suggestions message"));
+}
+
+void MockContentSuggestionsProvider::FireSuggestionsChanged(
+    Category category,
+    std::vector<ContentSuggestion> suggestions) {
+  observer()->OnNewSuggestions(this, category, std::move(suggestions));
+}
+
+void MockContentSuggestionsProvider::FireCategoryStatusChanged(
+    Category category,
+    CategoryStatus new_status) {
+  statuses_[category.id()] = new_status;
+  observer()->OnCategoryStatusChanged(this, category, new_status);
+}
+
+void MockContentSuggestionsProvider::FireCategoryStatusChangedWithCurrentStatus(
+    Category category) {
+  observer()->OnCategoryStatusChanged(this, category, statuses_[category.id()]);
+}
+
+void MockContentSuggestionsProvider::FireSuggestionInvalidated(
+    const ContentSuggestion::ID& suggestion_id) {
+  observer()->OnSuggestionInvalidated(this, suggestion_id);
+}
+
+}  // namespace ntp_snippets

components/ntp_snippets/mock_content_suggestions_provider.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_NTP_SNIPPETS_MOCK_CONTENT_SUGGESTIONS_PROVIDER_H_
+#define COMPONENTS_NTP_SNIPPETS_MOCK_CONTENT_SUGGESTIONS_PROVIDER_H_
+
+#include <map>
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "components/ntp_snippets/content_suggestions_provider.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace ntp_snippets {
+
+// TODO(treib): This is a weird combination of a mock and a fake. Fix this.
+class MockContentSuggestionsProvider : public ContentSuggestionsProvider {
+ public:
+  MockContentSuggestionsProvider(
+      Observer* observer,
+      const std::vector<Category>& provided_categories);
+  ~MockContentSuggestionsProvider();
+
+  void SetProvidedCategories(const std::vector<Category>& provided_categories);
+
+  // Returns the status for |category|. The initial status in
+  // CatgoryStatus::AVAILABLE. Will be updated on FireCategoryStatusChanged
+  // events.
+  CategoryStatus GetCategoryStatus(Category category) override;
+
+  // Returns a hard-coded category info object.
+  CategoryInfo GetCategoryInfo(Category category) override;
+
+  // Forwards events to the underlying oberservers.
+  // TODO(tschumann): This functionality does not belong here. Whoever injected
+  // the observer into the constructor can as well notify the observer itself.
+  void FireSuggestionsChanged(Category category,
+                              std::vector<ContentSuggestion> suggestions);
+  void FireCategoryStatusChanged(Category category, CategoryStatus new_status);
+  void FireCategoryStatusChangedWithCurrentStatus(Category category);
+  void FireSuggestionInvalidated(const ContentSuggestion::ID& suggestion_id);
+
+  MOCK_METHOD3(ClearHistory,
+               void(base::Time begin,
+                    base::Time end,
+                    const base::Callback<bool(const GURL& url)>& filter));
+  MOCK_METHOD3(Fetch,
+               void(const Category&,
+                    const std::set<std::string>&,
+                    const FetchDoneCallback&));
+  MOCK_METHOD1(ClearCachedSuggestions, void(Category category));
+  MOCK_METHOD2(GetDismissedSuggestionsForDebugging,
+               void(Category category,
+                    const DismissedSuggestionsCallback& callback));
+  MOCK_METHOD1(ClearDismissedSuggestionsForDebugging, void(Category category));
+  MOCK_METHOD1(DismissSuggestion,
+               void(const ContentSuggestion::ID& suggestion_id));
+  MOCK_METHOD2(FetchSuggestionImage,
+               void(const ContentSuggestion::ID& suggestion_id,
+                    const ImageFetchedCallback& callback));
+
+ private:
+  std::vector<Category> provided_categories_;
+  std::map<int, CategoryStatus> statuses_;
+};
+
+}  // namespace ntp_snippets
+
+#endif  // COMPONENTS_NTP_SNIPPETS_MOCK_CONTENT_SUGGESTIONS_PROVIDER_H_

components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc

@@ -31,6 +31,7 @@
 #include "components/ntp_snippets/category_rankers/category_ranker.h"
 #include "components/ntp_snippets/category_rankers/constant_category_ranker.h"
 #include "components/ntp_snippets/category_rankers/mock_category_ranker.h"
+#include "components/ntp_snippets/fake_content_suggestions_provider_observer.h"
 #include "components/ntp_snippets/ntp_snippets_constants.h"
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/remote/ntp_snippet.h"
@@ -343,51 +344,6 @@ class MockImageFetcher : public ImageFetcher {
            base::Callback<void(const std::string&, const gfx::Image&)>));
 };
 
-class FakeContentSuggestionsProviderObserver
-    : public ContentSuggestionsProvider::Observer {
- public:
-  FakeContentSuggestionsProviderObserver() = default;
-
-  void OnNewSuggestions(ContentSuggestionsProvider* provider,
-                        Category category,
-                        std::vector<ContentSuggestion> suggestions) override {
-    suggestions_[category] = std::move(suggestions);
-  }
-
-  void OnCategoryStatusChanged(ContentSuggestionsProvider* provider,
-                               Category category,
-                               CategoryStatus new_status) override {
-    statuses_[category] = new_status;
-  }
-
-  void OnSuggestionInvalidated(
-      ContentSuggestionsProvider* provider,
-      const ContentSuggestion::ID& suggestion_id) override {}
-
-  const std::map<Category, CategoryStatus, Category::CompareByID>& statuses()
-      const {
-    return statuses_;
-  }
-
-  CategoryStatus StatusForCategory(Category category) const {
-    auto it = statuses_.find(category);
-    EXPECT_THAT(it, Not(Eq(statuses_.end())));
-    return it->second;
-  }
-
-  const std::vector<ContentSuggestion>& SuggestionsForCategory(
-      Category category) {
-    return suggestions_[category];
-  }
-
- private:
-  std::map<Category, CategoryStatus, Category::CompareByID> statuses_;
-  std::map<Category, std::vector<ContentSuggestion>, Category::CompareByID>
-      suggestions_;
-
-  DISALLOW_COPY_AND_ASSIGN(FakeContentSuggestionsProviderObserver);
-};
-
 class FakeImageDecoder : public image_fetcher::ImageDecoder {
  public:
   FakeImageDecoder() {}