Add the leveldb site characteristic DB

This is an initial implementation that currently requires that all
tests run on a thread allowed to do IO operations. A follow-up CL will
focus on moving all the blocking operations to a separate thread.

BUG=773382

Change-Id: Iea5ee2ede75f4fff168f6546b515018f0fd2a0ca
Reviewed-on: https://chromium-review.googlesource.com/1005444
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557611}

chrome/browser/BUILD.gn

@@ -2637,6 +2637,8 @@ jumbo_split_static_library("browser") {
       "resource_coordinator/discard_metrics_lifecycle_unit_observer.cc",
       "resource_coordinator/discard_metrics_lifecycle_unit_observer.h",
       "resource_coordinator/discard_reason.h",
+      "resource_coordinator/leveldb_site_characteristics_database.cc",
+      "resource_coordinator/leveldb_site_characteristics_database.h",
       "resource_coordinator/lifecycle_unit.cc",
       "resource_coordinator/lifecycle_unit.h",
       "resource_coordinator/lifecycle_unit_base.cc",
@@ -2655,6 +2657,7 @@ jumbo_split_static_library("browser") {
       "resource_coordinator/local_site_characteristics_data_store.h",
       "resource_coordinator/local_site_characteristics_data_writer.cc",
       "resource_coordinator/local_site_characteristics_data_writer.h",
+      "resource_coordinator/local_site_characteristics_database.h",
       "resource_coordinator/local_site_characteristics_feature_usage.h",
       "resource_coordinator/local_site_characteristics_non_recording_data_store.cc",
       "resource_coordinator/local_site_characteristics_non_recording_data_store.h",

chrome/browser/resource_coordinator/leveldb_site_characteristics_database.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/resource_coordinator/leveldb_site_characteristics_database.h"
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/threading/thread_restrictions.h"
+#include "third_party/leveldatabase/env_chromium.h"
+#include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
+
+namespace resource_coordinator {
+
+namespace {
+
+// Try to open a database (creates it if necessary).
+// TODO(sebmarchand): Turn this into a callback that runs in TaskScheduler with
+// MayBlock().
+std::unique_ptr<leveldb::DB> OpenDatabase(const base::FilePath& db_path) {
+  base::AssertBlockingAllowed();
+  leveldb_env::Options options;
+  options.create_if_missing = true;
+  std::unique_ptr<leveldb::DB> db;
+  leveldb::Status status =
+      leveldb_env::OpenDB(options, db_path.AsUTF8Unsafe(), &db);
+  // TODO(sebmarchand): Do more validation here, try to repair the database if
+  // it's corrupt and report some metrics.
+  if (!status.ok()) {
+    LOG(ERROR) << "Unable to open the Site Characteristics database: "
+               << status.ToString();
+    return nullptr;
+  }
+  return db;
+}
+
+}  // namespace
+
+// static:
+std::unique_ptr<LevelDBSiteCharacteristicsDatabase>
+LevelDBSiteCharacteristicsDatabase::OpenOrCreateDatabase(
+    const base::FilePath& db_path) {
+  std::unique_ptr<leveldb::DB> db = OpenDatabase(db_path);
+  if (!db)
+    return nullptr;
+  LevelDBSiteCharacteristicsDatabase* characteristics_db =
+      new LevelDBSiteCharacteristicsDatabase(std::move(db), db_path);
+  return base::WrapUnique(characteristics_db);
+}
+
+LevelDBSiteCharacteristicsDatabase::~LevelDBSiteCharacteristicsDatabase() =
+    default;
+
+void LevelDBSiteCharacteristicsDatabase::ReadSiteCharacteristicsFromDB(
+    const std::string& site_origin,
+    LocalSiteCharacteristicsDatabase::ReadSiteCharacteristicsFromDBCallback
+        callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  base::AssertBlockingAllowed();
+  std::string protobuf_value;
+  // TODO(sebmarchand): Move this to a separate thread as this is blocking.
+  leveldb::Status s = db_->Get(read_options_, site_origin, &protobuf_value);
+  base::Optional<SiteCharacteristicsProto> site_characteristic_proto;
+  if (s.ok()) {
+    site_characteristic_proto = SiteCharacteristicsProto();
+    if (!site_characteristic_proto->ParseFromString(protobuf_value)) {
+      site_characteristic_proto = base::nullopt;
+      LOG(ERROR) << "Error while trying to parse a SiteCharacteristicsProto "
+                 << "protobuf.";
+    }
+  }
+  std::move(callback).Run(std::move(site_characteristic_proto));
+}
+
+void LevelDBSiteCharacteristicsDatabase::WriteSiteCharacteristicsIntoDB(
+    const std::string& site_origin,
+    const SiteCharacteristicsProto& site_characteristic_proto) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  base::AssertBlockingAllowed();
+  // TODO(sebmarchand): Move this to a separate thread as this is blocking.
+  leveldb::Status s = db_->Put(write_options_, site_origin,
+                               site_characteristic_proto.SerializeAsString());
+  if (!s.ok()) {
+    LOG(ERROR) << "Error while inserting an element in the site characteristic "
+               << "database: " << s.ToString();
+  }
+}
+
+void LevelDBSiteCharacteristicsDatabase::RemoveSiteCharacteristicsFromDB(
+    const std::vector<std::string>& site_origins) {
+  base::AssertBlockingAllowed();
+  // TODO(sebmarchand): Move this to a separate thread as this is blocking.
+  leveldb::WriteBatch batch;
+  for (const auto iter : site_origins)
+    batch.Delete(iter);
+  leveldb::Status status = db_->Write(write_options_, &batch);
+  if (!status.ok()) {
+    LOG(WARNING) << "Failed to remove some entries from the site "
+                 << "characteristics database: " << status.ToString();
+  }
+}
+
+void LevelDBSiteCharacteristicsDatabase::ClearDatabase() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  base::AssertBlockingAllowed();
+  // TODO(sebmarchand): Move this to a separate thread as this is blocking.
+  leveldb_env::Options options;
+  db_.reset();
+  leveldb::Status status = leveldb::DestroyDB(db_path_.AsUTF8Unsafe(), options);
+  if (status.ok()) {
+    db_ = OpenDatabase(db_path_);
+  } else {
+    LOG(WARNING) << "Failed to destroy the site characteristics database: "
+                 << status.ToString();
+  }
+}
+
+LevelDBSiteCharacteristicsDatabase::LevelDBSiteCharacteristicsDatabase(
+    std::unique_ptr<leveldb::DB> db,
+    const base::FilePath& db_path)
+    : db_(std::move(db)), db_path_(db_path) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // Setting |sync| to false might cause some data loss if the system crashes
+  // but it'll make the write operations faster (no data will be loss if only
+  // the process crashes).
+  write_options_.sync = false;
+}
+
+}  // namespace resource_coordinator

