Reland "[PM] Add the non recording site data cache."

There was a race around |g_use_in_memory_db_for_testing| :

[ RUN      ] NonRecordingSiteDataCacheTest.EndToEnd
==================
WARNING: ThreadSanitizer: data race (pid=1484)
  Write of size 1 at 0x55e6654d29d0 by main thread:
    #0 ~AutoReset base/auto_reset.h:40:25 (unit_tests+0x42d690c)
    #1 operator() buildtools/third_party/libc++/trunk/include/memory:2338 (unit_tests+0x42d690c)
    #2 reset buildtools/third_party/libc++/trunk/include/memory:2651 (unit_tests+0x42d690c)
    #3 ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2605 (unit_tests+0x42d690c)
    #4 performance_manager::(anonymous namespace)::NonRecordingSiteDataCacheTest::~NonRecordingSiteDataCacheTest() chrome/browser/performance_manager/persistence/site_data/non_recording_site_data_cache_unittest.cc:30 (unit_tests+0x42d690c)
    #5 performance_manager::NonRecordingSiteDataCacheTest_EndToEnd_Test::~NonRecordingSiteDataCacheTest_EndToEnd_Test() chrome/browser/performance_manager/persistence/site_data/non_recording_site_data_cache_unittest.cc:66:1 (unit_tests+0x42d65d9)
    #6 HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc (unit_tests+0x65f041e)
    #7 testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2704 (unit_tests+0x65f041e)
    #8 testing::TestSuite::Run() third_party/googletest/src/googletest/src/gtest.cc:2828:28 (unit_tests+0x65f0ef6)
    #9 testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5285:44 (unit_tests+0x6603366)
    #10 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc (unit_tests+0x6602859)
    #11 testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4873 (unit_tests+0x6602859)
    #12 RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2453:46 (unit_tests+0x9dd5692)
    #13 base::TestSuite::Run() base/test/test_suite.cc:316 (unit_tests+0x9dd5692)
    #14 content::UnitTestTestSuite::Run() content/public/test/unittest_test_suite.cc:103:23 (unit_tests+0x9ee213f)
    #15 Invoke<int (content::UnitTestTestSuite::*)(), content::UnitTestTestSuite *> base/bind_internal.h:499:12 (unit_tests+0x9dc3168)
    #16 MakeItSo<int (content::UnitTestTestSuite::*const &)(), content::UnitTestTestSuite *> base/bind_internal.h:599 (unit_tests+0x9dc3168)
    #17 RunImpl<int (content::UnitTestTestSuite::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> > &, 0> base/bind_internal.h:672 (unit_tests+0x9dc3168)
    #18 base::internal::Invoker<base::internal::BindState<int (content::UnitTestTestSuite::*)(), base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:654 (unit_tests+0x9dc3168)
    #19 Run base/callback.h:97:12 (unit_tests+0x9ddf24b)
    #20 base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:158 (unit_tests+0x9ddf24b)
    #21 base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:494:10 (unit_tests+0x9ddf0b0)
    #22 main chrome/test/base/run_all_unittests.cc:35:10 (unit_tests+0x9dc3099)

  Previous read of size 1 at 0x55e6654d29d0 by thread T18:
    #0 performance_manager::LevelDBSiteDataStore::AsyncHelper::OpenOrCreateDatabaseImpl() chrome/browser/performance_manager/persistence/site_data/leveldb_site_data_store.cc:374:7 (unit_tests+0xb3e7762)
    #1 performance_manager::LevelDBSiteDataStore::AsyncHelper::OpenOrCreateDatabase() chrome/browser/performance_manager/persistence/site_data/leveldb_site_data_store.cc:195:30 (unit_tests+0xb3e7394)
    #2 Invoke<void (performance_manager::LevelDBSiteDataStore::AsyncHelper::*)(), performance_manager::LevelDBSiteDataStore::AsyncHelper *> base/bind_internal.h:499:12 (unit_tests+0xb3e9608)
    #3 MakeItSo<void (performance_manager::LevelDBSiteDataStore::AsyncHelper::*)(), performance_manager::LevelDBSiteDataStore::AsyncHelper *> base/bind_internal.h:599 (unit_tests+0xb3e9608)
    #4 RunImpl<void (performance_manager::LevelDBSiteDataStore::AsyncHelper::*)(), std::__1::tuple<base::internal::UnretainedWrapper<performance_manager::LevelDBSiteDataStore::AsyncHelper> >, 0> base/bind_internal.h:672 (unit_tests+0xb3e9608)
    #5 base::internal::Invoker<base::internal::BindState<void (performance_manager::LevelDBSiteDataStore::AsyncHelper::*)(), base::internal::UnretainedWrapper<performance_manager::LevelDBSiteDataStore::AsyncHelper> >, void ()>::RunOnce(base::internal::BindStateBase*) base/bind_internal.h:641 (unit_tests+0xb3e9608)
    #6 Run base/callback.h:97:12 (unit_tests+0xacb91d5)
    #7 base::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/task/common/task_annotator.cc:142 (unit_tests+0xacb91d5)
    #8 base::internal::TaskTracker::RunBlockShutdown(base::internal::Task*) base/task/thread_pool/task_tracker.cc:773:19 (unit_tests+0xacdde01)
    #9 RunTaskWithShutdownBehavior base/task/thread_pool/task_tracker.cc:788:7 (unit_tests+0xacdd825)
    #10 base::internal::TaskTracker::RunOrSkipTask(base::internal::Task, base::internal::TaskSource*, base::TaskTraits const&, bool) base/task/thread_pool/task_tracker.cc:617 (unit_tests+0xacdd825)
    #11 base::internal::TaskTrackerPosix::RunOrSkipTask(base::internal::Task, base::internal::TaskSource*, base::TaskTraits const&, bool) base/task/thread_pool/task_tracker_posix.cc:24:16 (unit_tests+0xad5bfc4)
    #12 base::test::ScopedTaskEnvironment::TestTaskTracker::RunOrSkipTask(base::internal::Task, base::internal::TaskSource*, base::TaskTraits const&, bool) base/test/scoped_task_environment.cc:663:46 (unit_tests+0x9dd0a08)
    #13 base::internal::TaskTracker::RunAndPopNextTask(base::internal::RegisteredTaskSource) base/task/thread_pool/task_tracker.cc:479:5 (unit_tests+0xacdd119)
    #14 base::internal::WorkerThread::RunWorker() base/task/thread_pool/worker_thread.cc:320:34 (unit_tests+0xacf08cd)
    #15 base::internal::WorkerThread::RunPooledWorker() base/task/thread_pool/worker_thread.cc:222:3 (unit_tests+0xacf0561)
    #16 base::internal::WorkerThread::ThreadMain() base/task/thread_pool/worker_thread.cc:201:7 (unit_tests+0xacf03cf)
    #17 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:81:13 (unit_tests+0xad5cbb4)

  Location is global 'performance_manager::(anonymous namespace)::g_use_in_memory_db_for_testing' of size 1 at 0x55e6654d29d0 (unit_tests+0x0000132b39d0)

  Thread T18 'ThreadPoolForeg' (tid=1837, running) created by main thread at:
    #0 pthread_create /b/swarming/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:989:3 (unit_tests+0x3bd013b)
    #1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:120:13 (unit_tests+0xad5c5c7)
    #2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:246:10 (unit_tests+0xad5c4c5)
    #3 base::internal::WorkerThread::Start(base::WorkerThreadObserver*) base/task/thread_pool/worker_thread.cc:68:3 (unit_tests+0xaceff6d)
    #4 operator() base/task/thread_pool/thread_group_impl.cc:185:15 (unit_tests+0xace7e06)
    #5 ForEachWorker<(lambda at ../../base/task/thread_pool/thread_group_impl.cc:184:37)> base/task/thread_pool/thread_group_impl.cc:150 (unit_tests+0xace7e06)
    #6 base::internal::ThreadGroupImpl::ScopedWorkersExecutor::FlushImpl() base/task/thread_pool/thread_group_impl.cc:184 (unit_tests+0xace7e06)
    #7 base::internal::ThreadGroupImpl::ScopedWorkersExecutor::~ScopedWorkersExecutor() base/task/thread_pool/thread_group_impl.cc:103:30 (unit_tests+0xace47f4)
    #8 base::internal::ThreadGroupImpl::Start(int, int, base::TimeDelta, scoped_refptr<base::TaskRunner>, base::WorkerThreadObserver*, base::internal::ThreadGroup::WorkerEnvironment, base::Optional<base::TimeDelta>) base/task/thread_pool/thread_group_impl.cc:425:1 (unit_tests+0xace4799)
    #9 base::internal::ThreadPoolImpl::Start(base::ThreadPoolInstance::InitParams const&, base::WorkerThreadObserver*) base/task/thread_pool/thread_pool_impl.cc:197:11 (unit_tests+0xace0133)
    #10 base::test::ScopedTaskEnvironment::InitializeThreadPool() base/test/scoped_task_environment.cc:391:30 (unit_tests+0x9dcf8fc)
    #11 base::test::ScopedTaskEnvironment::ScopedTaskEnvironment(base::test::ScopedTaskEnvironment::MainThreadType, base::test::ScopedTaskEnvironment::ThreadPoolExecutionMode, base::test::ScopedTaskEnvironment::NowSource, base::test::ScopedTaskEnvironment::ThreadingMode, bool, base::trait_helpers::NotATraitTag) base/test/scoped_task_environment.cc:348:5 (unit_tests+0x9dcf2bc)
    #12 base::test::ScopedTaskEnvironment::ScopedTaskEnvironment<base::test::ScopedTaskEnvironment::SubclassCreatesDefaultTaskRunner, base::test::ScopedTaskEnvironment::MainThreadType, void>(base::test::ScopedTaskEnvironment::SubclassCreatesDefaultTaskRunner, base::test::ScopedTaskEnvironment::MainThreadType) base/test/scoped_task_environment.h:161:9 (unit_tests+0x3c4e2f0)
    #13 content::TestBrowserThreadBundle::TestBrowserThreadBundle<void>() content/public/test/test_browser_thread_bundle.h:131:13 (unit_tests+0x3c4e287)
    #14 performance_manager::(anonymous namespace)::NonRecordingSiteDataCacheTest::NonRecordingSiteDataCacheTest() chrome/browser/performance_manager/persistence/site_data/non_recording_site_data_cache_unittest.cc:23:3 (unit_tests+0x42d6a76)
    #15 NonRecordingSiteDataCacheTest_EndToEnd_Test chrome/browser/performance_manager/persistence/site_data/non_recording_site_data_cache_unittest.cc:66:1 (unit_tests+0x42d69d4)
    #16 testing::internal::TestFactoryImpl<performance_manager::NonRecordingSiteDataCacheTest_EndToEnd_Test>::CreateTest() third_party/googletest/src/googletest/include/gtest/internal/gtest-internal.h:460 (unit_tests+0x42d69d4)
    #17 HandleExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test *> third_party/googletest/src/googletest/src/gtest.cc (unit_tests+0x65f01df)
    #18 testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2688 (unit_tests+0x65f01df)
    #19 testing::TestSuite::Run() third_party/googletest/src/googletest/src/gtest.cc:2828:28 (unit_tests+0x65f0ef6)
    #20 testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5285:44 (unit_tests+0x6603366)
    #21 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc (unit_tests+0x6602859)
    #22 testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4873 (unit_tests+0x6602859)
    #23 RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2453:46 (unit_tests+0x9dd5692)
    #24 base::TestSuite::Run() base/test/test_suite.cc:316 (unit_tests+0x9dd5692)
    #25 content::UnitTestTestSuite::Run() content/public/test/unittest_test_suite.cc:103:23 (unit_tests+0x9ee213f)
    #26 Invoke<int (content::UnitTestTestSuite::*)(), content::UnitTestTestSuite *> base/bind_internal.h:499:12 (unit_tests+0x9dc3168)
    #27 MakeItSo<int (content::UnitTestTestSuite::*const &)(), content::UnitTestTestSuite *> base/bind_internal.h:599 (unit_tests+0x9dc3168)
    #28 RunImpl<int (content::UnitTestTestSuite::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> > &, 0> base/bind_internal.h:672 (unit_tests+0x9dc3168)
    #29 base::internal::Invoker<base::internal::BindState<int (content::UnitTestTestSuite::*)(), base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:654 (unit_tests+0x9dc3168)
    #30 Run base/callback.h:97:12 (unit_tests+0x9ddf24b)
    #31 base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:158 (unit_tests+0x9ddf24b)
    #32 base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:494:10 (unit_tests+0x9ddf0b0)
    #33 main chrome/test/base/run_all_unittests.cc:35:10 (unit_tests+0x9dc3099)

