Add BrowsingDataHistoryObserverService to listen for history deletions

In order to delete navigations, recents tabs and session entries,
a service needs to listen to history deletions and remove
this data when required. Because history deletions can not only be
triggered by UI but also by extensions, the listener should be created
at startup. Because ChromeBrowsingDataDelegate is not that small
and has a few other dependencies, a separate service in browsing_data/
is created.

Bug: 407074
Change-Id: If068023967d502ba663473d13ec7844d70e7ca14
Reviewed-on: https://chromium-review.googlesource.com/962762
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544363}

chrome/browser/BUILD.gn

@@ -170,6 +170,8 @@ jumbo_split_static_library("browser") {
     "browsing_data/browsing_data_file_system_helper.h",
     "browsing_data/browsing_data_helper.cc",
     "browsing_data/browsing_data_helper.h",
+    "browsing_data/browsing_data_history_observer_service.cc",
+    "browsing_data/browsing_data_history_observer_service.h",
     "browsing_data/browsing_data_important_sites_util.cc",
     "browsing_data/browsing_data_important_sites_util.h",
     "browsing_data/browsing_data_indexed_db_helper.cc",

chrome/browser/android/history/browsing_history_bridge.cc

@@ -43,9 +43,6 @@ BrowsingHistoryBridge::BrowsingHistoryBridge(JNIEnv* env,
       ProfileSyncServiceFactory::GetSyncServiceForBrowserContext(profile_);
   browsing_history_service_ = std::make_unique<BrowsingHistoryService>(
       this, local_history, sync_service);
-  // Make sure BrowsingDataRemoverDelegate is initialized and listening
-  // for history deletions.
-  profile_->GetBrowsingDataRemoverDelegate();
 
   j_history_service_obj_.Reset(env, obj);
 }

chrome/browser/browsing_data/browsing_data_history_observer_service.cc

+// Copyright 2018 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 "chrome/browser/browsing_data/browsing_data_history_observer_service.h"
+
+#include "chrome/browser/browsing_data/navigation_entry_remover.h"
+#include "chrome/browser/history/history_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/tab_restore_service_factory.h"
+#include "chrome/common/buildflags.h"
+#include "components/history/core/browser/history_service.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+
+#if BUILDFLAG(ENABLE_SESSION_SERVICE)
+#include "chrome/browser/sessions/session_service_factory.h"
+#endif
+
+BrowsingDataHistoryObserverService::BrowsingDataHistoryObserverService(
+    Profile* profile)
+    : profile_(profile), history_observer_(this) {
+  auto* history_service = HistoryServiceFactory::GetForProfile(
+      profile, ServiceAccessType::EXPLICIT_ACCESS);
+  if (history_service)
+    history_observer_.Add(history_service);
+}
+
+BrowsingDataHistoryObserverService::~BrowsingDataHistoryObserverService() {}
+
+void BrowsingDataHistoryObserverService::OnURLsDeleted(
+    history::HistoryService* history_service,
+    const history::DeletionTimeRange& time_range,
+    bool expired,
+    const history::URLRows& deleted_rows,
+    const std::set<GURL>& favicon_urls) {
+  if (!expired)
+    browsing_data::RemoveNavigationEntries(profile_, time_range, deleted_rows);
+}
+
+// static
+BrowsingDataHistoryObserverService::Factory*
+BrowsingDataHistoryObserverService::Factory::GetInstance() {
+  return base::Singleton<BrowsingDataHistoryObserverService::Factory>::get();
+}
+
+BrowsingDataHistoryObserverService::Factory::Factory()
+    : BrowserContextKeyedServiceFactory(
+          "BrowsingDataHistoryObserverService",
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(HistoryServiceFactory::GetInstance());
+  DependsOn(TabRestoreServiceFactory::GetInstance());
+#if BUILDFLAG(ENABLE_SESSION_SERVICE)
+  DependsOn(SessionServiceFactory::GetInstance());
+#endif
+}
+
+KeyedService*
+BrowsingDataHistoryObserverService::Factory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  Profile* profile = Profile::FromBrowserContext(context);
+  if (profile->IsOffTheRecord() || profile->IsGuestSession())
+    return nullptr;
+  return new BrowsingDataHistoryObserverService(profile);
+}
+
+bool BrowsingDataHistoryObserverService::Factory::
+    ServiceIsCreatedWithBrowserContext() const {
+  // Create this service at startup to receive all deletion events.
+  return true;
+}

