[Subresource Filter]: Abstract history dep from ContentSettingsManager

WebLayer does not use //components/history and does not want to take a
dep on the (large) component, as every dep that WebLayer adds has an
impact on WebView. SubresourceFilterContentSettingsManager, however,
currently depends on //components/history for its optional observance of
the user's deletion of URLs from history.

This CL abstracts that dependency via the following changes:
- Adds the ability for SubresourceFilterProfileContext to store opaque
  embedder data (via a simple
  SubresourceFilterProfileContext::EmbedderData interface)
- Moves the history observance to a new SubresourceFilterHistoryObserver
  class that is an EmbedderData and takes in a
  SubresourceFilterContentSettingsManager instance to call into
- Has SubresourceFilterProfileContextFactory create
  SubresourceFilterHistoryObserver instances to be stored by
  SubresourceFilterProfileContext instances

In this way, WebLayer will be able to use
SubresourceFilterProfileContext, AdsInterventionManager, and
SubresourceFilterContentSettingsManager without taking on the
//components/history dep.

Bug: 1116095
Change-Id: Ic81f63447f10afef1c4946ca7751d810d0a9cbbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466761Reviewed-by: Eric Robinson <ericrobinson@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817020}

chrome/browser/BUILD.gn

@@ -1712,6 +1712,8 @@ static_library("browser") {
     "subresource_filter/chrome_subresource_filter_client.h",
     "subresource_filter/subresource_filter_content_settings_manager.cc",
     "subresource_filter/subresource_filter_content_settings_manager.h",
+    "subresource_filter/subresource_filter_history_observer.cc",
+    "subresource_filter/subresource_filter_history_observer.h",
     "subresource_filter/subresource_filter_profile_context.cc",
     "subresource_filter/subresource_filter_profile_context.h",
     "subresource_filter/subresource_filter_profile_context_factory.cc",

chrome/browser/subresource_filter/subresource_filter_content_settings_manager.cc

@@ -14,7 +14,6 @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_types.h"
-#include "components/keyed_service/core/service_access_type.h"
 #include "url/gurl.h"
 
 namespace {
@@ -41,14 +40,11 @@ constexpr base::TimeDelta
 
 SubresourceFilterContentSettingsManager::
     SubresourceFilterContentSettingsManager(
-        HostContentSettingsMap* settings_map,
-        history::HistoryService* history_service)
+        HostContentSettingsMap* settings_map)
     : settings_map_(settings_map),
       clock_(std::make_unique<base::DefaultClock>(base::DefaultClock())),
       should_use_smart_ui_(ShouldUseSmartUI()) {
   DCHECK(settings_map_);
-  if (history_service)
-    history_observer_.Add(history_service);
 }
 
 SubresourceFilterContentSettingsManager::
@@ -207,23 +203,6 @@ bool SubresourceFilterContentSettingsManager::ShouldDeleteDataWithNoActivation(
   return clock_->Now() > expiry_time;
 }
 
-// When history URLs are deleted, clear the metadata for the smart UI.
-void SubresourceFilterContentSettingsManager::OnURLsDeleted(
-    history::HistoryService* history_service,
-    const history::DeletionInfo& deletion_info) {
-  if (deletion_info.IsAllHistory()) {
-    ClearMetadataForAllSites();
-    return;
-  }
-
-  for (const auto& entry : deletion_info.deleted_urls_origin_map()) {
-    const GURL& origin = entry.first;
-    int remaining_urls = entry.second.first;
-    if (!origin.is_empty() && remaining_urls == 0)
-      ClearSiteMetadata(origin);
-  }
-}
-
 bool SubresourceFilterContentSettingsManager::GetSiteActivationFromMetadata(
     const GURL& url) {
   std::unique_ptr<base::DictionaryValue> dict = GetSiteMetadata(url);

chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h

@@ -9,12 +9,9 @@
 #include <utility>
 
 #include "base/macros.h"
-#include "base/scoped_observer.h"
 #include "base/time/clock.h"
 #include "base/time/time.h"
 #include "components/content_settings/core/common/content_settings.h"
-#include "components/history/core/browser/history_service.h"
-#include "components/history/core/browser/history_service_observer.h"
 
 class GURL;
 class HostContentSettingsMap;
@@ -23,10 +20,6 @@ namespace base {
 class DictionaryValue;
 }  // namespace base
 
-namespace history {
-class HistoryService;
-}
-
 // This class contains helpers to get/set content and website settings related
 // to subresource filtering.
 //
@@ -65,13 +58,11 @@ class HistoryService;
 // content_settings::Observer. Generally speaking, we want a system where we can
 // easily log metrics if the content setting has changed meaningfully from it's
 // previous value.
-class SubresourceFilterContentSettingsManager
-    : public history::HistoryServiceObserver {
+class SubresourceFilterContentSettingsManager {
  public:
-  SubresourceFilterContentSettingsManager(
-      HostContentSettingsMap* settings_map,
-      history::HistoryService* history_service);
-  ~SubresourceFilterContentSettingsManager() override;
+  explicit SubresourceFilterContentSettingsManager(
+      HostContentSettingsMap* settings_map);
+  ~SubresourceFilterContentSettingsManager();
 
   ContentSetting GetSitePermission(const GURL& url) const;
 
@@ -139,10 +130,6 @@ class SubresourceFilterContentSettingsManager
                                  std::unique_ptr<base::DictionaryValue> dict);
 
  private:
-  // history::HistoryServiceObserver:
-  void OnURLsDeleted(history::HistoryService* history_service,
-                     const history::DeletionInfo& deletion_info) override;
-
   void SetSiteMetadata(const GURL& url,
                        std::unique_ptr<base::DictionaryValue> dict);
 
@@ -154,9 +141,6 @@ class SubresourceFilterContentSettingsManager
   bool ShouldDeleteDataWithNoActivation(base::DictionaryValue* dict,
                                         ActivationSource activation_source);
 
-  ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
-      history_observer_{this};
-
   HostContentSettingsMap* settings_map_;
 
   // A clock is injected into this class so tests can set arbitrary timestamps

chrome/browser/subresource_filter/subresource_filter_content_settings_manager_unittest.cc

@@ -15,15 +15,12 @@
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
-#include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
 #include "chrome/browser/subresource_filter/subresource_filter_profile_context.h"
 #include "chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
-#include "components/history/core/browser/history_service.h"
-#include "components/history/core/test/history_service_test_util.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -82,17 +79,6 @@ class SubresourceFilterContentSettingsManagerTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterContentSettingsManagerTest);
 };
 
-// It isn't very cheap to initialize the history service. Tests that need it can
-// use this harness.
-class SubresourceFilterContentSettingsManagerHistoryTest
-    : public SubresourceFilterContentSettingsManagerTest {
- public:
-  void SetUp() override {
-    ASSERT_TRUE(profile()->CreateHistoryService());
-    SubresourceFilterContentSettingsManagerTest::SetUp();
-  }
-};
-
 TEST_F(SubresourceFilterContentSettingsManagerTest, LogDefaultSetting) {
   const char kDefaultContentSetting[] =
       "ContentSettings.DefaultSubresourceFilterSetting";
@@ -359,78 +345,4 @@ TEST_F(SubresourceFilterContentSettingsManagerTest, ClearMetadataForAllSites) {
   EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(different_origin_url));
 }
 
-TEST_F(SubresourceFilterContentSettingsManagerHistoryTest,
-       HistoryUrlDeleted_ClearsWebsiteSetting) {
-  // Simulate a history already populated with a URL.
-  auto* history_service = HistoryServiceFactory::GetForProfile(
-      profile(), ServiceAccessType::EXPLICIT_ACCESS);
-  ASSERT_TRUE(history_service);
-  history_service->AddPage(GURL("https://already-browsed.com/"),
-                           base::Time::Now(), history::SOURCE_BROWSED);
-
-  // Ensure the website setting is set.
-  GURL url1("https://example.test/1");
-  GURL url2("https://example.test/2");
-  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url1));
-  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url2));
-  settings_manager()->OnDidShowUI(url1);
-
-  // Simulate adding two page to the history for example.test.
-  history_service->AddPage(url1, base::Time::Now(), history::SOURCE_BROWSED);
-  history_service->AddPage(url2, base::Time::Now(), history::SOURCE_BROWSED);
-  history::BlockUntilHistoryProcessesPendingRequests(history_service);
-
-  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url1));
-  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url2));
-
-  // Deleting a URL from history while there are still other urls for the
-  // same origin should not delete the setting.
-  history_service->DeleteURLs({url1});
-  history::BlockUntilHistoryProcessesPendingRequests(history_service);
-  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url1));
-  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url2));
-
-  // Deleting all URLs of an origin from history should clear the setting for
-  // this URL. Note that since there is another URL in the history this won't
-  // clear all items.
-  history_service->DeleteURLs({url2});
-  history::BlockUntilHistoryProcessesPendingRequests(history_service);
-
-  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url1));
-  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url2));
-}
-
-TEST_F(SubresourceFilterContentSettingsManagerHistoryTest,
-       AllHistoryUrlDeleted_ClearsWebsiteSetting) {
-  auto* history_service = HistoryServiceFactory::GetForProfile(
-      profile(), ServiceAccessType::EXPLICIT_ACCESS);
-  ASSERT_TRUE(history_service);
-
-  GURL url1("https://example.test");
-  GURL url2("https://example.test");
-  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url1));
-  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url2));
-  settings_manager()->OnDidShowUI(url1);
-  settings_manager()->OnDidShowUI(url2);
-
-  // Simulate adding the pages to the history.
-  history_service->AddPage(url1, base::Time::Now(), history::SOURCE_BROWSED);
-  history_service->AddPage(url2, base::Time::Now(), history::SOURCE_BROWSED);
-  history::BlockUntilHistoryProcessesPendingRequests(history_service);
-
-  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url1));
-  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url2));
-
-  // Deleting all the URLs should clear everything.
-  base::RunLoop run_loop;
-  base::CancelableTaskTracker task_tracker;
-  history_service->ExpireHistoryBetween(std::set<GURL>(), base::Time(),
-                                        base::Time(), /*user_initiated*/ true,
-                                        run_loop.QuitClosure(), &task_tracker);
-  run_loop.Run();
-
-  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url1));
-  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url2));
-}
-
 }  // namespace