chrome/browser/resource_coordinator/leveldb_site_characteristics_database.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_RESOURCE_COORDINATOR_LEVELDB_SITE_CHARACTERISTICS_DATABASE_H_
+#define CHROME_BROWSER_RESOURCE_COORDINATOR_LEVELDB_SITE_CHARACTERISTICS_DATABASE_H_
+
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/sequence_checker.h"
+#include "chrome/browser/resource_coordinator/local_site_characteristics_database.h"
+#include "third_party/leveldatabase/src/include/leveldb/db.h"
+
+namespace resource_coordinator {
+
+// Manages a LevelDB database used by a site characteristic data store.
+// TODO(sebmarchand):
+//   - Constraint the size of the database: Use a background task to trim the
+//     database if it becomes too big and ensure that this fail nicely when the
+//     disk is full.
+//   - Batch the write operations to reduce the number of I/O events.
+//   - Move the I/O operations to a different thread.
+//
+// All the DB operations need to be done on the I/O thread.
+class LevelDBSiteCharacteristicsDatabase
+    : public LocalSiteCharacteristicsDatabase {
+ public:
+  // Initialize the database, create it if it doesn't exist or just open it if
+  // it does. The on-disk database will be stored in |db_path|.
+  static std::unique_ptr<LevelDBSiteCharacteristicsDatabase>
+  OpenOrCreateDatabase(const base::FilePath& db_path);
+
+  ~LevelDBSiteCharacteristicsDatabase() override;
+
+  // LocalSiteCharacteristicDatabase:
+  void ReadSiteCharacteristicsFromDB(
+      const std::string& site_origin,
+      LocalSiteCharacteristicsDatabase::ReadSiteCharacteristicsFromDBCallback
+          callback) override;
+  void WriteSiteCharacteristicsIntoDB(
+      const std::string& site_origin,
+      const SiteCharacteristicsProto& site_characteristic_proto) override;
+  void RemoveSiteCharacteristicsFromDB(
+      const std::vector<std::string>& site_origin) override;
+  void ClearDatabase() override;
+
+ private:
+  LevelDBSiteCharacteristicsDatabase(std::unique_ptr<leveldb::DB> db,
+                                     const base::FilePath& db_path);
+
+  // The connection to the LevelDB database.
+  std::unique_ptr<leveldb::DB> db_;
+  // The options to be used for all database read operations.
+  leveldb::ReadOptions read_options_;
+  // The options to be used for all database write operations.
+  leveldb::WriteOptions write_options_;
+
+  // The on disk location of the database.
+  const base::FilePath db_path_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+  DISALLOW_COPY_AND_ASSIGN(LevelDBSiteCharacteristicsDatabase);
+};
+
+}  // namespace resource_coordinator
+
+#endif  // CHROME_BROWSER_RESOURCE_COORDINATOR_LEVELDB_SITE_CHARACTERISTICS_DATABASE_H_

chrome/browser/resource_coordinator/leveldb_site_characteristics_database_unittest.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/resource_coordinator/leveldb_site_characteristics_database.h"
+
+#include "base/bind.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/resource_coordinator/site_characteristics.pb.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace resource_coordinator {
+
+namespace {
+
+const char kOrigin1[] = "foo.com";
+
+// Initialize a SiteCharacteristicsProto object with a test value (the same
+// value is used to initialize all fields).
+void InitSiteCharacteristicProto(SiteCharacteristicsProto* proto,
+                                 ::google::protobuf::int64 test_value) {
+  proto->set_last_loaded(test_value);
+
+  SiteCharacteristicsFeatureProto feature_proto;
+  feature_proto.set_observation_duration(test_value);
+  feature_proto.set_use_timestamp(test_value);
+
+  proto->mutable_updates_favicon_in_background()->CopyFrom(feature_proto);
+  proto->mutable_updates_title_in_background()->CopyFrom(feature_proto);
+  proto->mutable_uses_notifications_in_background()->CopyFrom(feature_proto);
+  proto->mutable_uses_audio_in_background()->CopyFrom(feature_proto);
+}
+
+}  // namespace
+
+class LevelDBSiteCharacteristicsDatabaseTest : public ::testing::Test {
+ public:
+  LevelDBSiteCharacteristicsDatabaseTest() {}
+
+  void SetUp() override {
+    EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
+    db_ = LevelDBSiteCharacteristicsDatabase::OpenOrCreateDatabase(
+        temp_dir_.GetPath());
+    EXPECT_TRUE(db_);
+  }
+
+  void TearDown() override {
+    db_.reset();
+    EXPECT_TRUE(temp_dir_.Delete());
+  }
+
+ protected:
+  // Try to read an entry from the database, returns true if the entry is
+  // present and false otherwise. |receiving_proto| will receive the protobuf
+  // corresponding to this entry on success.
+  bool ReadFromDB(const std::string& origin,
+                  SiteCharacteristicsProto* receiving_proto) {
+    EXPECT_TRUE(receiving_proto);
+    bool success = false;
+    bool init_called = false;
+    auto init_callback = base::BindOnce(
+        [](SiteCharacteristicsProto* proto, bool* res, bool* init_called,
+           base::Optional<SiteCharacteristicsProto> proto_opt) {
+          *res = proto_opt.has_value();
+          *init_called = true;
+          if (proto_opt)
+            proto->CopyFrom(proto_opt.value());
+        },
+        base::Unretained(receiving_proto), base::Unretained(&success),
+        base::Unretained(&init_called));
+    db_->ReadSiteCharacteristicsFromDB(origin, std::move(init_callback));
+    EXPECT_TRUE(init_called);
+    return success;
+  }
+
+  base::ScopedTempDir temp_dir_;
+  std::unique_ptr<LevelDBSiteCharacteristicsDatabase> db_;
+};
+
+TEST_F(LevelDBSiteCharacteristicsDatabaseTest, InitAndStoreSiteCharacteristic) {
+  // Initializing an entry that doesn't exist in the database should fail.
+  SiteCharacteristicsProto early_read_proto;
+  EXPECT_FALSE(ReadFromDB(kOrigin1, &early_read_proto));
+
+  // Add an entry to the database and make sure that we can read it back.
+  ::google::protobuf::int64 test_value = 42;
+  SiteCharacteristicsProto stored_proto;
+  InitSiteCharacteristicProto(&stored_proto, test_value);
+  db_->WriteSiteCharacteristicsIntoDB(kOrigin1, stored_proto);
+  SiteCharacteristicsProto read_proto;
+  EXPECT_TRUE(ReadFromDB(kOrigin1, &read_proto));
+  EXPECT_TRUE(read_proto.IsInitialized());
+  EXPECT_EQ(stored_proto.SerializeAsString(), read_proto.SerializeAsString());
+}
+
+TEST_F(LevelDBSiteCharacteristicsDatabaseTest, RemoveEntries) {
+  // Add multiple origins to the database.
+  const size_t kEntryCount = 10;
+  SiteCharacteristicsProto protos[kEntryCount];
+  std::vector<std::string> site_origins;
+  for (size_t i = 0; i < kEntryCount; ++i) {
+    std::string site_origin = base::StringPrintf("%zu.com", i);
+    InitSiteCharacteristicProto(&protos[i],
+                                static_cast<::google::protobuf::int64>(i));
+    db_->WriteSiteCharacteristicsIntoDB(site_origin, protos[i]);
+    site_origins.emplace_back(site_origin);
+  }
+
+  for (size_t i = 0; i < kEntryCount; ++i)
+    EXPECT_TRUE(protos[i].IsInitialized());
+
+  // Remove half the origins from the database.
+  std::vector<std::string> site_origins_to_remove(
+      site_origins.begin(), site_origins.begin() + kEntryCount / 2);
+  db_->RemoveSiteCharacteristicsFromDB(site_origins_to_remove);
+
+  // Verify that the origins were removed correctly.
+  SiteCharacteristicsProto proto_temp;
+  for (const auto& iter : site_origins_to_remove)
+    EXPECT_FALSE(ReadFromDB(iter, &proto_temp));
+
+  for (auto iter = site_origins.begin() + kEntryCount / 2;
+       iter != site_origins.end(); ++iter) {
+    EXPECT_TRUE(ReadFromDB(*iter, &proto_temp));
+  }
+
+  // Clear the database.
+  db_->ClearDatabase();
+
+  // Verify that no origin remains.
+  for (auto iter : site_origins)
+    EXPECT_FALSE(ReadFromDB(iter, &proto_temp));
+}
+
+}  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_impl.cc

