Remove deleted offline page suggestions from opened NTPs

components/ntp_snippets/content_suggestions_service_unittest.cc

Index: components/ntp_snippets/content_suggestions_service_unittest.cc
diff --git a/components/ntp_snippets/content_suggestions_service_unittest.cc b/components/ntp_snippets/content_suggestions_service_unittest.cc
index a7f75386acb4268a0af656e142c1ca04ab12ffdd..84c047a623e2db5732d50220e1f870c7e9b3fe13 100644
--- a/components/ntp_snippets/content_suggestions_service_unittest.cc
+++ b/components/ntp_snippets/content_suggestions_service_unittest.cc
@@ -87,6 +87,11 @@ class MockProvider : public ContentSuggestionsProvider {
     observer()->OnCategoryStatusChanged(this, category, new_status);
   }
 
+  void FireSuggestionInvalidated(Category category,
+                                 const std::string& suggestion_id) {
+    observer()->OnSuggestionInvalidated(this, category, suggestion_id);
+  }
+
   MOCK_METHOD1(ClearCachedSuggestionsForDebugging, void(Category category));
   MOCK_METHOD1(GetDismissedSuggestionsForDebugging,
                std::vector<ContentSuggestion>(Category category));
@@ -106,6 +111,8 @@ class MockServiceObserver : public ContentSuggestionsService::Observer {
   MOCK_METHOD1(OnNewSuggestions, void(Category category));
   MOCK_METHOD2(OnCategoryStatusChanged,
                void(Category changed_category, CategoryStatus new_status));
+  MOCK_METHOD2(OnSuggestionInvalidated,
+               void(Category category, const std::string& suggestion_id));
   MOCK_METHOD0(ContentSuggestionsServiceShutdown, void());
   ~MockServiceObserver() override {}
 };
@@ -263,7 +270,7 @@ TEST_F(ContentSuggestionsServiceTest, ShouldRedirectFetchSuggestionImage) {
   provider1->FireSuggestionsChanged(articles_category, {1});
   std::string suggestion_id = CreateSuggestion(1).id();
 
-  EXPECT_CALL(*provider1, FetchSuggestionImage(suggestion_id, _)).Times(1);
+  EXPECT_CALL(*provider1, FetchSuggestionImage(suggestion_id, _));
   EXPECT_CALL(*provider2, FetchSuggestionImage(_, _)).Times(0);
   service()->FetchSuggestionImage(
       suggestion_id, base::Bind(&ContentSuggestionsServiceTest::OnImageFetched,
@@ -297,10 +304,36 @@ TEST_F(ContentSuggestionsServiceTest, ShouldRedirectDismissSuggestion) {
   std::string suggestion_id = CreateSuggestion(11).id();
 
   EXPECT_CALL(*provider1, DismissSuggestion(_)).Times(0);
-  EXPECT_CALL(*provider2, DismissSuggestion(suggestion_id)).Times(1);
+  EXPECT_CALL(*provider2, DismissSuggestion(suggestion_id));
   service()->DismissSuggestion(suggestion_id);
 }
 
+TEST_F(ContentSuggestionsServiceTest, ShouldRedirectSuggestionInvalidated) {
+  Category articles_category = FromKnownCategory(KnownCategories::ARTICLES);
+
+  MockProvider* provider = MakeProvider(articles_category);
+  MockServiceObserver observer;
+  service()->AddObserver(&observer);
+
+  provider->FireSuggestionsChanged(articles_category, {11, 12, 13});
+  ExpectThatSuggestionsAre(articles_category, {11, 12, 13});
+  std::string suggestion_id = CreateSuggestion(12).id();

[Marc Treib, 2016/08/16 11:40:10]

nitty nit: Move this after the empty line?

[Philipp Keck, 2016/08/16 11:51:03]

Done.

+
+  EXPECT_CALL(observer,
+              OnSuggestionInvalidated(articles_category, suggestion_id));
+  provider->FireSuggestionInvalidated(articles_category, suggestion_id);
+  ExpectThatSuggestionsAre(articles_category, {11, 13});
+  Mock::VerifyAndClearExpectations(&observer);
+
+  std::string unknown_id = CreateSuggestion(1234).id();

[Marc Treib, 2016/08/16 11:40:09]

nit: Add a comment explaining why? Otherwise the next reader will probably
wonder.

[Philipp Keck, 2016/08/16 11:51:03]

Done.

+  EXPECT_CALL(observer, OnSuggestionInvalidated(articles_category, unknown_id));
+  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 =
@@ -317,27 +350,27 @@ TEST_F(ContentSuggestionsServiceTest, ShouldForwardSuggestions) {
   service()->AddObserver(&observer);
 
   // Send suggestions 1 and 2
-  EXPECT_CALL(observer, OnNewSuggestions(articles_category)).Times(1);
+  EXPECT_CALL(observer, OnNewSuggestions(articles_category));
   provider1->FireSuggestionsChanged(articles_category, {1, 2});
   ExpectThatSuggestionsAre(articles_category, {1, 2});
   Mock::VerifyAndClearExpectations(&observer);
 
   // Send them again, make sure they're not reported twice
-  EXPECT_CALL(observer, OnNewSuggestions(articles_category)).Times(1);
+  EXPECT_CALL(observer, OnNewSuggestions(articles_category));
   provider1->FireSuggestionsChanged(articles_category, {1, 2});
   ExpectThatSuggestionsAre(articles_category, {1, 2});
   ExpectThatSuggestionsAre(offline_pages_category, std::vector<int>());
   Mock::VerifyAndClearExpectations(&observer);
 
   // Send suggestions 13 and 14
-  EXPECT_CALL(observer, OnNewSuggestions(offline_pages_category)).Times(1);
+  EXPECT_CALL(observer, OnNewSuggestions(offline_pages_category));
   provider2->FireSuggestionsChanged(offline_pages_category, {13, 14});
   ExpectThatSuggestionsAre(articles_category, {1, 2});
   ExpectThatSuggestionsAre(offline_pages_category, {13, 14});
   Mock::VerifyAndClearExpectations(&observer);
 
   // Send suggestion 1 only
-  EXPECT_CALL(observer, OnNewSuggestions(articles_category)).Times(1);
+  EXPECT_CALL(observer, OnNewSuggestions(articles_category));
   provider1->FireSuggestionsChanged(articles_category, {1});
   ExpectThatSuggestionsAre(articles_category, {1});
   ExpectThatSuggestionsAre(offline_pages_category, {13, 14});
@@ -346,8 +379,7 @@ TEST_F(ContentSuggestionsServiceTest, ShouldForwardSuggestions) {
   // provider2 reports BOOKMARKS as unavailable
   EXPECT_CALL(observer, OnCategoryStatusChanged(
                             offline_pages_category,
-                            CategoryStatus::CATEGORY_EXPLICITLY_DISABLED))
-      .Times(1);
+                            CategoryStatus::CATEGORY_EXPLICITLY_DISABLED));
   provider2->FireCategoryStatusChanged(
       offline_pages_category, CategoryStatus::CATEGORY_EXPLICITLY_DISABLED);
   EXPECT_THAT(service()->GetCategoryStatus(articles_category),