chrome/browser/subresource_filter/subresource_filter_history_observer.cc

+// Copyright 2020 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/subresource_filter/subresource_filter_history_observer.h"
+
+#include "chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h"
+#include "url/gurl.h"
+
+SubresourceFilterHistoryObserver::SubresourceFilterHistoryObserver(
+    SubresourceFilterContentSettingsManager* settings_manager,
+    history::HistoryService* history_service)
+    : settings_manager_(settings_manager) {
+  DCHECK(settings_manager_);
+  DCHECK(history_service);
+  history_observer_.Add(history_service);
+}
+
+SubresourceFilterHistoryObserver::~SubresourceFilterHistoryObserver() = default;
+
+// Instructs |settings_manager_| to clear the relevant site metadata on URLs
+// being deleted from history.
+void SubresourceFilterHistoryObserver::OnURLsDeleted(
+    history::HistoryService* history_service,
+    const history::DeletionInfo& deletion_info) {
+  if (deletion_info.IsAllHistory()) {
+    settings_manager_->ClearMetadataForAllSites();
+    return;
+  }
+
+  for (const auto& entry : deletion_info.deleted_urls_origin_map()) {
+    const GURL& origin = entry.first;
+    int remaining_urls = entry.second.first;
+    if (!origin.is_empty() && remaining_urls == 0)
+      settings_manager_->ClearSiteMetadata(origin);
+  }
+}

