Refactor OfflinePageSuggestionsProviderTest

components/ntp_snippets/offline_pages/offline_page_suggestions_provider_unittest.cc

Index: components/ntp_snippets/offline_pages/offline_page_suggestions_provider_unittest.cc
diff --git a/components/ntp_snippets/offline_pages/offline_page_suggestions_provider_unittest.cc b/components/ntp_snippets/offline_pages/offline_page_suggestions_provider_unittest.cc
index bdceafd23489f3695232dba18db693e0f91036ba..039ac5b9869cb720d987cbf5314fd4d462041c44 100644
--- a/components/ntp_snippets/offline_pages/offline_page_suggestions_provider_unittest.cc
+++ b/components/ntp_snippets/offline_pages/offline_page_suggestions_provider_unittest.cc
@@ -75,17 +75,25 @@ OfflinePageItem CreateDummyDownload(int id) {
   return CreateDummyItem(offline_pages::kAsyncNamespace, id);
 }
 
+void CaptureDismissedSuggestions(
+    std::vector<ContentSuggestion>* captured_suggestions,
+    std::vector<ContentSuggestion> dismissed_suggestions) {
+  std::move(dismissed_suggestions.begin(), dismissed_suggestions.end(),
+            std::back_inserter(*captured_suggestions));
+}
+
 }  // namespace
 
-class MockOfflinePageModel : public StubOfflinePageModel {
+class FakeOfflinePageModel : public StubOfflinePageModel {
  public:
-  MockOfflinePageModel() {}
+  FakeOfflinePageModel() {}
 
   void GetAllPages(const MultipleOfflinePageItemCallback& callback) override {
     callback.Run(items_);
   }
 
-  std::vector<OfflinePageItem>* items() { return &items_; }
+  const std::vector<OfflinePageItem>& items() { return items_; }
+  std::vector<OfflinePageItem>* mutable_items() { return &items_; }
 
  private:
   std::vector<OfflinePageItem> items_;
@@ -126,8 +134,6 @@ class OfflinePageSuggestionsProviderTest : public testing::Test {
     return category_factory_.FromKnownCategory(KnownCategories::DOWNLOADS);
   }
 
-  void AddItem(OfflinePageItem item) { model()->items()->push_back(item); }
-
   std::string GetDummySuggestionId(Category category, int id) {
     return provider_->MakeUniqueID(category, base::IntToString(id));
   }
@@ -153,24 +159,13 @@ class OfflinePageSuggestionsProviderTest : public testing::Test {
     return provider_->ReadDismissedIDsFromPrefs(category);
   }
 
-  // Workaround to realize a DismissedSuggestionsCallback. Because gMock can't
-  // handle non-movable parameters, a helper method is needed to forward the
-  // call to the actual MOCK_METHOD.
-  void DismissedSuggestionsHelper(
-      std::vector<ContentSuggestion> dismissed_suggestions) {
-    ReceivedDismissedSuggestions(dismissed_suggestions);
-  }
-  MOCK_METHOD1(
-      ReceivedDismissedSuggestions,
-      void(const std::vector<ContentSuggestion>& dismissed_suggestions));
-
   ContentSuggestionsProvider* provider() { return provider_.get(); }
-  MockOfflinePageModel* model() { return &model_; }
+  FakeOfflinePageModel* model() { return &model_; }
   MockContentSuggestionsProviderObserver* observer() { return &observer_; }
   TestingPrefServiceSimple* pref_service() { return pref_service_.get(); }
 
  private:
-  MockOfflinePageModel model_;
+  FakeOfflinePageModel model_;
   MockContentSuggestionsProviderObserver observer_;
   CategoryFactory category_factory_;
   std::unique_ptr<TestingPrefServiceSimple> pref_service_;
@@ -181,10 +176,10 @@ class OfflinePageSuggestionsProviderTest : public testing::Test {
 };
 
 TEST_F(OfflinePageSuggestionsProviderTest, ShouldSplitAndConvertToSuggestions) {
-  AddItem(CreateDummyRecentTab(1));
-  AddItem(CreateDummyRecentTab(2));
-  AddItem(CreateDummyRecentTab(3));
-  AddItem(CreateDummyDownload(101));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(1));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(2));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(3));
+  model()->mutable_items()->push_back(CreateDummyDownload(101));
 
   EXPECT_CALL(
       *observer(),
@@ -210,10 +205,10 @@ TEST_F(OfflinePageSuggestionsProviderTest, ShouldSplitAndConvertToSuggestions) {
 }
 
 TEST_F(OfflinePageSuggestionsProviderTest, ShouldIgnoreDisabledCategories) {
-  AddItem(CreateDummyRecentTab(1));
-  AddItem(CreateDummyRecentTab(2));
-  AddItem(CreateDummyRecentTab(3));
-  AddItem(CreateDummyDownload(101));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(1));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(2));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(3));
+  model()->mutable_items()->push_back(CreateDummyDownload(101));
 
   // Disable recent tabs, enable downloads.
   EXPECT_CALL(*observer(), OnNewSuggestions(_, recent_tabs_category(), _))
