Remove deleted offline page suggestions from opened NTPs

components/ntp_snippets/offline_pages/offline_page_suggestions_provider.cc

Index: components/ntp_snippets/offline_pages/offline_page_suggestions_provider.cc
diff --git a/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.cc b/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.cc
index 0c153735bee2a199a1d5cf08bd9c0b2de1880202..3491a708c13ecb33c1a035c8b69d517587f9db49 100644
--- a/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.cc
+++ b/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.cc
@@ -216,7 +216,35 @@ void OfflinePageSuggestionsProvider::OfflinePageModelChanged(
 void OfflinePageSuggestionsProvider::OfflinePageDeleted(
     int64_t offline_id,
     const offline_pages::ClientId& client_id) {
-  // TODO(pke): Implement, suggestion has to be removed from UI immediately.
+  std::string offline_page_id = base::IntToString(offline_id);
+  if (recent_tabs_status_ != CategoryStatus::NOT_PROVIDED &&
+      client_id.name_space == offline_pages::kLastNNamespace) {
+    auto it = std::find(dismissed_recent_tab_ids_.begin(),
+                        dismissed_recent_tab_ids_.end(), offline_page_id);
+    if (it == dismissed_recent_tab_ids_.end()) {
+      observer()->OnSuggestionInvalidated(

[Marc Treib, 2016/08/16 09:03:54]

I think we should send the invalidation in any case, even if it was dismissed -
there might still be *another* open NTP that has it. (super edge case, but
still)

Probably also if the category status is "bad" - since we don't clear the UI in
that case, the suggestion might also still be there.

[Philipp Keck, 2016/08/16 10:54:40]

I will fix this in an upcoming CL (all this code is totally refactored there).
There is another detail worth noting: The OfflinePageSuggestionsProvider doesn't
know what it's forwarding here. As we only return the first 10 results from the
OfflinePageModel when delivering suggestions, we have a good chance of reporting
a suggestion as invalidated which has never existed in the first place. I will
also document this in the next CL.
If we switched to the NOT_PROVIDED status dynamically, you would be right - it's
the only case where the category becomes unavailable but the section is still
visible on the UI. But the OfflinePageSuggestionsProvider doesn't do that.
We could still forward it, though I don't see why we would need to.

Calling OnSuggestionInvalidated for categories that never existed is a bit
weird, but not a problem.

[Philipp Keck, 2016/08/16 11:21:18]

The former is now implemented here:
https://codereview.chromium.org/2251743002/diff/20001/components/ntp_snippets...

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

Fair enough, but probably worth a comment?

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

Done.

+          this, recent_tabs_category_,
+          MakeUniqueID(recent_tabs_category_, offline_page_id));
+    } else {
+      dismissed_recent_tab_ids_.erase(it);
+      StoreDismissedIDsToPrefs(prefs::kDismissedRecentOfflineTabSuggestions,
+                               dismissed_recent_tab_ids_);
+    }
+  } else if (downloads_status_ != CategoryStatus::NOT_PROVIDED &&
+             client_id.name_space == offline_pages::kAsyncNamespace &&
+             base::IsValidGUID(client_id.id)) {
+    auto it = std::find(dismissed_download_ids_.begin(),
+                        dismissed_download_ids_.end(), offline_page_id);
+    if (it == dismissed_download_ids_.end()) {
+      observer()->OnSuggestionInvalidated(
+          this, downloads_category_,
+          MakeUniqueID(downloads_category_, offline_page_id));
+    } else {
+      dismissed_download_ids_.erase(it);
+      StoreDismissedIDsToPrefs(prefs::kDismissedDownloadSuggestions,
+                               dismissed_download_ids_);
+    }
+  }
 }
 
 void OfflinePageSuggestionsProvider::FetchOfflinePages() {
@@ -245,6 +273,7 @@ void OfflinePageSuggestionsProvider::OnOfflinePagesLoaded(
 
     // TODO(pke): Use kDownloadNamespace once the OfflinePageModel uses that.
     // The current logic is taken from DownloadUIAdapter::IsVisibleInUI.
+    // Note: This is also copied in OfflinePageDeleted above.
     if (need_downloads &&
         item.client_id.name_space == offline_pages::kAsyncNamespace &&
         base::IsValidGUID(item.client_id.id) &&