The problem is that opening the DB (in LevelDBSiteDataStore::AsyncHelper::OpenOrCreateDatabaseImpl)
is done asynchronously, and by the time we get there the AutoReset might
have been destroyed. Waiting for the DB to be initialized fixes this.

This reverts commit b7310e3f.

Bug: 961336
Change-Id: Ia7d9367b10857837340358d7a516c26b87d0cebf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1682668Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676639}

chrome/browser/BUILD.gn

@@ -1080,6 +1080,8 @@ jumbo_split_static_library("browser") {
     "performance_manager/persistence/site_data/feature_usage.h",
     "performance_manager/persistence/site_data/leveldb_site_data_store.cc",
     "performance_manager/persistence/site_data/leveldb_site_data_store.h",
+    "performance_manager/persistence/site_data/non_recording_site_data_cache.cc",
+    "performance_manager/persistence/site_data/non_recording_site_data_cache.h",
     "performance_manager/persistence/site_data/noop_site_data_writer.cc",
     "performance_manager/persistence/site_data/noop_site_data_writer.h",
     "performance_manager/persistence/site_data/site_data_cache.h",

chrome/browser/performance_manager/persistence/site_data/leveldb_site_data_store.cc

@@ -7,7 +7,9 @@
 #include <limits>
 #include <string>
 
+#include "base/auto_reset.h"
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/files/file_util.h"
 #include "base/hash/md5.h"
 #include "base/logging.h"
@@ -20,12 +22,15 @@
 #include "build/build_config.h"
 #include "third_party/leveldatabase/env_chromium.h"
 #include "third_party/leveldatabase/leveldb_chrome.h"
+#include "third_party/leveldatabase/src/include/leveldb/env.h"
 #include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
 
 namespace performance_manager {
 
 namespace {
 
+bool g_use_in_memory_db_for_testing = false;
+
 // The name of the following histograms is the same as the one used in the
 // //c/b/resource_coordinator version of this file. It's fine to keep the same
 // name as these 2 codepath will never be enabled at the same time. These
@@ -159,6 +164,12 @@ class LevelDBSiteDataStore::AsyncHelper {
     return db_.get();
   }
 
+  void SetInitializationCallbackForTesting(base::OnceClosure callback) {
+    init_callback_for_testing_ = std::move(callback);
+    if (DBIsInitialized())
+      std::move(init_callback_for_testing_).Run();
+  }
+
  private:
   enum class OpeningType {
     // A new database has been created.
@@ -170,6 +181,10 @@ class LevelDBSiteDataStore::AsyncHelper {
   // Implementation for the OpenOrCreateDatabase function.
   OpeningType OpenOrCreateDatabaseImpl();
 
+  // A levelDB environment that gets used for testing. This allows using an
+  // in-memory database when needed.
+  std::unique_ptr<leveldb::Env> env_for_testing_;
+
   // The on disk location of the database.
   const base::FilePath db_path_;
   // The connection to the LevelDB database.
@@ -179,12 +194,18 @@ class LevelDBSiteDataStore::AsyncHelper {
   // The options to be used for all database write operations.
   leveldb::WriteOptions write_options_;
 
+  base::OnceClosure init_callback_for_testing_;
+
   SEQUENCE_CHECKER(sequence_checker_);
   DISALLOW_COPY_AND_ASSIGN(AsyncHelper);
 };
 
 void LevelDBSiteDataStore::AsyncHelper::OpenOrCreateDatabase() {
   OpeningType opening_type = OpenOrCreateDatabaseImpl();
+
+  if (init_callback_for_testing_)
+    std::move(init_callback_for_testing_).Run();
+
   if (!db_)
     return;
   std::string db_metadata;
@@ -362,6 +383,12 @@ LevelDBSiteDataStore::AsyncHelper::OpenOrCreateDatabaseImpl() {
 
   leveldb_env::Options options;
   options.create_if_missing = true;
+
+  if (g_use_in_memory_db_for_testing) {
+    env_for_testing_ = leveldb_chrome::NewMemEnv("LevelDBSiteDataStore");
+    options.env = env_for_testing_.get();
+  }
+
   leveldb::Status status =
       leveldb_env::OpenDB(options, db_path_.AsUTF8Unsafe(), &db_);
 
@@ -468,6 +495,16 @@ void LevelDBSiteDataStore::GetStoreSize(GetStoreSizeCallback callback) {
       std::move(reply_callback));
 }
 
+void LevelDBSiteDataStore::SetInitializationCallbackForTesting(
+    base::OnceClosure callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  blocking_task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&LevelDBSiteDataStore::AsyncHelper::
+                                    SetInitializationCallbackForTesting,
+                                base::Unretained(async_helper_.get()),
+                                std::move(callback)));
+}
+
 bool LevelDBSiteDataStore::DatabaseIsInitializedForTesting() {
   return async_helper_->DBIsInitialized();
 }
@@ -476,4 +513,11 @@ leveldb::DB* LevelDBSiteDataStore::GetDBForTesting() {
   return async_helper_->GetDBForTesting();
 }
 
+// static
+std::unique_ptr<base::AutoReset<bool>>
+LevelDBSiteDataStore::UseInMemoryDBForTesting() {
+  return std::make_unique<base::AutoReset<bool>>(
+      &g_use_in_memory_db_for_testing, true);
+}
+
 }  // namespace performance_manager

chrome/browser/performance_manager/persistence/site_data/leveldb_site_data_store.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_PERFORMANCE_MANAGER_PERSISTENCE_SITE_DATA_LEVELDB_SITE_DATA_STORE_H_
 #define CHROME_BROWSER_PERFORMANCE_MANAGER_PERSISTENCE_SITE_DATA_LEVELDB_SITE_DATA_STORE_H_
 
+#include "base/auto_reset.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/sequence_checker.h"
@@ -41,6 +42,7 @@ class LevelDBSiteDataStore : public SiteDataStore {
       const std::vector<url::Origin>& site_origins) override;
   void ClearStore() override;
   void GetStoreSize(GetStoreSizeCallback callback) override;
+  void SetInitializationCallbackForTesting(base::OnceClosure callback) override;
 
   bool DatabaseIsInitializedForTesting();
 
@@ -51,6 +53,10 @@ class LevelDBSiteDataStore : public SiteDataStore {
   // thread safe.
   leveldb::DB* GetDBForTesting();
 
+  // Make the new instances of this class use an in memory database rather than
+  // creating it on disk.
+  static std::unique_ptr<base::AutoReset<bool>> UseInMemoryDBForTesting();
+
   static const size_t kDbVersion;
   static const char kDbMetadataKey[];
 

chrome/browser/performance_manager/persistence/site_data/non_recording_site_data_cache.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/performance_manager/persistence/site_data/non_recording_site_data_cache.h"
+
+#include "base/memory/ptr_util.h"
+#include "chrome/browser/performance_manager/persistence/site_data/noop_site_data_writer.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_cache_factory.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_reader.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_writer.h"
+
+namespace performance_manager {
+
+NonRecordingSiteDataCache::NonRecordingSiteDataCache(
+    const std::string& browser_context_id,
+    SiteDataCacheInspector* data_cache_inspector,
+    SiteDataCache* data_cache_for_readers)
+    : data_cache_for_readers_(data_cache_for_readers),
+      data_cache_inspector_(data_cache_inspector),
+      browser_context_id_(browser_context_id) {
+  DCHECK(data_cache_for_readers_);
+  // Register the debug interface against the browser context.
+  SiteDataCacheFactory::GetInstance()->SetDataCacheInspectorForBrowserContext(
+      this, browser_context_id_);
+}
+
+NonRecordingSiteDataCache::~NonRecordingSiteDataCache() {
+  SiteDataCacheFactory::GetInstance()->SetDataCacheInspectorForBrowserContext(
+      nullptr, browser_context_id_);
+}
+
+std::unique_ptr<SiteDataReader> NonRecordingSiteDataCache::GetReaderForOrigin(
+    const url::Origin& origin) {
+  return data_cache_for_readers_->GetReaderForOrigin(origin);
+}
+
+std::unique_ptr<SiteDataWriter> NonRecordingSiteDataCache::GetWriterForOrigin(
+    const url::Origin& origin,
+    performance_manager::TabVisibility tab_visibility) {
+  // Return a fake data writer.
+  SiteDataWriter* writer = new NoopSiteDataWriter();
+  return base::WrapUnique(writer);
+}
+
+bool NonRecordingSiteDataCache::IsRecordingForTesting() {
+  return false;
+}
+
+const char* NonRecordingSiteDataCache::GetDataCacheName() {
+  return "NonRecordingSiteDataCache";
+}
+
+std::vector<url::Origin> NonRecordingSiteDataCache::GetAllInMemoryOrigins() {
+  if (!data_cache_inspector_)
+    return std::vector<url::Origin>();
+
+  return data_cache_inspector_->GetAllInMemoryOrigins();
+}
+
+void NonRecordingSiteDataCache::GetDataStoreSize(
+    DataStoreSizeCallback on_have_data) {
+  if (!data_cache_inspector_) {
+    std::move(on_have_data).Run(base::nullopt, base::nullopt);
+    return;
+  }
+
+  data_cache_inspector_->GetDataStoreSize(std::move(on_have_data));
+}
+
+bool NonRecordingSiteDataCache::GetDataForOrigin(
+    const url::Origin& origin,
+    bool* is_dirty,
+    std::unique_ptr<SiteDataProto>* data) {
+  if (!data_cache_inspector_)
+    return false;
+
+  return data_cache_inspector_->GetDataForOrigin(origin, is_dirty, data);
+}
+
+NonRecordingSiteDataCache* NonRecordingSiteDataCache::GetDataCache() {
+  return this;
+}
+
+}  // namespace performance_manager

chrome/browser/performance_manager/persistence/site_data/non_recording_site_data_cache.h

+// 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.
+
+#ifndef CHROME_BROWSER_PERFORMANCE_MANAGER_PERSISTENCE_SITE_DATA_NON_RECORDING_SITE_DATA_CACHE_H_
+#define CHROME_BROWSER_PERFORMANCE_MANAGER_PERSISTENCE_SITE_DATA_NON_RECORDING_SITE_DATA_CACHE_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_cache.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_cache_inspector.h"
+
+namespace performance_manager {
+
+// Implementation of a SiteDataCache that ensures that no data gets persisted.
+//
+// This class should be used for off the record profiles.
+class NonRecordingSiteDataCache : public SiteDataCache,
+                                  public SiteDataCacheInspector {
+ public:
+  NonRecordingSiteDataCache(const std::string& browser_context_id,
+                            SiteDataCacheInspector* data_cache_inspector,
+                            SiteDataCache* data_cache_for_readers);
+  ~NonRecordingSiteDataCache() override;
+
+  // SiteDataCache:
+  std::unique_ptr<SiteDataReader> GetReaderForOrigin(
+      const url::Origin& origin) override;
+  std::unique_ptr<SiteDataWriter> GetWriterForOrigin(
+      const url::Origin& origin,
+      performance_manager::TabVisibility tab_visibility) override;
+  bool IsRecordingForTesting() override;
+
+  // SiteDataCacheInspector:
+  const char* GetDataCacheName() override;
+  std::vector<url::Origin> GetAllInMemoryOrigins() override;
+  void GetDataStoreSize(DataStoreSizeCallback on_have_data) override;
+  bool GetDataForOrigin(const url::Origin& origin,
+                        bool* is_dirty,
+                        std::unique_ptr<SiteDataProto>* data) override;
+  NonRecordingSiteDataCache* GetDataCache() override;
+
+ private:
+  // The data cache to use to create the readers served by this data store. E.g.
+  // during an incognito session it should point to the data cache used by the
+  // parent session.
+  SiteDataCache* data_cache_for_readers_;
+
+  // The inspector implementation this instance delegates to.
+  SiteDataCacheInspector* data_cache_inspector_;
+
+  // The ID of the browser context this data store is associated with.
+  const std::string browser_context_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(NonRecordingSiteDataCache);
+};
+
+}  // namespace performance_manager
+
+#endif  // CHROME_BROWSER_PERFORMANCE_MANAGER_PERSISTENCE_SITE_DATA_NON_RECORDING_SITE_DATA_CACHE_H_

chrome/browser/performance_manager/persistence/site_data/non_recording_site_data_cache_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/performance_manager/persistence/site_data/non_recording_site_data_cache.h"
+
+#include "chrome/browser/performance_manager/persistence/site_data/leveldb_site_data_store.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_cache.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_cache_factory.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_cache_impl.h"
+#include "chrome/browser/performance_manager/persistence/site_data/site_data_cache_inspector.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace performance_manager {
+
+namespace {
+
+class NonRecordingSiteDataCacheTest : public testing::Test {
+ public:
+  NonRecordingSiteDataCacheTest()
+      : use_in_memory_db_for_testing_(
+            LevelDBSiteDataStore::UseInMemoryDBForTesting()),
+        factory_(SiteDataCacheFactory::CreateForTesting(
+            test_browser_thread_bundle_.GetMainThreadTaskRunner())),
+        off_the_record_profile_(parent_profile_.GetOffTheRecordProfile()) {}
+
+  ~NonRecordingSiteDataCacheTest() override { factory_.reset(); }
+
+  void SetUp() override {
+    recording_data_cache_ = base::WrapUnique(new SiteDataCacheImpl(
+        parent_profile_.UniqueId(), parent_profile_.GetPath()));
+
+    // Wait for the database to be initialized.
+    base::RunLoop run_loop;
+    recording_data_cache_->SetInitializationCallbackForTesting(
+        run_loop.QuitClosure());
+    run_loop.Run();
+
+    non_recording_data_cache_ = std::make_unique<NonRecordingSiteDataCache>(
+        off_the_record_profile_->UniqueId(), recording_data_cache_.get(),
+        recording_data_cache_.get());
+  }
+
+ protected:
+  const url::Origin kTestOrigin =
+      url::Origin::Create(GURL("http://www.foo.com"));
+
+  content::TestBrowserThreadBundle test_browser_thread_bundle_;
+
+  // Ensure that the database used by the data store owned by
+  // |recording_data_cache_| gets created in memory. This avoid having to wait
+  // for it to be fully closed before destroying |parent_profile_|.
+  std::unique_ptr<base::AutoReset<bool>> use_in_memory_db_for_testing_;
+
+  // The data cache factory that will be used by the caches tested here.
+  std::unique_ptr<SiteDataCacheFactory, base::OnTaskRunnerDeleter> factory_;
+
+  // The on the record profile.
+  TestingProfile parent_profile_;
+  // An off the record profile owned by |parent_profile|.
+  Profile* off_the_record_profile_;
+
+  std::unique_ptr<SiteDataCacheImpl> recording_data_cache_;
+  std::unique_ptr<NonRecordingSiteDataCache> non_recording_data_cache_;
+};
+
+}  // namespace
+
+TEST_F(NonRecordingSiteDataCacheTest, EndToEnd) {
+  // Ensures that the observation made via a writer created by the non
+  // recording data cache aren't recorded.
+  auto reader = non_recording_data_cache_->GetReaderForOrigin(kTestOrigin);
+  EXPECT_TRUE(reader);
+  auto fake_writer = non_recording_data_cache_->GetWriterForOrigin(
+      kTestOrigin, performance_manager::TabVisibility::kBackground);
+  EXPECT_TRUE(fake_writer);
+  auto real_writer = recording_data_cache_->GetWriterForOrigin(
+      kTestOrigin, performance_manager::TabVisibility::kBackground);
+  EXPECT_TRUE(real_writer);
+
+  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            reader->UpdatesTitleInBackground());
+  fake_writer->NotifySiteLoaded();
+  fake_writer->NotifyUpdatesTitleInBackground();
+  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            reader->UpdatesTitleInBackground());
+
+  real_writer->NotifySiteLoaded();
+  real_writer->NotifyUpdatesTitleInBackground();
+  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
+            reader->UpdatesTitleInBackground());
+
+  // These unload events shouldn't be registered, make sure that they aren't by
+  // unloading the site more time than it has been loaded.
+  fake_writer->NotifySiteUnloaded();
+  fake_writer->NotifySiteUnloaded();
+
+  real_writer->NotifySiteUnloaded();
+}
+
+TEST_F(NonRecordingSiteDataCacheTest, InspectorWorks) {
+  // Make sure the inspector interface was registered at construction.
+  SiteDataCacheInspector* inspector = factory_->GetInspectorForBrowserContext(
+      off_the_record_profile_->UniqueId());
+  EXPECT_NE(nullptr, inspector);
+  EXPECT_EQ(non_recording_data_cache_.get(), inspector);
+
+  EXPECT_STREQ("NonRecordingSiteDataCache", inspector->GetDataCacheName());
+
+  // We expect an empty data cache at the outset.
+  EXPECT_EQ(0U, inspector->GetAllInMemoryOrigins().size());
+  std::unique_ptr<SiteDataProto> data;
+  bool is_dirty = false;
+  EXPECT_FALSE(inspector->GetDataForOrigin(kTestOrigin, &is_dirty, &data));
+  EXPECT_FALSE(is_dirty);
+  EXPECT_EQ(nullptr, data.get());
+
+  {
+    // Add an entry through the writing data cache, see that it's reflected in
+    // the inspector interface.
+    auto writer = recording_data_cache_->GetWriterForOrigin(
+        kTestOrigin, performance_manager::TabVisibility::kBackground);
+
+    EXPECT_EQ(1U, inspector->GetAllInMemoryOrigins().size());
+    EXPECT_TRUE(inspector->GetDataForOrigin(kTestOrigin, &is_dirty, &data));
+    EXPECT_FALSE(is_dirty);
+    ASSERT_NE(nullptr, data.get());
+
+    // Touch the underlying data, see that the dirty bit updates.
+    writer->NotifySiteLoaded();
+    EXPECT_TRUE(inspector->GetDataForOrigin(kTestOrigin, &is_dirty, &data));
+  }
+
+  // Make sure the interface is unregistered from the browser context on
+  // destruction.
+  non_recording_data_cache_.reset();
+  EXPECT_EQ(nullptr, factory_->GetInspectorForBrowserContext(
+                         off_the_record_profile_->UniqueId()));
+}
+
+}  // namespace performance_manager

