Last visit dates of bookmarks - fix browsing data removal on desktop

chrome/browser/browsing_data/browsing_data_remover.cc

Index: chrome/browser/browsing_data/browsing_data_remover.cc
diff --git a/chrome/browser/browsing_data/browsing_data_remover.cc b/chrome/browser/browsing_data/browsing_data_remover.cc
index cc955a24263840ed51271fdf9ef8d382c7154cee..8af58b7ac33c3500348bd90392a4c0ca654b1590 100644
--- a/chrome/browser/browsing_data/browsing_data_remover.cc
+++ b/chrome/browser/browsing_data/browsing_data_remover.cc
@@ -16,6 +16,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
+#include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_filter_builder.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
@@ -51,6 +52,7 @@
 #include "chrome/common/url_constants.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
+#include "components/bookmarks/browser/bookmark_model.h"
 #include "components/browsing_data/content/storage_partition_http_cache_data_remover.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
@@ -62,6 +64,7 @@
 #include "components/history/core/browser/history_service.h"
 #include "components/nacl/browser/nacl_browser.h"
 #include "components/nacl/browser/pnacl_host.h"
+#include "components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h"
 #include "components/ntp_snippets/content_suggestions_service.h"
 #include "components/omnibox/browser/omnibox_pref_names.h"
 #include "components/password_manager/core/browser/password_store.h"
@@ -535,6 +538,13 @@ void BrowsingDataRemover::RemoveImpl(
                                                 filter);
     }
 
+    // Remove the last visit dates meta-data from the bookmark model.
+    // TODO(vitaliii): Do not remove all dates, but only the ones matched by the
+    // time range and the filter.
+    bookmarks::BookmarkModel* bookmark_model =
+        BookmarkModelFactory::GetForBrowserContext(profile_);
+    ntp_snippets::RemoveAllLastVisitDates(bookmark_model);

[tschumann, 2016/12/12 13:26:18]

should we handle nullptr? seems safer...

[msramek, 2016/12/12 13:33:40]

+1

Also, please add a unittest; that should not hurt the simplicity w.r.t. merging,
and is a good practice when you're fixing an oversight bug.

[jkrcal, 2016/12/12 19:22:04]

True. Definitely safer.

[jkrcal, 2016/12/12 19:22:04]

Hmm, I tried hard, the cost for the unit-test is quite high ;)
 - As this a simple call of a static function, I need to do it slightly more
end-to-end for the test to make sense. Makes sense?
 - BookmarkModelFactory returns nullptr in unittests. I need to inject a fake
inside the remover. Any objections?

Any other comments?

+
 #if BUILDFLAG(ENABLE_EXTENSIONS)
     // The extension activity log contains details of which websites extensions
     // were active on. It therefore indirectly stores details of websites a