@@ -248,9 +243,9 @@ TEST_F(OfflinePageSuggestionsProviderTest, ShouldSortByMostRecentlyVisited) {
   base::Time now = base::Time::Now();
   base::Time yesterday = now - base::TimeDelta::FromDays(1);
   base::Time tomorrow = now + base::TimeDelta::FromDays(1);
-  AddItem(CreateDummyRecentTab(1, now));
-  AddItem(CreateDummyRecentTab(2, yesterday));
-  AddItem(CreateDummyRecentTab(3, tomorrow));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(1, now));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(2, yesterday));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(3, tomorrow));
 
   EXPECT_CALL(
       *observer(),
@@ -279,10 +274,10 @@ TEST_F(OfflinePageSuggestionsProviderTest, ShouldDeliverCorrectCategoryInfo) {
 }
 
 TEST_F(OfflinePageSuggestionsProviderTest, ShouldDismiss) {
-  AddItem(CreateDummyRecentTab(1));
-  AddItem(CreateDummyRecentTab(2));
-  AddItem(CreateDummyRecentTab(3));
-  AddItem(CreateDummyRecentTab(4));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(1));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(2));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(3));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(4));
   FireOfflinePageModelChanged();
 
   // Dismiss 2 and 3.
@@ -308,38 +303,33 @@ TEST_F(OfflinePageSuggestionsProviderTest, ShouldDismiss) {
   Mock::VerifyAndClearExpectations(observer());
 
   // And appear in the dismissed suggestions for the right category.
-  EXPECT_CALL(*this, ReceivedDismissedSuggestions(UnorderedElementsAre(
-                         Property(&ContentSuggestion::url,
-                                  GURL("file:///some/folder/test2.mhtml")),
-                         Property(&ContentSuggestion::url,
-                                  GURL("file:///some/folder/test3.mhtml")))));
+  std::vector<ContentSuggestion> dismissed_suggestions;
   provider()->GetDismissedSuggestionsForDebugging(
       recent_tabs_category(),
-      base::Bind(
-          &OfflinePageSuggestionsProviderTest::DismissedSuggestionsHelper,
-          base::Unretained(this)));
-  Mock::VerifyAndClearExpectations(this);
+      base::Bind(&CaptureDismissedSuggestions, &dismissed_suggestions));
+  EXPECT_THAT(
+      dismissed_suggestions,
+      UnorderedElementsAre(Property(&ContentSuggestion::url,
+                                    GURL("file:///some/folder/test2.mhtml")),
+                           Property(&ContentSuggestion::url,
+                                    GURL("file:///some/folder/test3.mhtml"))));
 
   // The other category should have no dismissed suggestions.
-  EXPECT_CALL(*this, ReceivedDismissedSuggestions(IsEmpty()));
+  dismissed_suggestions.clear();
   provider()->GetDismissedSuggestionsForDebugging(
       downloads_category(),
-      base::Bind(
-          &OfflinePageSuggestionsProviderTest::DismissedSuggestionsHelper,
-          base::Unretained(this)));
-  Mock::VerifyAndClearExpectations(this);
+      base::Bind(&CaptureDismissedSuggestions, &dismissed_suggestions));
+  EXPECT_THAT(dismissed_suggestions, IsEmpty());
 
   // Clear dismissed suggestions.
   provider()->ClearDismissedSuggestionsForDebugging(recent_tabs_category());
 
   // They should be gone from the dismissed suggestions.