chrome/browser/performance_manager/persistence/site_data/noop_site_data_writer.h

@@ -30,8 +30,9 @@ class NoopSiteDataWriter : public SiteDataWriter {
       uint64_t private_footprint_kb_estimate) override;
 
  private:
-  // Private constructor, these objects are meant to be created by a noop site
-  // data store.
+  friend class NonRecordingSiteDataCache;
+  // Private constructor, these objects are meant to be created by a
+  // NonRecordingSiteDataCache.
   NoopSiteDataWriter();
 
   DISALLOW_COPY_AND_ASSIGN(NoopSiteDataWriter);

chrome/browser/performance_manager/persistence/site_data/site_data_cache_factory.cc

@@ -9,8 +9,8 @@
 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "base/task_runner_util.h"
-#include "base/threading/sequenced_task_runner_handle.h"
 #include "chrome/browser/performance_manager/performance_manager.h"
+#include "chrome/browser/performance_manager/persistence/site_data/non_recording_site_data_cache.h"
 #include "chrome/browser/performance_manager/persistence/site_data/site_data_cache_impl.h"
 #include "chrome/browser/performance_manager/persistence/site_data/site_data_cache_inspector.h"
 #include "content/public/browser/browser_context.h"
@@ -30,7 +30,6 @@ SiteDataCacheFactory::SiteDataCacheFactory(
     const scoped_refptr<base::SequencedTaskRunner> task_runner)
     : task_runner_(task_runner) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 }
 
 SiteDataCacheFactory::~SiteDataCacheFactory() {
@@ -68,19 +67,24 @@ SiteDataCacheFactory::CreateForTesting(
 // static
 void SiteDataCacheFactory::OnBrowserContextCreatedOnUIThread(
     SiteDataCacheFactory* factory,
-    content::BrowserContext* browser_context) {
+    content::BrowserContext* browser_context,
+    content::BrowserContext* parent_context) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(factory);
 
   // As |factory| will be deleted on its task runner it's safe to pass the raw
   // pointer to BindOnce, it's guaranteed that this task will run before the
   // factory.
+  base::Optional<std::string> parent_context_id;
+  if (parent_context) {
+    DCHECK(browser_context->IsOffTheRecord());
+    parent_context_id = parent_context->UniqueId();
+  }
   factory->task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&SiteDataCacheFactory::OnBrowserContextCreated,
                      base::Unretained(factory), browser_context->UniqueId(),
-                     browser_context->GetPath(),
-                     browser_context->IsOffTheRecord()));
+                     browser_context->GetPath(), parent_context_id));
 }
 
 // static