chrome/browser/browsing_data/browsing_data_history_observer_service.h

+// Copyright 2018 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 CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_HISTORY_OBSERVER_SERVICE_H_
+#define CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_HISTORY_OBSERVER_SERVICE_H_
+
+#include "base/scoped_observer.h"
+#include "components/history/core/browser/history_service_observer.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+class Profile;
+
+namespace base {
+template <typename T>
+struct DefaultSingletonTraits;
+}
+
+// BrowsingDataHistoryObserverService is listening for history deletions to
+// remove navigation, session and recent tab entries.
+class BrowsingDataHistoryObserverService
+    : public KeyedService,
+      public history::HistoryServiceObserver {
+ public:
+  explicit BrowsingDataHistoryObserverService(Profile* profile);
+  ~BrowsingDataHistoryObserverService() override;
+
+  // history::HistoryServiceObserver:
+  void OnURLsDeleted(history::HistoryService* history_service,
+                     const history::DeletionTimeRange& time_range,
+                     bool expired,
+                     const history::URLRows& deleted_rows,
+                     const std::set<GURL>& favicon_urls) override;
+
+  class Factory : public BrowserContextKeyedServiceFactory {
+   public:
+    static Factory* GetInstance();
+
+   private:
+    friend struct base::DefaultSingletonTraits<Factory>;
+
+    Factory();
+    ~Factory() override = default;
+
+    // BrowserContextKeyedServiceFactory:
+    KeyedService* BuildServiceInstanceFor(
+        content::BrowserContext* context) const override;
+    bool ServiceIsCreatedWithBrowserContext() const override;
+  };
+
+ private:
+  Profile* profile_;
+
+  ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
+      history_observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowsingDataHistoryObserverService);
+};
+
+#endif  // CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_HISTORY_OBSERVER_SERVICE_H_

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -327,8 +327,7 @@ bool DoesOriginMatchEmbedderMask(int origin_type_mask,
 }  // namespace
 
 ChromeBrowsingDataRemoverDelegate::ChromeBrowsingDataRemoverDelegate(
-    BrowserContext* browser_context,
-    history::HistoryService* history_service)
+    BrowserContext* browser_context)
     : profile_(Profile::FromBrowserContext(browser_context)),
 #if BUILDFLAG(ENABLE_PLUGINS)
       flash_lso_helper_(BrowsingDataFlashLSOHelper::Create(browser_context)),