chrome/browser/subresource_filter/subresource_filter_history_observer.h

+// Copyright 2020 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_SUBRESOURCE_FILTER_SUBRESOURCE_FILTER_HISTORY_OBSERVER_H_
+#define CHROME_BROWSER_SUBRESOURCE_FILTER_SUBRESOURCE_FILTER_HISTORY_OBSERVER_H_
+
+#include "base/scoped_observer.h"
+#include "chrome/browser/subresource_filter/subresource_filter_profile_context.h"
+#include "components/history/core/browser/history_service.h"
+#include "components/history/core/browser/history_service_observer.h"
+
+class SubresourceFilterContentSettingsManager;
+
+namespace history {
+class HistoryService;
+}
+
+// Class that observes user changes to history via the HistoryService and
+// updates subresource filter-related content settings appropriately.
+class SubresourceFilterHistoryObserver
+    : public history::HistoryServiceObserver,
+      public SubresourceFilterProfileContext::EmbedderData {
+ public:
+  // Both |settings_manager| and |history_service| should be non-null.
+  SubresourceFilterHistoryObserver(
+      SubresourceFilterContentSettingsManager* settings_manager,
+      history::HistoryService* history_service);
+  ~SubresourceFilterHistoryObserver() override;
+  SubresourceFilterHistoryObserver(const SubresourceFilterHistoryObserver&) =
+      delete;
+  SubresourceFilterHistoryObserver& operator=(
+      const SubresourceFilterHistoryObserver&) = delete;
+
+ private:
+  // history::HistoryServiceObserver:
+  void OnURLsDeleted(history::HistoryService* history_service,
+                     const history::DeletionInfo& deletion_info) override;
+
+  ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
+      history_observer_{this};
+
+  // Outlives this object.
+  SubresourceFilterContentSettingsManager* settings_manager_;
+};
+
+#endif  // CHROME_BROWSER_SUBRESOURCE_FILTER_SUBRESOURCE_FILTER_HISTORY_OBSERVER_H_