@@ -97,28 +101,28 @@ void SiteDataCacheFactory::OnBrowserContextDestroyedOnUIThread(
                      base::Unretained(factory), browser_context->UniqueId()));
 }
 
-// static
 SiteDataCache* SiteDataCacheFactory::GetDataCacheForBrowserContext(
     const std::string& browser_context_id) const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto it = data_cache_map_.find(browser_context_id);
   if (it != data_cache_map_.end())
     return it->second.get();
   return nullptr;
 }
 
-// static
 SiteDataCacheInspector* SiteDataCacheFactory::GetInspectorForBrowserContext(
     const std::string& browser_context_id) const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto it = data_cache_inspector_map_.find(browser_context_id);
   if (it != data_cache_inspector_map_.end())
     return it->second;
   return nullptr;
 }
 
-// static
 void SiteDataCacheFactory::SetDataCacheInspectorForBrowserContext(
     SiteDataCacheInspector* inspector,
     const std::string& browser_context_id) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (inspector) {
     DCHECK_EQ(nullptr, GetInspectorForBrowserContext(browser_context_id));
     data_cache_inspector_map_.emplace(
@@ -132,14 +136,23 @@ void SiteDataCacheFactory::SetDataCacheInspectorForBrowserContext(
 void SiteDataCacheFactory::OnBrowserContextCreated(
     const std::string& browser_context_id,
     const base::FilePath& context_path,
-    bool context_is_off_the_record) {
+    base::Optional<std::string> parent_context_id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   DCHECK(!base::Contains(data_cache_map_, browser_context_id));
 
-  if (context_is_off_the_record) {
-    // TODO(sebmarchand): Add support for off-the-record contexts.
-    NOTREACHED();
+  if (parent_context_id) {
+    SiteDataCacheInspector* parent_debug =
+        GetInspectorForBrowserContext(parent_context_id.value());
+    DCHECK(parent_debug);
+    DCHECK(base::Contains(data_cache_map_, parent_context_id.value()));
+    SiteDataCache* data_cache_for_readers =
+        data_cache_map_[parent_context_id.value()].get();
+    DCHECK(data_cache_for_readers);
+    data_cache_map_.emplace(std::make_pair(
+        std::move(browser_context_id),
+        std::make_unique<NonRecordingSiteDataCache>(
+            browser_context_id, parent_debug, data_cache_for_readers)));
   } else {
     data_cache_map_.emplace(std::make_pair(
         std::move(browser_context_id),

chrome/browser/performance_manager/persistence/site_data/site_data_cache_factory.h

@@ -12,6 +12,7 @@
 #include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "chrome/browser/performance_manager/persistence/site_data/site_data_cache.h"
@@ -57,9 +58,14 @@ class SiteDataCacheFactory {
   // destroyed. They should be called from the UI thread, a task will then be
   // posted to the task_runner owned by |factory| to create the data store
   // associated with this browser context.
+  //
+  // If this browser context is inheriting from a parent context (e.g. if it's
+  // off the record) then this parent context should be specified via
+  // |parent_context|.
   static void OnBrowserContextCreatedOnUIThread(
       SiteDataCacheFactory* factory,
-      content::BrowserContext* browser_context);
+      content::BrowserContext* browser_context,
+      content::BrowserContext* parent_context);
   static void OnBrowserContextDestroyedOnUIThread(
       SiteDataCacheFactory* factory,
       content::BrowserContext* browser_context);
@@ -98,7 +104,7 @@ class SiteDataCacheFactory {
   // that runs on this object's task runner.
   void OnBrowserContextCreated(const std::string& browser_context_id,
                                const base::FilePath& context_path,
-                               bool context_is_off_the_record);
+                               base::Optional<std::string> parent_context_id);
   void OnBrowserContextDestroyed(const std::string& browser_context_id);
 
   // The task runner on which this object lives, this is expected to be the

chrome/browser/performance_manager/persistence/site_data/site_data_cache_factory_unittest.cc

@@ -41,7 +41,7 @@ class SiteDataCacheFactoryTest : public ::testing::Test {
 
 TEST_F(SiteDataCacheFactoryTest, EndToEnd) {
   SiteDataCacheFactory::OnBrowserContextCreatedOnUIThread(factory_.get(),
-                                                          &profile_);
+                                                          &profile_, nullptr);
 
   base::RunLoop run_loop;
   task_runner_->PostTask(

chrome/browser/performance_manager/persistence/site_data/site_data_cache_impl.cc

@@ -6,6 +6,7 @@
 
 #include <set>
 
+#include "base/callback.h"
 #include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
@@ -158,4 +159,9 @@ void SiteDataCacheImpl::ClearAllSiteData() {
   data_store_->ClearStore();
 }
 
+void SiteDataCacheImpl::SetInitializationCallbackForTesting(
+    base::OnceClosure callback) {
+  data_store_->SetInitializationCallbackForTesting(std::move(callback));
+}
+
 }  // namespace performance_manager

chrome/browser/performance_manager/persistence/site_data/site_data_cache_impl.h

@@ -10,6 +10,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/callback_forward.h"
 #include "base/containers/flat_map.h"
 #include "base/files/file_path.h"
 #include "base/gtest_prod_util.h"
@@ -36,7 +37,7 @@ class SiteDataCacheImpl : public SiteDataCache,
                     const base::FilePath& browser_context_path);
   ~SiteDataCacheImpl() override;
 
-  // SiteCharacteristicDataCache:
+  // SiteDataCache:
   std::unique_ptr<SiteDataReader> GetReaderForOrigin(
       const url::Origin& origin) override;
   std::unique_ptr<SiteDataWriter> GetWriterForOrigin(
@@ -70,6 +71,10 @@ class SiteDataCacheImpl : public SiteDataCache,
   // Clear the data cache and the on-disk store.
   void ClearAllSiteData();
 
+  // Set a callback that will be called once the data store backing this cache
+  // has been fully initialized.
+  void SetInitializationCallbackForTesting(base::OnceClosure callback);
+
  private:
   // Returns a pointer to the SiteDataImpl object associated with |origin|,
   // create one and add it to |origin_data_map_| if it doesn't exist.

chrome/browser/performance_manager/persistence/site_data/site_data_impl_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/performance_manager/persistence/site_data/site_data_impl.h"
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/test/bind_test_util.h"

chrome/browser/performance_manager/persistence/site_data/site_data_reader.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/callback.h"
 #include "chrome/browser/performance_manager/persistence/site_data/site_data_impl.h"
 
 namespace performance_manager {

chrome/browser/performance_manager/persistence/site_data/site_data_reader_unittest.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/callback.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"

chrome/browser/performance_manager/persistence/site_data/site_data_store.h

@@ -7,7 +7,7 @@
 
 #include <vector>
 
-#include "base/callback.h"
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/optional.h"
 #include "chrome/browser/performance_manager/persistence/site_data/site_data.pb.h"
@@ -52,6 +52,11 @@ class SiteDataStore {
 
   // Retrieve the size of the store.
   virtual void GetStoreSize(GetStoreSizeCallback callback) = 0;
+
+  // Set a callback that will be called once the data store has been fully
+  // initialized.
+  virtual void SetInitializationCallbackForTesting(
+      base::OnceClosure callback) = 0;
 };
 
 }  // namespace performance_manager

chrome/browser/performance_manager/persistence/site_data/unittest_utils.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/performance_manager/persistence/site_data/unittest_utils.h"
+#include "base/callback.h"
 
 #include <utility>
 
@@ -36,5 +37,10 @@ void NoopSiteDataStore::GetStoreSize(GetStoreSizeCallback callback) {
   std::move(callback).Run(base::nullopt, base::nullopt);
 }
 
+void NoopSiteDataStore::SetInitializationCallbackForTesting(
+    base::OnceClosure callback) {
+  std::move(callback).Run();
+}
+
 }  // namespace testing
 }  // namespace performance_manager

chrome/browser/performance_manager/persistence/site_data/unittest_utils.h

@@ -44,6 +44,7 @@ class NoopSiteDataStore : public SiteDataStore {
       const std::vector<url::Origin>& site_origins) override;
   void ClearStore() override;
   void GetStoreSize(GetStoreSizeCallback callback) override;
+  void SetInitializationCallbackForTesting(base::OnceClosure callback) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NoopSiteDataStore);

chrome/test/BUILD.gn

@@ -2981,6 +2981,7 @@ test("unit_tests") {
     "../browser/performance_manager/performance_manager_unittest.cc",
     "../browser/performance_manager/persistence/site_data/exponential_moving_average_unittest.cc",
     "../browser/performance_manager/persistence/site_data/leveldb_site_data_store_unittest.cc",
+    "../browser/performance_manager/persistence/site_data/non_recording_site_data_cache_unittest.cc",
     "../browser/performance_manager/persistence/site_data/site_data_cache_factory_unittest.cc",
     "../browser/performance_manager/persistence/site_data/site_data_cache_impl_unittest.cc",
     "../browser/performance_manager/persistence/site_data/site_data_impl_unittest.cc",