-  EXPECT_CALL(*this, ReceivedDismissedSuggestions(IsEmpty()));
+  dismissed_suggestions.clear();
   provider()->GetDismissedSuggestionsForDebugging(
       recent_tabs_category(),
-      base::Bind(
-          &OfflinePageSuggestionsProviderTest::DismissedSuggestionsHelper,
-          base::Unretained(this)));
-  Mock::VerifyAndClearExpectations(this);
+      base::Bind(&CaptureDismissedSuggestions, &dismissed_suggestions));
+  EXPECT_THAT(dismissed_suggestions, IsEmpty());
 
   // And appear in the reported suggestions for the category again.
   EXPECT_CALL(*observer(),
@@ -352,9 +342,9 @@ TEST_F(OfflinePageSuggestionsProviderTest, ShouldDismiss) {
 
 TEST_F(OfflinePageSuggestionsProviderTest,
        ShouldInvalidateWhenOfflinePageDeleted) {
-  AddItem(CreateDummyRecentTab(1));
-  AddItem(CreateDummyRecentTab(2));
-  AddItem(CreateDummyRecentTab(3));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(1));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(2));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(3));
   FireOfflinePageModelChanged();
 
   // Invalidation of suggestion 2 should be forwarded.
@@ -362,13 +352,13 @@ TEST_F(OfflinePageSuggestionsProviderTest,
       *observer(),
       OnSuggestionInvalidated(_, recent_tabs_category(),
                               GetDummySuggestionId(recent_tabs_category(), 2)));
-  FireOfflinePageDeleted(model()->items()->at(1));
+  FireOfflinePageDeleted(model()->items().at(1));
 }
 
 TEST_F(OfflinePageSuggestionsProviderTest, ShouldClearDismissedOnInvalidate) {
-  AddItem(CreateDummyRecentTab(1));
-  AddItem(CreateDummyRecentTab(2));
-  AddItem(CreateDummyRecentTab(3));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(1));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(2));
+  model()->mutable_items()->push_back(CreateDummyRecentTab(3));
   FireOfflinePageModelChanged();
   EXPECT_THAT(ReadDismissedIDsFromPrefs(recent_tabs_category()), IsEmpty());
   EXPECT_THAT(ReadDismissedIDsFromPrefs(downloads_category()), IsEmpty());
@@ -378,15 +368,15 @@ TEST_F(OfflinePageSuggestionsProviderTest, ShouldClearDismissedOnInvalidate) {
   EXPECT_THAT(ReadDismissedIDsFromPrefs(recent_tabs_category()), SizeIs(1));
   EXPECT_THAT(ReadDismissedIDsFromPrefs(downloads_category()), IsEmpty());
 
-  FireOfflinePageDeleted(model()->items()->at(1));
+  FireOfflinePageDeleted(model()->items().at(1));
   EXPECT_THAT(ReadDismissedIDsFromPrefs(recent_tabs_category()), IsEmpty());
   EXPECT_THAT(ReadDismissedIDsFromPrefs(downloads_category()), IsEmpty());
 }
 
 TEST_F(OfflinePageSuggestionsProviderTest, ShouldClearDismissedOnFetch) {
-  AddItem(CreateDummyDownload(1));
-  AddItem(CreateDummyDownload(2));
-  AddItem(CreateDummyDownload(3));
+  model()->mutable_items()->push_back(CreateDummyDownload(1));
+  model()->mutable_items()->push_back(CreateDummyDownload(2));
+  model()->mutable_items()->push_back(CreateDummyDownload(3));
   FireOfflinePageModelChanged();
 
   provider()->DismissSuggestion(GetDummySuggestionId(downloads_category(), 2));
@@ -394,13 +384,13 @@ TEST_F(OfflinePageSuggestionsProviderTest, ShouldClearDismissedOnFetch) {
   EXPECT_THAT(ReadDismissedIDsFromPrefs(recent_tabs_category()), IsEmpty());
   EXPECT_THAT(ReadDismissedIDsFromPrefs(downloads_category()), SizeIs(2));
 
-  model()->items()->clear();
-  AddItem(CreateDummyDownload(2));
+  model()->mutable_items()->clear();
+  model()->mutable_items()->push_back(CreateDummyDownload(2));
   FireOfflinePageModelChanged();
   EXPECT_THAT(ReadDismissedIDsFromPrefs(recent_tabs_category()), IsEmpty());
   EXPECT_THAT(ReadDismissedIDsFromPrefs(downloads_category()), SizeIs(1));
 
-  model()->items()->clear();
+  model()->mutable_items()->clear();
   FireOfflinePageModelChanged();
   EXPECT_THAT(ReadDismissedIDsFromPrefs(recent_tabs_category()), IsEmpty());
   EXPECT_THAT(ReadDismissedIDsFromPrefs(downloads_category()), IsEmpty());