chrome/browser/subresource_filter/subresource_filter_history_observer_unittest.cc

+// Copyright 2020 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 <set>
+#include <string>
+
+#include "base/run_loop.h"
+#include "chrome/browser/history/history_service_factory.h"
+#include "chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h"
+#include "chrome/browser/subresource_filter/subresource_filter_profile_context.h"
+#include "chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/history/core/browser/history_service.h"
+#include "components/history/core/test/history_service_test_util.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace {
+
+// Tests that SubresourceFilterHistoryObserver is operating as expected in the
+// context of //chrome-level setup of SubresourceFilterProfileContext. More of
+// an integration test than a unittest in spirit, but requires unittest
+// constructs such as TestingProfile and HistoryService-related unittest
+// utilities to be able to perform the needed operations on history.
+class SubresourceFilterHistoryObserverTest : public testing::Test {
+ public:
+  SubresourceFilterHistoryObserverTest() = default;
+
+  SubresourceFilterHistoryObserverTest(
+      const SubresourceFilterHistoryObserverTest&) = delete;
+  SubresourceFilterHistoryObserverTest& operator=(
+      const SubresourceFilterHistoryObserverTest&) = delete;
+
+  void SetUp() override {
+    ASSERT_TRUE(profile()->CreateHistoryService());
+    settings_manager_ =
+        SubresourceFilterProfileContextFactory::GetForProfile(&testing_profile_)
+            ->settings_manager();
+    settings_manager_->set_should_use_smart_ui_for_testing(true);
+  }
+
+  SubresourceFilterContentSettingsManager* settings_manager() {
+    return settings_manager_;
+  }
+
+  TestingProfile* profile() { return &testing_profile_; }
+
+ private:
+  content::BrowserTaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+  TestingProfile testing_profile_;
+
+  // Owned by the testing_profile_.
+  SubresourceFilterContentSettingsManager* settings_manager_ = nullptr;
+};
+
+// Tests that SubresourceFilterHistoryObserver observes deletions of URLs from
+// history and instructs SubresourceFilterContentSettingsManager to delete the
+// appropriate site metadata from content settings (if any).
+TEST_F(SubresourceFilterHistoryObserverTest,
+       HistoryUrlDeleted_ClearsWebsiteSetting) {
+  // Simulate a history already populated with a URL.
+  auto* history_service = HistoryServiceFactory::GetForProfile(
+      profile(), ServiceAccessType::EXPLICIT_ACCESS);
+  ASSERT_TRUE(history_service);
+  history_service->AddPage(GURL("https://already-browsed.com/"),
+                           base::Time::Now(), history::SOURCE_BROWSED);
+
+  // Ensure the website setting is set.
+  GURL url1("https://example.test/1");
+  GURL url2("https://example.test/2");
+  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url1));
+  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url2));
+  settings_manager()->OnDidShowUI(url1);
+
+  // Simulate adding two page to the history for example.test.
+  history_service->AddPage(url1, base::Time::Now(), history::SOURCE_BROWSED);
+  history_service->AddPage(url2, base::Time::Now(), history::SOURCE_BROWSED);
+  history::BlockUntilHistoryProcessesPendingRequests(history_service);
+
+  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url1));
+  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url2));
+
+  // Deleting a URL from history while there are still other urls for the
+  // same origin should not delete the setting.
+  history_service->DeleteURLs({url1});
+  history::BlockUntilHistoryProcessesPendingRequests(history_service);
+  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url1));
+  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url2));
+
+  // Deleting all URLs of an origin from history should clear the setting for
+  // this URL. Note that since there is another URL in the history this won't
+  // clear all items.
+  history_service->DeleteURLs({url2});
+  history::BlockUntilHistoryProcessesPendingRequests(history_service);
+
+  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url1));
+  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url2));
+}
+
+// Tests that SubresourceFilterHistoryObserver observes deletions of all URLs
+// from history and instructs SubresourceFilterContentSettingsManager to delete
+// all site metadata from content settings.
+TEST_F(SubresourceFilterHistoryObserverTest,
+       AllHistoryUrlDeleted_ClearsWebsiteSetting) {
+  auto* history_service = HistoryServiceFactory::GetForProfile(
+      profile(), ServiceAccessType::EXPLICIT_ACCESS);
+  ASSERT_TRUE(history_service);
+
+  GURL url1("https://example.test");
+  GURL url2("https://example.test");
+  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url1));
+  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url2));
+  settings_manager()->OnDidShowUI(url1);
+  settings_manager()->OnDidShowUI(url2);
+
+  // Simulate adding the pages to the history.
+  history_service->AddPage(url1, base::Time::Now(), history::SOURCE_BROWSED);
+  history_service->AddPage(url2, base::Time::Now(), history::SOURCE_BROWSED);
+  history::BlockUntilHistoryProcessesPendingRequests(history_service);
+
+  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url1));
+  EXPECT_FALSE(settings_manager()->ShouldShowUIForSite(url2));
+
+  // Deleting all the URLs should clear everything.
+  base::RunLoop run_loop;
+  base::CancelableTaskTracker task_tracker;
+  history_service->ExpireHistoryBetween(std::set<GURL>(), base::Time(),
+                                        base::Time(), /*user_initiated*/ true,
+                                        run_loop.QuitClosure(), &task_tracker);
+  run_loop.Run();
+
+  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url1));
+  EXPECT_TRUE(settings_manager()->ShouldShowUIForSite(url2));
+}
+
+}  // namespace