@@ -7,6 +7,8 @@
 #include <algorithm>
 #include <vector>
 
+#include "base/bind.h"
+#include "chrome/browser/resource_coordinator/local_site_characteristics_database.h"
 #include "chrome/browser/resource_coordinator/time.h"
 
 namespace resource_coordinator {
@@ -123,12 +125,36 @@ void LocalSiteCharacteristicsDataImpl::NotifyUsesNotificationsInBackground() {
 
 LocalSiteCharacteristicsDataImpl::LocalSiteCharacteristicsDataImpl(
     const std::string& origin_str,
-    OnDestroyDelegate* delegate)
+    OnDestroyDelegate* delegate,
+    LocalSiteCharacteristicsDatabase* database)
     : origin_str_(origin_str),
       active_webcontents_count_(0U),
-      delegate_(delegate) {
+      database_(database),
+      delegate_(delegate),
+      weak_factory_(this) {
   DCHECK_NE(nullptr, delegate_);
-  InitWithDefaultValues();
+  DCHECK_NE(nullptr, database_);
+  DCHECK(!site_characteristics_.IsInitialized());
+  database_->ReadSiteCharacteristicsFromDB(
+      origin_str_,
+      base::BindOnce(&LocalSiteCharacteristicsDataImpl::OnInitCallback,
+                     weak_factory_.GetWeakPtr()));
+}
+
+LocalSiteCharacteristicsDataImpl::~LocalSiteCharacteristicsDataImpl() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // It's currently required that the site gets unloaded before destroying this
+  // object.
+  // TODO(sebmarchand): Check if this is a valid assumption.
+  DCHECK(!IsLoaded());
+
+  DCHECK_NE(nullptr, delegate_);
+  delegate_->OnLocalSiteCharacteristicsDataImplDestroyed(this);
+
+  if (site_characteristics_.IsInitialized()) {
+    database_->WriteSiteCharacteristicsIntoDB(origin_str_,
+                                              site_characteristics_);
+  }
 }
 
 base::TimeDelta LocalSiteCharacteristicsDataImpl::FeatureObservationDuration(
@@ -153,22 +179,12 @@ base::TimeDelta LocalSiteCharacteristicsDataImpl::FeatureObservationDuration(
   return observation_time_for_feature;
 }
 
-LocalSiteCharacteristicsDataImpl::~LocalSiteCharacteristicsDataImpl() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // It's currently required that the site gets unloaded before destroying this
-  // object.
-  // TODO(sebmarchand): Check if this is a valid assumption.
-  DCHECK(!IsLoaded());
-
-  DCHECK_NE(nullptr, delegate_);
-  delegate_->OnLocalSiteCharacteristicsDataImplDestroyed(this);
-}
-
 // static:
 void LocalSiteCharacteristicsDataImpl::IncrementFeatureObservationDuration(
     SiteCharacteristicsFeatureProto* feature_proto,
     base::TimeDelta extra_observation_duration) {
-  if (InternalRepresentationToTimeDelta(feature_proto->use_timestamp())
+  if (!feature_proto->has_use_timestamp() ||
+      InternalRepresentationToTimeDelta(feature_proto->use_timestamp())
           .is_zero()) {
     feature_proto->set_observation_duration(TimeDeltaToInternalRepresentation(
         InternalRepresentationToTimeDelta(
@@ -186,20 +202,26 @@ void LocalSiteCharacteristicsDataImpl::
   proto->set_use_timestamp(kZeroIntervalInternalRepresentation);
 }
 
-void LocalSiteCharacteristicsDataImpl::InitWithDefaultValues() {
+void LocalSiteCharacteristicsDataImpl::InitWithDefaultValues(
+    bool only_init_uninitialized_fields) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // Initialize the feature elements with the default value, this is required
   // because some fields might otherwise never be initialized.
-  for (auto* iter : GetAllFeaturesFromProto(&site_characteristics_))
-    InitSiteCharacteristicsFeatureProtoWithDefaultValues(iter);
+  for (auto* iter : GetAllFeaturesFromProto(&site_characteristics_)) {
+    if (!only_init_uninitialized_fields || !iter->IsInitialized())
+      InitSiteCharacteristicsFeatureProtoWithDefaultValues(iter);
+  }
 
-  site_characteristics_.set_last_loaded(kZeroIntervalInternalRepresentation);
+  if (!only_init_uninitialized_fields ||
+      !site_characteristics_.has_last_loaded()) {
+    site_characteristics_.set_last_loaded(kZeroIntervalInternalRepresentation);
+  }
 }
 
 void LocalSiteCharacteristicsDataImpl::ClearObservations() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // Reset all the observations.
-  InitWithDefaultValues();
+  InitWithDefaultValues(false);
 
   // Set the last loaded time to the current time if there's some loaded
   // instances of this site.
@@ -214,6 +236,9 @@ SiteFeatureUsage LocalSiteCharacteristicsDataImpl::GetFeatureUsage(
     const base::TimeDelta min_obs_time) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  if (!feature_proto.IsInitialized())
+    return SiteFeatureUsage::kSiteFeatureUsageUnknown;
+
   // Checks if this feature has already been observed.
   // TODO(sebmarchand): Check the timestamp and reset features that haven't been
   // observed in a long time, https://crbug.com/826446.
@@ -238,5 +263,58 @@ void LocalSiteCharacteristicsDataImpl::NotifyFeatureUsage(
   feature_proto->set_observation_duration(kZeroIntervalInternalRepresentation);
 }
 
+void LocalSiteCharacteristicsDataImpl::OnInitCallback(
+    base::Optional<SiteCharacteristicsProto> db_site_characteristics) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  // Check if the initialization has succeeded.
+  if (db_site_characteristics) {
+    // If so, iterates over all the features and initialize them.
+    auto this_features = GetAllFeaturesFromProto(&site_characteristics_);
+    auto db_features =
+        GetAllFeaturesFromProto(&db_site_characteristics.value());
+    auto this_features_iter = this_features.begin();
+    auto db_features_iter = db_features.begin();
+    for (; this_features_iter != this_features.end() &&
+           db_features_iter != db_features.end();
+         ++this_features_iter, ++db_features_iter) {
+      // If the |use_timestamp| field is set for the in-memory entry for this
+      // feature then there's nothing to do, otherwise update it with the values
+      // from the database.
+      if (!(*this_features_iter)->has_use_timestamp()) {
+        if ((*db_features_iter)->has_use_timestamp()) {
+          // Keep the use timestamp from the database, if any.
+          (*this_features_iter)
+              ->set_use_timestamp((*db_features_iter)->use_timestamp());
+          (*this_features_iter)
+              ->set_observation_duration(kZeroIntervalInternalRepresentation);
+        } else {
+          // Else, add the observation duration from the database to the
+          // in-memory observation duration.
+          if (!(*this_features_iter)->has_observation_duration()) {
+            (*this_features_iter)
+                ->set_observation_duration(kZeroIntervalInternalRepresentation);
+          }
+          IncrementFeatureObservationDuration(
+              (*this_features_iter),
+              InternalRepresentationToTimeDelta(
+                  (*db_features_iter)->observation_duration()));
+        }
+      }
+    }
+    // Only update the last loaded field if we haven't updated it since the
+    // creation of this object.
+    if (!site_characteristics_.has_last_loaded()) {
+      site_characteristics_.set_last_loaded(
+          db_site_characteristics->last_loaded());
+    }
+  } else {
+    // Init all the fields that haven't been initialized with a default value.
+    InitWithDefaultValues(true /* only_init_uninitialized_fields */);
+  }
+
+  DCHECK(site_characteristics_.IsInitialized());
+}
+
 }  // namespace internal
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h

@@ -9,14 +9,17 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
 #include "base/time/time.h"
+#include "chrome/browser/resource_coordinator/local_site_characteristics_database.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_feature_usage.h"
 #include "chrome/browser/resource_coordinator/site_characteristics.pb.h"
 #include "chrome/browser/resource_coordinator/tab_manager_features.h"
 
 namespace resource_coordinator {
 
+class LocalSiteCharacteristicsDatabase;
 class LocalSiteCharacteristicsDataStore;
 class LocalSiteCharacteristicsDataReaderTest;
 class LocalSiteCharacteristicsDataWriterTest;
@@ -88,7 +91,10 @@ class LocalSiteCharacteristicsDataImpl
   friend class resource_coordinator::LocalSiteCharacteristicsDataWriterTest;
 
   LocalSiteCharacteristicsDataImpl(const std::string& origin_str,
-                                   OnDestroyDelegate* delegate);
+                                   OnDestroyDelegate* delegate,
+                                   LocalSiteCharacteristicsDatabase* database);
+
+  virtual ~LocalSiteCharacteristicsDataImpl();
 
   // Helper functions to convert from/to the internal representation that is
   // used to store TimeDelta values in the |SiteCharacteristicsProto| protobuf.
@@ -100,8 +106,6 @@ class LocalSiteCharacteristicsDataImpl
     return delta.InSeconds();
   }
 
-  virtual ~LocalSiteCharacteristicsDataImpl();
-
   // Returns for how long a given feature has been observed, this is the sum of
   // the recorded observation duration and the current observation duration
   // since this site has been loaded (if applicable). If a feature has been
@@ -123,10 +127,13 @@ class LocalSiteCharacteristicsDataImpl
   static void InitSiteCharacteristicsFeatureProtoWithDefaultValues(
       SiteCharacteristicsFeatureProto* proto);
 
-  // Initialize this object with default values.
+  // Initialize this object with default values. If
+  // |only_init_uninitialized_fields| is set to true then only the fields that
+  // haven't yet been initialized will be initialized, otherwise everything will
+  // be overriden with default values.
   // NOTE: Do not call this directly while the site is loaded as this will not
   // properly update the last_loaded time, instead call |ClearObservations|.
-  void InitWithDefaultValues();
+  void InitWithDefaultValues(bool only_init_uninitialized_fields);
 
   // Clear all the past observations about this site.
   void ClearObservations();
@@ -144,6 +151,11 @@ class LocalSiteCharacteristicsDataImpl
 
   bool IsLoaded() const { return active_webcontents_count_ > 0U; }
 
+  // Callback that needs to be called by the database once it has finished
+  // trying to read the protobuf.
+  void OnInitCallback(
+      base::Optional<SiteCharacteristicsProto> site_characteristic_proto);
+
   // This site's characteristics, contains the features and other values are
   // measured.
   SiteCharacteristicsProto site_characteristics_;
@@ -155,13 +167,24 @@ class LocalSiteCharacteristicsDataImpl
   // same origin might share the same instance of this object, this counter
   // will allow to properly update the observation time (starts when the first
   // tab gets loaded, stops when the last one gets unloaded).
+  //
+  // TODO(sebmarchand): Also track the number of tabs that are in background for
+  // this origin and use this to update the observation windows. The number of
+  // active WebContents doesn't tell anything about the background/foreground
+  // state of a tab.
   size_t active_webcontents_count_;
 
+  // The database used to store the site characteristics.
+  LocalSiteCharacteristicsDatabase* database_;
+
   // The delegate that should get notified when this object is about to get
   // destroyed.
   OnDestroyDelegate* const delegate_;
 
   SEQUENCE_CHECKER(sequence_checker_);
+
+  base::WeakPtrFactory<LocalSiteCharacteristicsDataImpl> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(LocalSiteCharacteristicsDataImpl);
 };
 

chrome/browser/resource_coordinator/local_site_characteristics_data_impl_unittest.cc

@@ -31,8 +31,9 @@ class TestLocalSiteCharacteristicsDataImpl
 
   explicit TestLocalSiteCharacteristicsDataImpl(
       const std::string& origin_str,
-      LocalSiteCharacteristicsDataImpl::OnDestroyDelegate* delegate)
-      : LocalSiteCharacteristicsDataImpl(origin_str, delegate) {}
+      LocalSiteCharacteristicsDataImpl::OnDestroyDelegate* delegate,
+      LocalSiteCharacteristicsDatabase* database)
+      : LocalSiteCharacteristicsDataImpl(origin_str, delegate, database) {}
 
   base::TimeDelta FeatureObservationTimestamp(
       const SiteCharacteristicsFeatureProto& feature_proto) {
@@ -43,6 +44,47 @@ class TestLocalSiteCharacteristicsDataImpl
   ~TestLocalSiteCharacteristicsDataImpl() override {}
 };
 
+class MockLocalSiteCharacteristicsDatabase
+    : public testing::NoopLocalSiteCharacteristicsDatabase {
+ public:
+  MockLocalSiteCharacteristicsDatabase() = default;
+  ~MockLocalSiteCharacteristicsDatabase() = default;
+
+  // Note: As move-only parameters (e.g. OnceCallback) aren't supported by mock
+  // methods, add On... methods to pass a non-const reference to OnceCallback.
+  void ReadSiteCharacteristicsFromDB(
+      const std::string& origin_str,
+      LocalSiteCharacteristicsDatabase::ReadSiteCharacteristicsFromDBCallback
+          callback) override {
+    OnReadSiteCharacteristicsFromDB(origin_str, callback);
+  }
+  MOCK_METHOD2(OnReadSiteCharacteristicsFromDB,
+               void(const std::string&,
+                    LocalSiteCharacteristicsDatabase::
+                        ReadSiteCharacteristicsFromDBCallback&));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockLocalSiteCharacteristicsDatabase);
+};
+
+// Returns a SiteCharacteristicsFeatureProto that indicates that a feature
+// hasn't been used.
+SiteCharacteristicsFeatureProto GetUnusedFeatureProto() {
+  SiteCharacteristicsFeatureProto unused_feature_proto;
+  unused_feature_proto.set_observation_duration(1U);
+  unused_feature_proto.set_use_timestamp(0U);
+  return unused_feature_proto;
+}
+
+// Returns a SiteCharacteristicsFeatureProto that indicates that a feature
+// has been used.
+SiteCharacteristicsFeatureProto GetUsedFeatureProto() {
+  SiteCharacteristicsFeatureProto used_feature_proto;
+  used_feature_proto.set_observation_duration(0U);
+  used_feature_proto.set_use_timestamp(1U);
+  return used_feature_proto;
+}
+
 }  // namespace
 
 class LocalSiteCharacteristicsDataImplTest : public ::testing::Test {
@@ -65,12 +107,17 @@ class LocalSiteCharacteristicsDataImplTest : public ::testing::Test {
   ::testing::NiceMock<
       testing::MockLocalSiteCharacteristicsDataImplOnDestroyDelegate>
       destroy_delegate_;
+
+  testing::NoopLocalSiteCharacteristicsDatabase database_;
 };
 
 TEST_F(LocalSiteCharacteristicsDataImplTest, BasicTestEndToEnd) {
   auto local_site_data =
       base::MakeRefCounted<TestLocalSiteCharacteristicsDataImpl>(
-          kDummyOrigin, &destroy_delegate_);
+          kDummyOrigin, &destroy_delegate_, &database_);
+
+  EXPECT_TRUE(
+      local_site_data->site_characteristics_for_testing().IsInitialized());
 
   local_site_data->NotifySiteLoaded();
 
@@ -126,7 +173,7 @@ TEST_F(LocalSiteCharacteristicsDataImplTest, BasicTestEndToEnd) {
 TEST_F(LocalSiteCharacteristicsDataImplTest, LastLoadedTime) {
   auto local_site_data =
       base::MakeRefCounted<TestLocalSiteCharacteristicsDataImpl>(
-          kDummyOrigin, &destroy_delegate_);
+          kDummyOrigin, &destroy_delegate_, &database_);
   // Create a second instance of this object, simulates having several tab
   // owning it.
   auto local_site_data2(local_site_data);
@@ -157,7 +204,7 @@ TEST_F(LocalSiteCharacteristicsDataImplTest, LastLoadedTime) {
 TEST_F(LocalSiteCharacteristicsDataImplTest, GetFeatureUsageForUnloadedSite) {
   auto local_site_data =
       base::MakeRefCounted<TestLocalSiteCharacteristicsDataImpl>(
-          kDummyOrigin, &destroy_delegate_);
+          kDummyOrigin, &destroy_delegate_, &database_);
 
   local_site_data->NotifySiteLoaded();
   local_site_data->NotifyUsesAudioInBackground();
@@ -210,7 +257,7 @@ TEST_F(LocalSiteCharacteristicsDataImplTest, AllDurationGetSavedOnUnload) {
   // for all the features being tracked.
   auto local_site_data =
       base::MakeRefCounted<TestLocalSiteCharacteristicsDataImpl>(
-          kDummyOrigin, &destroy_delegate_);
+          kDummyOrigin, &destroy_delegate_, &database_);
 
   const base::TimeDelta kInterval = base::TimeDelta::FromSeconds(1);
   const auto kIntervalInternalRepresentation =
@@ -266,6 +313,7 @@ TEST_F(LocalSiteCharacteristicsDataImplTest, AllDurationGetSavedOnUnload) {
   expected_proto.mutable_uses_audio_in_background()->CopyFrom(
       used_feature_proto);
 
+  EXPECT_TRUE(expected_proto.IsInitialized());
   EXPECT_TRUE(
       local_site_data->site_characteristics_for_testing().IsInitialized());
   EXPECT_EQ(
@@ -282,12 +330,110 @@ TEST_F(LocalSiteCharacteristicsDataImplTest, DestroyNotifiesDelegate) {
   {
     auto local_site_data =
         base::MakeRefCounted<TestLocalSiteCharacteristicsDataImpl>(
-            kDummyOrigin, &strict_delegate);
+            kDummyOrigin, &strict_delegate, &database_);
     EXPECT_CALL(strict_delegate, OnLocalSiteCharacteristicsDataImplDestroyed(
                                      local_site_data.get()));
   }
   ::testing::Mock::VerifyAndClear(&strict_delegate);
 }
 
+TEST_F(LocalSiteCharacteristicsDataImplTest,
+       OnInitCallbackMergePreviousObservations) {
+  // Use a mock database to intercept the initialization callback and save it
+  // locally so it can be run later. This simulates an asynchronous
+  // initialization of this object and is used to test that the observations
+  // made between the time this object has been created and the callback is
+  // called get properly merged.
+  ::testing::StrictMock<MockLocalSiteCharacteristicsDatabase> mock_db;
+  LocalSiteCharacteristicsDatabase::ReadSiteCharacteristicsFromDBCallback
+      read_cb;
+  auto read_from_db_mock_impl =
+      [&](const std::string& site_origin,
+          LocalSiteCharacteristicsDatabase::
+              ReadSiteCharacteristicsFromDBCallback& callback) {
+        read_cb = std::move(callback);
+      };
+
+  EXPECT_CALL(mock_db,
+              OnReadSiteCharacteristicsFromDB(::testing::_, ::testing::_))
+      .WillOnce(::testing::Invoke(read_from_db_mock_impl));
+  auto local_site_data =
+      base::MakeRefCounted<TestLocalSiteCharacteristicsDataImpl>(
+          kDummyOrigin, &destroy_delegate_, &mock_db);
+  ::testing::Mock::VerifyAndClear(&mock_db);
+
+  // Simulates audio in background usage before the callback gets called.
+  local_site_data->NotifySiteLoaded();
+  local_site_data->NotifyUsesAudioInBackground();
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            local_site_data->UsesAudioInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            local_site_data->UsesNotificationsInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            local_site_data->UpdatesFaviconInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            local_site_data->UpdatesTitleInBackground());
+
+  // Unload the site and save the last loaded time to make sure the
+  // initialization doesn't overwrite it.
+  test_clock_.Advance(base::TimeDelta::FromSeconds(1));
+  local_site_data->NotifySiteUnloaded();
+  test_clock_.Advance(base::TimeDelta::FromSeconds(1));
+  auto last_loaded = local_site_data->last_loaded_time_for_testing();
+
+  // This protobuf should have a valid |last_loaded| field and valid observation
+  // durations for each features, but the |use_timestamp| field shouldn't have
+  // been initialized for the features that haven't been used.
+  EXPECT_FALSE(
+      local_site_data->site_characteristics_for_testing().IsInitialized());
+  EXPECT_TRUE(
+      local_site_data->site_characteristics_for_testing().has_last_loaded());
+  EXPECT_TRUE(local_site_data->site_characteristics_for_testing()
+                  .uses_audio_in_background()
+                  .IsInitialized());
+  EXPECT_FALSE(local_site_data->site_characteristics_for_testing()
+                   .uses_notifications_in_background()
+                   .IsInitialized());
+  EXPECT_FALSE(local_site_data->site_characteristics_for_testing()
+                   .uses_notifications_in_background()
+                   .has_use_timestamp());
+  EXPECT_TRUE(local_site_data->site_characteristics_for_testing()
+                  .uses_notifications_in_background()
+                  .has_observation_duration());
+
+  // Initialize a fake protobuf that indicates that this site updates its title
+  // while in background and set a fake last loaded time (this should be
+  // overriden once the callback runs).
+  base::Optional<SiteCharacteristicsProto> test_proto =
+      SiteCharacteristicsProto();
+  SiteCharacteristicsFeatureProto unused_feature_proto =
+      GetUnusedFeatureProto();
+  test_proto->mutable_updates_title_in_background()->CopyFrom(
+      GetUsedFeatureProto());
+  test_proto->mutable_updates_favicon_in_background()->CopyFrom(
+      unused_feature_proto);
+  test_proto->mutable_uses_audio_in_background()->CopyFrom(
+      unused_feature_proto);
+  test_proto->mutable_uses_notifications_in_background()->CopyFrom(
+      unused_feature_proto);
+  test_proto->set_last_loaded(42);
+
+  // Run the callback to indicate that the initialization has completed.
+  std::move(read_cb).Run(test_proto);
+
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            local_site_data->UsesAudioInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            local_site_data->UpdatesTitleInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            local_site_data->UpdatesFaviconInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            local_site_data->UsesNotificationsInBackground());
+  EXPECT_EQ(last_loaded, local_site_data->last_loaded_time_for_testing());
+
+  EXPECT_TRUE(
+      local_site_data->site_characteristics_for_testing().IsInitialized());
+}
+
 }  // namespace internal
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_reader_unittest.cc

@@ -21,10 +21,10 @@ class LocalSiteCharacteristicsDataReaderTest : public ::testing::Test {
   // LocalSiteCharacteristicsDataImpl is protected and not visible to
   // base::MakeRefCounted.
   LocalSiteCharacteristicsDataReaderTest()
-      : scoped_set_tick_clock_for_testing_(&test_clock_),
-        test_impl_(base::WrapRefCounted(
-            new internal::LocalSiteCharacteristicsDataImpl("foo.com",
-                                                           &delegate_))) {
+      : scoped_set_tick_clock_for_testing_(&test_clock_) {
+    test_impl_ =
+        base::WrapRefCounted(new internal::LocalSiteCharacteristicsDataImpl(
+            "foo.com", &delegate_, &database_));
     test_impl_->NotifySiteLoaded();
     LocalSiteCharacteristicsDataReader* reader =
         new LocalSiteCharacteristicsDataReader(test_impl_.get());
@@ -52,6 +52,8 @@ class LocalSiteCharacteristicsDataReaderTest : public ::testing::Test {
   // to create this object.
   std::unique_ptr<LocalSiteCharacteristicsDataReader> reader_;
 
+  testing::NoopLocalSiteCharacteristicsDatabase database_;
+
   DISALLOW_COPY_AND_ASSIGN(LocalSiteCharacteristicsDataReaderTest);
 };
 

chrome/browser/resource_coordinator/local_site_characteristics_data_store.cc

@@ -8,15 +8,24 @@
 #include "base/stl_util.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/resource_coordinator/leveldb_site_characteristics_database.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_reader.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_writer.h"
 #include "components/history/core/browser/history_service.h"
 
 namespace resource_coordinator {
 
+namespace {
+constexpr char kSiteCharacteristicsDirectoryName[] =
+    "Site Characteristics Database";
+}
+
 LocalSiteCharacteristicsDataStore::LocalSiteCharacteristicsDataStore(
     Profile* profile)
     : history_observer_(this) {
+  database_ = LevelDBSiteCharacteristicsDatabase::OpenOrCreateDatabase(
+      profile->GetPath().AppendASCII(kSiteCharacteristicsDirectoryName));
+
   history::HistoryService* history =
       HistoryServiceFactory::GetForProfileWithoutCreating(profile);
   if (history)
@@ -58,7 +67,8 @@ LocalSiteCharacteristicsDataStore::GetOrCreateFeatureImpl(
 
   // If not create a new one and add it to the map.
   internal::LocalSiteCharacteristicsDataImpl* site_characteristic_data =
-      new internal::LocalSiteCharacteristicsDataImpl(origin_str, this);
+      new internal::LocalSiteCharacteristicsDataImpl(origin_str, this,
+                                                     database_.get());
   // internal::LocalSiteCharacteristicsDataImpl is a ref-counted object, it's
   // safe to store a raw pointer to it here as this class will get notified when
   // it's about to be destroyed and it'll be removed from the map.
@@ -91,21 +101,25 @@ LocalSiteCharacteristicsDataStore::ResetLocalSiteCharacteristicsEntry(
 void LocalSiteCharacteristicsDataStore::OnURLsDeleted(
     history::HistoryService* history_service,
     const history::DeletionInfo& deletion_info) {
-  // TODO(sebmarchand): Removes these entry from the on-disk database once it's
-  // implemented.
+  // TODO(sebmarchand): Invalidates all the pending read/write operations to the
+  // database once it's asynchronous.
   if (deletion_info.IsAllHistory()) {
     for (auto iter = origin_data_map_.begin();
          iter != origin_data_map_.end();) {
       iter = ResetLocalSiteCharacteristicsEntry(iter);
     }
+    database_->ClearDatabase();
   } else {
-    for (const auto& deleted_row : deletion_info.deleted_rows()) {
+    std::vector<std::string> entries_to_remove;
+    for (auto deleted_row : deletion_info.deleted_rows()) {
       auto map_iter =
           origin_data_map_.find(deleted_row.url().GetOrigin().GetContent());
       if (map_iter != origin_data_map_.end()) {
         ResetLocalSiteCharacteristicsEntry(map_iter);
+        entries_to_remove.emplace_back(map_iter->first);
       }
     }
+    database_->RemoveSiteCharacteristicsFromDB(entries_to_remove);
   }
 }
 

chrome/browser/resource_coordinator/local_site_characteristics_data_store.h

@@ -20,8 +20,13 @@ class Profile;
 
 namespace resource_coordinator {
 
+class LocalSiteCharacteristicsDatabase;
+
 // Implementation of a SiteCharacteristicsDataStore that use the local site
 // characteristics database as a backend.
+//
+// TODO(sebmarchand): Expose a method to receive a dummy writer that doesn't
+// record anything, for use in incognito sessions.
 class LocalSiteCharacteristicsDataStore
     : public SiteCharacteristicsDataStore,
       public internal::LocalSiteCharacteristicsDataImpl::OnDestroyDelegate,
@@ -44,6 +49,14 @@ class LocalSiteCharacteristicsDataStore
     return origin_data_map_;
   }
 
+  // NOTE: This should be called before creating any
+  // LocalSiteCharacteristicsDataImpl object (this doesn't update the database
+  // used by these objects).
+  void SetDatabaseForTesting(
+      std::unique_ptr<LocalSiteCharacteristicsDatabase> database) {
+    database_ = std::move(database);
+  }
+
  private:
   FRIEND_TEST_ALL_PREFIXES(LocalSiteCharacteristicsDataStoreTest, EndToEnd);
   FRIEND_TEST_ALL_PREFIXES(LocalSiteCharacteristicsDataStoreTest,
@@ -76,6 +89,8 @@ class LocalSiteCharacteristicsDataStore
   ScopedObserver<history::HistoryService, LocalSiteCharacteristicsDataStore>
       history_observer_;
 
+  std::unique_ptr<LocalSiteCharacteristicsDatabase> database_;
+
   DISALLOW_COPY_AND_ASSIGN(LocalSiteCharacteristicsDataStore);
 };
 

chrome/browser/resource_coordinator/local_site_characteristics_data_store_unittest.cc

@@ -7,18 +7,36 @@
 #include "base/macros.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h"
+#include "chrome/browser/resource_coordinator/local_site_characteristics_data_unittest_utils.h"
 #include "chrome/browser/resource_coordinator/time.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/history/core/browser/url_row.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace resource_coordinator {
 
 namespace {
+
 const char kTestOrigin[] = "http://www.foo.com";
 const char kTestOrigin2[] = "http://www.bar.com";
+
+class MockLocalSiteCharacteristicsDatabase
+    : public testing::NoopLocalSiteCharacteristicsDatabase {
+ public:
+  MockLocalSiteCharacteristicsDatabase() = default;
+  ~MockLocalSiteCharacteristicsDatabase() = default;
+
+  MOCK_METHOD1(RemoveSiteCharacteristicsFromDB,
+               void(const std::vector<std::string>& site_origin));
+  MOCK_METHOD0(ClearDatabase, void());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockLocalSiteCharacteristicsDatabase);
+};
+
 }  // namespace
 
 class LocalSiteCharacteristicsDataStoreTest : public ::testing::Test {
@@ -72,11 +90,17 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, EndToEnd) {
 }
 
 TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
+  // Mock the database to ensure that the delete events get propagated properly.
+  ::testing::StrictMock<MockLocalSiteCharacteristicsDatabase>* mock_db =
+      new ::testing::StrictMock<MockLocalSiteCharacteristicsDatabase>();
+  data_store_.SetDatabaseForTesting(base::WrapUnique(mock_db));
+
   // Load a first origin, and then make use of a feature on it.
 
   const std::string kOrigin1Url = GURL(kTestOrigin).GetOrigin().GetContent();
   auto reader = data_store_.GetReaderForOrigin(kOrigin1Url);
   EXPECT_TRUE(reader);
+
   auto writer = data_store_.GetWriterForOrigin(kOrigin1Url);
   EXPECT_TRUE(writer);
 
@@ -104,8 +128,8 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
   internal::LocalSiteCharacteristicsDataImpl* data2 =
       data_store_.origin_data_map_for_testing().find(kOrigin2Url)->second;
   EXPECT_NE(nullptr, data2);
-  writer2->NotifySiteLoaded();
-  writer2->NotifyUpdatesFaviconInBackground();
+  data2->NotifySiteLoaded();
+  data2->NotifyUpdatesFaviconInBackground();
 
   // This site hasn'be been unloaded yet, so the last loaded time shouldn't have
   // changed.
@@ -114,8 +138,11 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
   history::URLRows urls_to_delete = {
       history::URLRow(GURL(kTestOrigin)),
       history::URLRow(GURL("http://www.url-not-in-map.com"))};
+  EXPECT_CALL(*mock_db, RemoveSiteCharacteristicsFromDB(::testing::ContainerEq(
+                            std::vector<std::string>({kOrigin1Url}))));
   data_store_.OnURLsDeleted(nullptr, history::DeletionInfo::ForUrls(
                                          urls_to_delete, std::set<GURL>()));
+  ::testing::Mock::VerifyAndClear(mock_db);
 
   // The information for this site have been reset, so the last loaded time
   // should now be equal to the current time and the title update feature
@@ -132,7 +159,9 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
   test_clock_.Advance(kDelay);
 
   // Delete all the information stored in the data store.
+  EXPECT_CALL(*mock_db, ClearDatabase());
   data_store_.OnURLsDeleted(nullptr, history::DeletionInfo::ForAllHistory());
+  ::testing::Mock::VerifyAndClear(mock_db);
 
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             reader2->UpdatesFaviconInBackground());

chrome/browser/resource_coordinator/local_site_characteristics_data_unittest_utils.cc

@@ -12,5 +12,24 @@ MockLocalSiteCharacteristicsDataImplOnDestroyDelegate::
 MockLocalSiteCharacteristicsDataImplOnDestroyDelegate::
     ~MockLocalSiteCharacteristicsDataImplOnDestroyDelegate() = default;
 
+NoopLocalSiteCharacteristicsDatabase::NoopLocalSiteCharacteristicsDatabase() =
+    default;
+NoopLocalSiteCharacteristicsDatabase::~NoopLocalSiteCharacteristicsDatabase() {}
+
+void NoopLocalSiteCharacteristicsDatabase::ReadSiteCharacteristicsFromDB(
+    const std::string& site_origin,
+    ReadSiteCharacteristicsFromDBCallback callback) {
+  std::move(callback).Run(base::nullopt);
+}
+
+void NoopLocalSiteCharacteristicsDatabase::WriteSiteCharacteristicsIntoDB(
+    const std::string& site_origin,
+    const SiteCharacteristicsProto& site_characteristic_proto) {}
+
+void NoopLocalSiteCharacteristicsDatabase::RemoveSiteCharacteristicsFromDB(
+    const std::vector<std::string>& site_origins) {}
+
+void NoopLocalSiteCharacteristicsDatabase::ClearDatabase() {}
+
 }  // namespace testing
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_unittest_utils.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h"
+#include "chrome/browser/resource_coordinator/local_site_characteristics_database.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace resource_coordinator {
@@ -26,6 +27,29 @@ class MockLocalSiteCharacteristicsDataImplOnDestroyDelegate
       MockLocalSiteCharacteristicsDataImplOnDestroyDelegate);
 };
 
+// An implementation of a LocalSiteCharacteristicsDatabase that doesn't record
+// anything.
+class NoopLocalSiteCharacteristicsDatabase
+    : public LocalSiteCharacteristicsDatabase {
+ public:
+  NoopLocalSiteCharacteristicsDatabase();
+  ~NoopLocalSiteCharacteristicsDatabase() override;
+
+  // LocalSiteCharacteristicsDatabase:
+  void ReadSiteCharacteristicsFromDB(
+      const std::string& site_origin,
+      ReadSiteCharacteristicsFromDBCallback callback) override;
+  void WriteSiteCharacteristicsIntoDB(
+      const std::string& site_origin,
+      const SiteCharacteristicsProto& site_characteristic_proto) override;
+  void RemoveSiteCharacteristicsFromDB(
+      const std::vector<std::string>& site_origins) override;
+  void ClearDatabase() override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NoopLocalSiteCharacteristicsDatabase);
+};
+
 }  // namespace testing
 }  // namespace resource_coordinator
 

chrome/browser/resource_coordinator/local_site_characteristics_data_writer_unittest.cc

@@ -22,7 +22,8 @@ class LocalSiteCharacteristicsDataWriterTest : public ::testing::Test {
   LocalSiteCharacteristicsDataWriterTest()
       : test_impl_(base::WrapRefCounted(
             new internal::LocalSiteCharacteristicsDataImpl("foo.com",
-                                                           &delegate_))) {
+                                                           &delegate_,
+                                                           &database_))) {
     LocalSiteCharacteristicsDataWriter* writer =
         new LocalSiteCharacteristicsDataWriter(test_impl_.get());
     writer_ = base::WrapUnique(writer);
@@ -30,6 +31,8 @@ class LocalSiteCharacteristicsDataWriterTest : public ::testing::Test {
 
   ~LocalSiteCharacteristicsDataWriterTest() override = default;
 
+  bool TabIsLoaded() { return test_impl_->IsLoaded(); }
+
   // The mock delegate used by the LocalSiteCharacteristicsDataImpl objects
   // created by this class, NiceMock is used to avoid having to set
   // expectations in test cases that don't care about this.
@@ -37,6 +40,8 @@ class LocalSiteCharacteristicsDataWriterTest : public ::testing::Test {
       testing::MockLocalSiteCharacteristicsDataImplOnDestroyDelegate>
       delegate_;
 
+  testing::NoopLocalSiteCharacteristicsDatabase database_;
+
   // The LocalSiteCharacteristicsDataImpl object used in these tests.
   scoped_refptr<internal::LocalSiteCharacteristicsDataImpl> test_impl_;
 
@@ -44,8 +49,6 @@ class LocalSiteCharacteristicsDataWriterTest : public ::testing::Test {
   // to create this object.
   std::unique_ptr<LocalSiteCharacteristicsDataWriter> writer_;
 
-  bool TabIsLoaded() { return test_impl_->IsLoaded(); }
-
   DISALLOW_COPY_AND_ASSIGN(LocalSiteCharacteristicsDataWriterTest);
 };
 

chrome/browser/resource_coordinator/local_site_characteristics_database.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_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATABASE_H_
+#define CHROME_BROWSER_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATABASE_H_
+
+#include <string>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/optional.h"
+#include "chrome/browser/resource_coordinator/site_characteristics.pb.h"
+
+class SiteCharacteristicsProto;
+
+namespace resource_coordinator {
+
+// Interface for a local site characteristic database.
+class LocalSiteCharacteristicsDatabase {
+ public:
+  // Callback to call once the initialization from the database has completed,
+  // |site_characteristic_proto| should be equal to base::nullopt if the
+  // initialization has failed.
+  using ReadSiteCharacteristicsFromDBCallback = base::OnceCallback<void(
+      base::Optional<SiteCharacteristicsProto> site_characteristic_proto)>;
+
+  LocalSiteCharacteristicsDatabase() = default;
+  virtual ~LocalSiteCharacteristicsDatabase() {}
+
+  // Checks the if there's an entry with the key |site_origin| and if so use it
+  // to initialize |site_characteristic_proto|. Calls |callback| to indicate
+  // whether or not the initialization has been successful.
+  virtual void ReadSiteCharacteristicsFromDB(
+      const std::string& site_origin,
+      ReadSiteCharacteristicsFromDBCallback callback) = 0;
+
+  // Store an entry in the database, create it if it doesn't exist and update it
+  // if it does.
+  virtual void WriteSiteCharacteristicsIntoDB(
+      const std::string& site_origin,
+      const SiteCharacteristicsProto& site_characteristic_proto) = 0;
+
+  // Removes some entries from the database.
+  virtual void RemoveSiteCharacteristicsFromDB(
+      const std::vector<std::string>& site_origins) = 0;
+
+  // Clear the database, removes every entries that it contains.
+  virtual void ClearDatabase() = 0;
+};
+
+}  // namespace resource_coordinator
+
+#endif  // CHROME_BROWSER_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATABASE_H_

chrome/test/BUILD.gn

@@ -2908,6 +2908,7 @@ test("unit_tests") {
       "../browser/page_load_metrics/observers/session_restore_page_load_metrics_observer_unittest.cc",
       "../browser/resource_coordinator/background_tab_navigation_throttle_unittest.cc",
       "../browser/resource_coordinator/discard_metrics_lifecycle_unit_observer_unittest.cc",
+      "../browser/resource_coordinator/leveldb_site_characteristics_database_unittest.cc",
       "../browser/resource_coordinator/lifecycle_unit_base_unittest.cc",
       "../browser/resource_coordinator/lifecycle_unit_unittest.cc",
       "../browser/resource_coordinator/local_site_characteristics_data_impl_unittest.cc",