@@ -336,11 +335,7 @@ ChromeBrowsingDataRemoverDelegate::ChromeBrowsingDataRemoverDelegate(
 #if defined(OS_ANDROID)
       webapp_registry_(new WebappRegistry()),
 #endif
-      history_observer_(this),
       weak_ptr_factory_(this) {
-  if (history_service) {
-    history_observer_.Add(history_service);
-  }
 }
 
 ChromeBrowsingDataRemoverDelegate::~ChromeBrowsingDataRemoverDelegate() {}
@@ -355,15 +350,6 @@ ChromeBrowsingDataRemoverDelegate::GetOriginTypeMatcher() const {
   return base::BindRepeating(&DoesOriginMatchEmbedderMask);
 }
 
-void ChromeBrowsingDataRemoverDelegate::OnURLsDeleted(
-    history::HistoryService* history_service,
-    const history::DeletionTimeRange& time_range,
-    bool expired,
-    const history::URLRows& deleted_rows,
-    const std::set<GURL>& favicon_urls) {
-  if (!expired && !profile_->IsGuestSession())
-    browsing_data::RemoveNavigationEntries(profile_, time_range, deleted_rows);
-}
 
 bool ChromeBrowsingDataRemoverDelegate::MayRemoveDownloadHistory() const {
   return profile_->GetPrefs()->GetBoolean(prefs::kAllowDeletingBrowserHistory);

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h

@@ -11,13 +11,11 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
-#include "base/scoped_observer.h"
 #include "base/synchronization/waitable_event_watcher.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "build/build_config.h"
 #include "chrome/common/buildflags.h"
 #include "components/browsing_data/core/browsing_data_utils.h"
-#include "components/history/core/browser/history_service_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/nacl/common/buildflags.h"
 #include "components/offline_pages/core/offline_page_model.h"
@@ -45,7 +43,6 @@ class PluginDataRemover;
 // as the embedder.
 class ChromeBrowsingDataRemoverDelegate
     : public content::BrowsingDataRemoverDelegate,
-      public history::HistoryServiceObserver,
       public KeyedService
 #if BUILDFLAG(ENABLE_PLUGINS)
     ,
@@ -151,8 +148,7 @@ class ChromeBrowsingDataRemoverDelegate
   static_assert((IMPORTANT_SITES_DATA_TYPES & ~FILTERABLE_DATA_TYPES) == 0,
                 "All important sites datatypes must be filterable.");
 
-  ChromeBrowsingDataRemoverDelegate(content::BrowserContext* browser_context,
-                                    history::HistoryService* history_service);
+  ChromeBrowsingDataRemoverDelegate(content::BrowserContext* browser_context);
   ~ChromeBrowsingDataRemoverDelegate() override;
 
   // KeyedService:
@@ -170,13 +166,6 @@ class ChromeBrowsingDataRemoverDelegate
       int origin_type_mask,
       base::OnceClosure callback) override;
 
-  // history::HistoryServiceObserver:
-  void OnURLsDeleted(history::HistoryService* history_service,
-                     const history::DeletionTimeRange& time_range,
-                     bool expired,
-                     const history::URLRows& deleted_rows,
-                     const std::set<GURL>& favicon_urls) override;
-
 #if defined(OS_ANDROID)
   void OverrideWebappRegistryForTesting(
       std::unique_ptr<WebappRegistry> webapp_registry);
@@ -269,9 +258,6 @@ class ChromeBrowsingDataRemoverDelegate
   std::unique_ptr<WebappRegistry> webapp_registry_;
 #endif
 
-  ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
-      history_observer_;
-
   base::WeakPtrFactory<ChromeBrowsingDataRemoverDelegate> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowsingDataRemoverDelegate);

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_factory.cc

@@ -95,8 +95,5 @@ ChromeBrowsingDataRemoverDelegateFactory::GetBrowserContextToUse(
 
 KeyedService* ChromeBrowsingDataRemoverDelegateFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  Profile* profile = Profile::FromBrowserContext(context);
-  auto* history_service = HistoryServiceFactory::GetForProfile(
-      profile, ServiceAccessType::EXPLICIT_ACCESS);
-  return new ChromeBrowsingDataRemoverDelegate(context, history_service);
+  return new ChromeBrowsingDataRemoverDelegate(context);
 }

chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/background/background_contents_service_factory.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#include "chrome/browser/browsing_data/browsing_data_history_observer_service.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_factory.h"
 #include "chrome/browser/chrome_browser_main.h"
 #include "chrome/browser/consent_auditor/consent_auditor_factory.h"
@@ -215,6 +216,7 @@ EnsureBrowserContextKeyedServiceFactoriesBuilt() {
 #endif
   BookmarkModelFactory::GetInstance();
   BookmarkUndoServiceFactory::GetInstance();
+  BrowsingDataHistoryObserverService::Factory::GetInstance();
   browser_sync::UserEventServiceFactory::GetInstance();
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
   CaptivePortalServiceFactory::GetInstance();

chrome/browser/ui/webui/browsing_history_handler.cc

@@ -243,9 +243,6 @@ void BrowsingHistoryHandler::RegisterMessages() {
       ProfileSyncServiceFactory::GetSyncServiceForBrowserContext(profile);
   browsing_history_service_ = std::make_unique<BrowsingHistoryService>(
       this, local_history, sync_service);
-  // Make sure BrowsingDataRemoverDelegate is initialized and listening
-  // for history deletions.
-  profile->GetBrowsingDataRemoverDelegate();
 
   // Create our favicon data source.
   content::URLDataSource::Add(profile, new FaviconSource(profile));