chrome/browser/subresource_filter/subresource_filter_profile_context.cc

@@ -8,17 +8,22 @@
 #include "chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h"
 
 SubresourceFilterProfileContext::SubresourceFilterProfileContext(
-    HostContentSettingsMap* settings_map,
-    history::HistoryService* history_service)
+    HostContentSettingsMap* settings_map)
     : settings_manager_(
           std::make_unique<SubresourceFilterContentSettingsManager>(
-              settings_map,
-              history_service)),
+              settings_map)),
       ads_intervention_manager_(
           std::make_unique<AdsInterventionManager>(settings_manager_.get())) {}
 
 SubresourceFilterProfileContext::~SubresourceFilterProfileContext() {}
 
+void SubresourceFilterProfileContext::SetEmbedderData(
+    std::unique_ptr<SubresourceFilterProfileContext::EmbedderData>
+        embedder_data) {
+  DCHECK(!embedder_data_);
+  embedder_data_ = std::move(embedder_data);
+}
+
 void SubresourceFilterProfileContext::Shutdown() {
   settings_manager_.reset();
   ads_intervention_manager_.reset();

chrome/browser/subresource_filter/subresource_filter_profile_context.h

@@ -14,15 +14,18 @@ class HostContentSettingsMap;
 class SubresourceFilterContentSettingsManager;
 class AdsInterventionManager;
 
-namespace history {
-class HistoryService;
-}
-
 // This class holds profile scoped context for subresource filtering.
 class SubresourceFilterProfileContext : public KeyedService {
  public:
-  SubresourceFilterProfileContext(HostContentSettingsMap* settings_map,
-                                  history::HistoryService* history_service);
+  // An opaque class that the embedder can use to scope an embedder-level object
+  // to SubresourceFilterProfileContext via SetEmbedderData().
+  class EmbedderData {
+   public:
+    virtual ~EmbedderData() = default;
+  };
+
+  explicit SubresourceFilterProfileContext(
+      HostContentSettingsMap* settings_map);
   ~SubresourceFilterProfileContext() override;
 
   SubresourceFilterContentSettingsManager* settings_manager() {
@@ -33,6 +36,11 @@ class SubresourceFilterProfileContext : public KeyedService {
     return ads_intervention_manager_.get();
   }
 
+  // Can be used to attach an embedder-level object to this object. Can only be
+  // invoked once. |embedder_data| will be destroyed before the other objects
+  // owned by this object, and thus it can safely depend on those other objects.
+  void SetEmbedderData(std::unique_ptr<EmbedderData> embedder_data);
+
  private:
   // KeyedService:
   void Shutdown() override;
@@ -43,6 +51,10 @@ class SubresourceFilterProfileContext : public KeyedService {
   // navigations.
   std::unique_ptr<AdsInterventionManager> ads_intervention_manager_;
 
+  // NOTE: Declared after the objects above to ensure that it is destroyed
+  // before them.
+  std::unique_ptr<EmbedderData> embedder_data_;
+
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterProfileContext);
 };
 

chrome/browser/subresource_filter/subresource_filter_profile_context_factory.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/subresource_filter/subresource_filter_history_observer.h"
 #include "chrome/browser/subresource_filter/subresource_filter_profile_context.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -38,10 +39,21 @@ KeyedService* SubresourceFilterProfileContextFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = Profile::FromBrowserContext(context);
 
-  return new SubresourceFilterProfileContext(
-      HostContentSettingsMapFactory::GetForProfile(profile),
-      HistoryServiceFactory::GetForProfile(profile,
-                                           ServiceAccessType::EXPLICIT_ACCESS));
+  auto* subresource_filter_profile_context =
+      new SubresourceFilterProfileContext(
+          HostContentSettingsMapFactory::GetForProfile(profile));
+
+  // Create and attach a SubresourceFilterHistoryObserver instance if possible.
+  auto* history_service = HistoryServiceFactory::GetForProfile(
+      profile, ServiceAccessType::EXPLICIT_ACCESS);
+  if (history_service) {
+    subresource_filter_profile_context->SetEmbedderData(
+        std::make_unique<SubresourceFilterHistoryObserver>(
+            subresource_filter_profile_context->settings_manager(),
+            history_service));
+  }
+
+  return subresource_filter_profile_context;
 }
 
 content::BrowserContext*

chrome/test/BUILD.gn

@@ -3682,6 +3682,7 @@ test("unit_tests") {
     "../browser/subresource_filter/subresource_filter_abusive_unittest.cc",
     "../browser/subresource_filter/subresource_filter_configuration_unittest.cc",
     "../browser/subresource_filter/subresource_filter_content_settings_manager_unittest.cc",
+    "../browser/subresource_filter/subresource_filter_history_observer_unittest.cc",
     "../browser/subresource_filter/subresource_filter_test_harness.cc",
     "../browser/subresource_filter/subresource_filter_test_harness.h",
     "../browser/subresource_filter/subresource_filter_unittest.cc",