[Media History] Reset the database if history is erased

If we erase all of the history we should reset the
entire Media History database. We do this by deleting
the database and creating a new one.

BUG=1024352

Change-Id: I14589cdee5ed8b807d0463f1efb5ccfb6d7e7d9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1990218
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737522}

chrome/browser/media/history/media_history_contents_observer.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/media/history/media_history_contents_observer.h"
 
 #include "chrome/browser/media/history/media_history_keyed_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/media_player_watch_time.h"
 #include "content/public/browser/media_session.h"

chrome/browser/media/history/media_history_keyed_service.cc

@@ -6,22 +6,31 @@
 
 #include "base/feature_list.h"
 #include "base/task/post_task.h"
+#include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/media/history/media_history_keyed_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/history/core/browser/history_service.h"
 #include "content/public/browser/browser_context.h"
 #include "media/base/media_switches.h"
 
 namespace media_history {
 
-MediaHistoryKeyedService::MediaHistoryKeyedService(
-    content::BrowserContext* browser_context) {
-  DCHECK(!browser_context->IsOffTheRecord());
+MediaHistoryKeyedService::MediaHistoryKeyedService(Profile* profile)
+    : profile_(profile) {
+  DCHECK(!profile->IsOffTheRecord());
+
+  // May be null in tests.
+  history::HistoryService* history = HistoryServiceFactory::GetForProfile(
+      profile, ServiceAccessType::IMPLICIT_ACCESS);
+  if (history)
+    history->AddObserver(this);
 
   auto db_task_runner = base::CreateUpdateableSequencedTaskRunner(
       {base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE,
        base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
 
-  media_history_store_ = std::make_unique<MediaHistoryStore>(
-      Profile::FromBrowserContext(browser_context), std::move(db_task_runner));
+  media_history_store_ =
+      std::make_unique<MediaHistoryStore>(profile_, std::move(db_task_runner));
 }
 
 // static
@@ -35,4 +44,23 @@ bool MediaHistoryKeyedService::IsEnabled() {
   return base::FeatureList::IsEnabled(media::kUseMediaHistoryStore);
 }
 
+void MediaHistoryKeyedService::Shutdown() {
+  history::HistoryService* history = HistoryServiceFactory::GetForProfile(
+      profile_, ServiceAccessType::IMPLICIT_ACCESS);
+  if (history)
+    history->RemoveObserver(this);
+}
+
+void MediaHistoryKeyedService::OnURLsDeleted(
+    history::HistoryService* history_service,
+    const history::DeletionInfo& deletion_info) {
+  if (!deletion_info.IsAllHistory()) {
+    // TODO(https://crbug.com/1024352): Handle fine-grained history deletion.
+    return;
+  }
+
+  // Destroy the old database and create a new one.
+  media_history_store_->EraseDatabaseAndCreateNew();
+}
+
 }  // namespace media_history

chrome/browser/media/history/media_history_keyed_service.h

@@ -7,17 +7,21 @@
 
 #include "base/macros.h"
 #include "chrome/browser/media/history/media_history_store.h"
+#include "components/history/core/browser/history_service_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 
-namespace content {
-class BrowserContext;
-}  // namespace content
+class Profile;
+
+namespace history {
+class HistoryService;
+}  // namespace history
 
 namespace media_history {
 
-class MediaHistoryKeyedService : public KeyedService {
+class MediaHistoryKeyedService : public KeyedService,
+                                 public history::HistoryServiceObserver {
  public:
-  explicit MediaHistoryKeyedService(content::BrowserContext* browser_context);
+  explicit MediaHistoryKeyedService(Profile* profile);
   ~MediaHistoryKeyedService() override;
 
   static bool IsEnabled();
@@ -29,9 +33,18 @@ class MediaHistoryKeyedService : public KeyedService {
     return media_history_store_.get();
   }
 
+  // Overridden from KeyedService:
+  void Shutdown() override;
+
+  // Overridden from history::HistoryServiceObserver:
+  void OnURLsDeleted(history::HistoryService* history_service,
+                     const history::DeletionInfo& deletion_info) override;
+
  private:
   std::unique_ptr<MediaHistoryStore> media_history_store_;
 
+  Profile* profile_;
+
   DISALLOW_COPY_AND_ASSIGN(MediaHistoryKeyedService);
 };
 

chrome/browser/media/history/media_history_keyed_service_factory.cc

@@ -5,7 +5,9 @@
 #include "chrome/browser/media/history/media_history_keyed_service_factory.h"
 
 #include "base/logging.h"
+#include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/media/history/media_history_keyed_service.h"
+#include "chrome/browser/profiles/profile.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "content/public/browser/browser_context.h"
 
@@ -31,7 +33,9 @@ MediaHistoryKeyedServiceFactory::GetInstance() {
 MediaHistoryKeyedServiceFactory::MediaHistoryKeyedServiceFactory()
     : BrowserContextKeyedServiceFactory(
           "MediaHistoryKeyedService",
-          BrowserContextDependencyManager::GetInstance()) {}
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(HistoryServiceFactory::GetInstance());
+}
 
 MediaHistoryKeyedServiceFactory::~MediaHistoryKeyedServiceFactory() = default;
 
@@ -44,7 +48,7 @@ KeyedService* MediaHistoryKeyedServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   DCHECK(!context->IsOffTheRecord());
 
-  return new MediaHistoryKeyedService(context);
+  return new MediaHistoryKeyedService(Profile::FromBrowserContext(context));
 }
 
 }  // namespace media_history

chrome/browser/media/history/media_history_keyed_service_unittest.cc

+// Copyright 2019 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/media/history/media_history_keyed_service.h"
+
+#include <memory>
+
+#include "base/bind.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/run_loop.h"
+#include "base/test/gmock_callback_support.h"
+#include "base/test/mock_callback.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/test_mock_time_task_runner.h"
+#include "build/build_config.h"
+#include "chrome/browser/history/history_service_factory.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/history/core/browser/history_database_params.h"
+#include "components/history/core/browser/history_service.h"
+#include "components/history/core/test/test_history_database.h"
+#include "content/public/browser/media_player_watch_time.h"
+#include "content/public/test/test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media_history {
+
+namespace {
+
+base::FilePath g_temp_history_dir;
+
+std::unique_ptr<KeyedService> BuildTestHistoryService(
+    scoped_refptr<base::SequencedTaskRunner> backend_runner,
+    content::BrowserContext* context) {
+  std::unique_ptr<history::HistoryService> service(
+      new history::HistoryService());
+  service->set_backend_task_runner_for_testing(std::move(backend_runner));
+  service->Init(history::TestHistoryDatabaseParamsForPath(g_temp_history_dir));
+  return service;
+}
+
+}  // namespace
+
+class MediaHistoryKeyedServiceTest : public ChromeRenderViewHostTestHarness {
+ public:
+  void SetUp() override {
+    scoped_feature_list_.InitWithFeatures(
+        {history::HistoryService::kHistoryServiceUsesTaskScheduler}, {});
+
+    ChromeRenderViewHostTestHarness::SetUp();
+
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    g_temp_history_dir = temp_dir_.GetPath();
+
+    mock_time_task_runner_ =
+        base::MakeRefCounted<base::TestMockTimeTaskRunner>();
+
+    HistoryServiceFactory::GetInstance()->SetTestingFactory(
+        profile(),
+        base::BindRepeating(&BuildTestHistoryService, mock_time_task_runner_));
+
+    service_ = std::make_unique<MediaHistoryKeyedService>(profile());
+
+    // Sleep the thread to allow the media history store to asynchronously
+    // create the database and tables.
+    content::RunAllTasksUntilIdle();
+  }
+
+  MediaHistoryKeyedService* service() const { return service_.get(); }
+
+  void ConfigureHistoryService(
+      scoped_refptr<base::SequencedTaskRunner> backend_runner) {
+    HistoryServiceFactory::GetInstance()->SetTestingFactory(
+        profile(), base::BindRepeating(&BuildTestHistoryService,
+                                       std::move(backend_runner)));
+  }
+
+  void TearDown() override {
+    service_->Shutdown();
+
+    // Tests that run a history service that uses the mock task runner for
+    // backend processing will post tasks there during TearDown. Run them now to
+    // avoid leaks.
+    mock_time_task_runner_->RunUntilIdle();
+    service_.reset();
+
+    ChromeRenderViewHostTestHarness::TearDown();
+  }
+
+  int GetUserDataTableRowCount() {
+    int count = 0;
+    mojom::MediaHistoryStatsPtr stats = GetStatsSync();
+
+    for (auto& entry : stats->table_row_counts) {
+      // The meta table should not count as it does not contain any user data.
+      if (entry.first == "meta")
+        continue;
+
+      count += entry.second;
+    }
+
+    return count;
+  }
+
+  scoped_refptr<base::TestMockTimeTaskRunner> mock_time_task_runner_;
+
+ private:
+  mojom::MediaHistoryStatsPtr GetStatsSync() {
+    base::RunLoop run_loop;
+    mojom::MediaHistoryStatsPtr stats_out;
+
+    service()->GetMediaHistoryStore()->GetMediaHistoryStats(base::BindOnce(
+        [](mojom::MediaHistoryStatsPtr* stats_out,
+           base::RepeatingClosure callback, mojom::MediaHistoryStatsPtr stats) {
+          stats_out->Swap(&stats);
+          std::move(callback).Run();
+        },
+        &stats_out, run_loop.QuitClosure()));
+
+    run_loop.Run();
+    return stats_out;
+  }
+
+  base::ScopedTempDir temp_dir_;
+
+  std::unique_ptr<MediaHistoryKeyedService> service_;
+
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+TEST_F(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenHistoryIsDeleted) {
+  history::HistoryService* history = HistoryServiceFactory::GetForProfile(
+      profile(), ServiceAccessType::IMPLICIT_ACCESS);
+  GURL url("http://google.com/test");
+
+  EXPECT_EQ(0, GetUserDataTableRowCount());
+
+  // Record a playback in the database.
+  {
+    content::MediaPlayerWatchTime watch_time(
+        url, url.GetOrigin(), base::TimeDelta::FromMilliseconds(123),
+        base::TimeDelta::FromMilliseconds(321), true, false);
+
+    history->AddPage(url, base::Time::Now(), history::SOURCE_BROWSED);
+    service()->GetMediaHistoryStore()->SavePlayback(watch_time);
+
+    // Wait until the playbacks have finished saving.
+    content::RunAllTasksUntilIdle();
+  }
+
+  EXPECT_EQ(2, GetUserDataTableRowCount());
+
+  {
+    base::CancelableTaskTracker task_tracker;
+
+    // Clear all history.
+    history->ExpireHistoryBetween(std::set<GURL>(), base::Time(), base::Time(),
+                                  /* user_initiated */ true, base::DoNothing(),
+                                  &task_tracker);
+
+    mock_time_task_runner_->RunUntilIdle();
+
+    // Wait for the database to update.
+    content::RunAllTasksUntilIdle();
+  }
+
+  EXPECT_EQ(0, GetUserDataTableRowCount());
+}
+
+}  // namespace media_history

chrome/browser/media/history/media_history_store.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/media/history/media_history_store.h"
 
 #include "base/callback.h"
+#include "base/files/file_path.h"
 #include "base/strings/stringprintf.h"
 #include "base/task_runner_util.h"
 #include "chrome/browser/media/history/media_history_engagement_table.h"
@@ -45,7 +46,7 @@ class MediaHistoryStoreInternal
       scoped_refptr<base::UpdateableSequencedTaskRunner> db_task_runner);
   virtual ~MediaHistoryStoreInternal();
 
-  // Opens the database file from the profile path. Separated from the
+  // Opens the database file from the |db_path|. Separated from the
   // constructor to ease construction/destruction of this object on one thread
   // and database access on the DB sequence of |db_task_runner_|.
   void Initialize();
@@ -71,8 +72,10 @@ class MediaHistoryStoreInternal
   GetPlaybackSessions(unsigned int num_sessions,
                       MediaHistoryStore::GetPlaybackSessionsFilter filter);
 
+  void RazeAndClose();
+
   scoped_refptr<base::UpdateableSequencedTaskRunner> db_task_runner_;
-  base::FilePath db_path_;
+  const base::FilePath db_path_;
   std::unique_ptr<sql::Database> db_;
   sql::MetaTable meta_table_;
   scoped_refptr<MediaHistoryEngagementTable> engagement_table_;
@@ -261,10 +264,20 @@ MediaHistoryStoreInternal::GetPlaybackSessions(
   return session_table_->GetPlaybackSessions(num_sessions, std::move(filter));
 }
 
+void MediaHistoryStoreInternal::RazeAndClose() {
+  DCHECK(db_task_runner_->RunsTasksInCurrentSequence());
+
+  if (db_ && db_->is_open())
+    db_->RazeAndClose();
+
+  sql::Database::Delete(db_path_);
+}
+
 MediaHistoryStore::MediaHistoryStore(
     Profile* profile,
     scoped_refptr<base::UpdateableSequencedTaskRunner> db_task_runner)
-    : db_(new MediaHistoryStoreInternal(profile, db_task_runner)) {
+    : db_(new MediaHistoryStoreInternal(profile, db_task_runner)),
+      profile_(profile) {
   db_task_runner->PostTask(
       FROM_HERE, base::BindOnce(&MediaHistoryStoreInternal::Initialize, db_));
 }
@@ -281,6 +294,24 @@ void MediaHistoryStore::SavePlayback(
                                 watch_time));
 }
 
+scoped_refptr<base::UpdateableSequencedTaskRunner>
+MediaHistoryStore::GetDBTaskRunnerForTest() {
+  return db_->db_task_runner_;
+}
+
+void MediaHistoryStore::EraseDatabaseAndCreateNew() {
+  auto db_task_runner = db_->db_task_runner_;
+  auto db_path = db_->db_path_;
+
+  db_task_runner->PostTask(
+      FROM_HERE, base::BindOnce(&MediaHistoryStoreInternal::RazeAndClose, db_));
+
+  // Create a new internal store.
+  db_ = new MediaHistoryStoreInternal(profile_, db_task_runner);
+  db_task_runner->PostTask(
+      FROM_HERE, base::BindOnce(&MediaHistoryStoreInternal::Initialize, db_));
+}
+
 void MediaHistoryStore::GetMediaHistoryStats(
     base::OnceCallback<void(mojom::MediaHistoryStatsPtr)> callback) {
   if (!db_->initialization_successful_)

chrome/browser/media/history/media_history_store.h

@@ -31,7 +31,7 @@ struct MediaPosition;
 
 namespace sql {
 class Database;
-}
+}  // namespace sql
 
 namespace media_history {
 
@@ -78,8 +78,17 @@ class MediaHistoryStore {
       const media_session::MediaMetadata& metadata,
       const base::Optional<media_session::MediaPosition>& position);
 
+  scoped_refptr<base::UpdateableSequencedTaskRunner> GetDBTaskRunnerForTest();
+
+ protected:
+  friend class MediaHistoryKeyedService;
+
+  void EraseDatabaseAndCreateNew();
+
  private:
   scoped_refptr<MediaHistoryStoreInternal> db_;
+
+  Profile* const profile_;
 };
 
 }  // namespace media_history

chrome/browser/media/history/media_history_store_unittest.cc

@@ -47,7 +47,7 @@ class MediaHistoryStoreUnitTest : public testing::Test {
     // Set up the local DB connection used for assertions.
     base::FilePath db_file =
         temp_dir_.GetPath().Append(FILE_PATH_LITERAL("Media History"));
-    EXPECT_TRUE(db_.Open(db_file));
+    ASSERT_TRUE(db_.Open(db_file));
   }
 
   void TearDown() override { content::RunAllTasksUntilIdle(); }

chrome/test/BUILD.gn

@@ -3164,6 +3164,7 @@ test("unit_tests") {
     "../browser/media/android/router/media_router_android_unittest.cc",
     "../browser/media/cast_mirroring_service_host_unittest.cc",
     "../browser/media/history/media_history_keyed_service_factory_unittest.cc",
+    "../browser/media/history/media_history_keyed_service_unittest.cc",
     "../browser/media/history/media_history_store_unittest.cc",
     "../browser/media/media_engagement_contents_observer_unittest.cc",
     "../browser/media/media_engagement_preloaded_list_unittest.cc",