Add the LocalSiteCharacteristicsDataWriter class

This class will be used to forward feature usage events to the site
characteristics database.

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

chrome/browser/BUILD.gn

@@ -2622,6 +2622,8 @@ jumbo_split_static_library("browser") {
       "resource_coordinator/local_site_characteristics_data_reader.h",
       "resource_coordinator/local_site_characteristics_data_store.cc",
       "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_feature_usage.h",
       "resource_coordinator/site_characteristics_data_reader.h",
       "resource_coordinator/site_characteristics_data_store.h",

chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h

@@ -19,6 +19,7 @@ namespace resource_coordinator {
 
 class LocalSiteCharacteristicsDataStore;
 class LocalSiteCharacteristicsDataReaderTest;
+class LocalSiteCharacteristicsDataWriterTest;
 
 namespace internal {
 
@@ -84,6 +85,7 @@ class LocalSiteCharacteristicsDataImpl
   friend class LocalSiteCharacteristicsDataImplTest;
   friend class resource_coordinator::LocalSiteCharacteristicsDataReaderTest;
   friend class resource_coordinator::LocalSiteCharacteristicsDataStore;
+  friend class resource_coordinator::LocalSiteCharacteristicsDataWriterTest;
 
   LocalSiteCharacteristicsDataImpl(const std::string& origin_str,
                                    OnDestroyDelegate* delegate);

chrome/browser/resource_coordinator/local_site_characteristics_data_reader_unittest.cc

@@ -33,20 +33,11 @@ class LocalSiteCharacteristicsDataReaderTest : public ::testing::Test {
 
   ~LocalSiteCharacteristicsDataReaderTest() override {
     test_impl_->NotifySiteUnloaded();
-    reader_.reset();
-    test_impl_ = nullptr;
   }
 
   base::SimpleTestTickClock test_clock_;
   ScopedSetTickClockForTesting scoped_set_tick_clock_for_testing_;
 
-  // The LocalSiteCharacteristicsDataImpl object used in these tests.
-  scoped_refptr<internal::LocalSiteCharacteristicsDataImpl> test_impl_;
-
-  // A LocalSiteCharacteristicsDataReader object associated with the origin used
-  // to create this object.
-  std::unique_ptr<LocalSiteCharacteristicsDataReader> reader_;
-
   // 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.
@@ -54,6 +45,13 @@ class LocalSiteCharacteristicsDataReaderTest : public ::testing::Test {
       testing::MockLocalSiteCharacteristicsDataImplOnDestroyDelegate>
       delegate_;
 
+  // The LocalSiteCharacteristicsDataImpl object used in these tests.
+  scoped_refptr<internal::LocalSiteCharacteristicsDataImpl> test_impl_;
+
+  // A LocalSiteCharacteristicsDataReader object associated with the origin used
+  // to create this object.
+  std::unique_ptr<LocalSiteCharacteristicsDataReader> reader_;
+
   DISALLOW_COPY_AND_ASSIGN(LocalSiteCharacteristicsDataReaderTest);
 };
 

chrome/browser/resource_coordinator/local_site_characteristics_data_store.cc

@@ -30,12 +30,23 @@ LocalSiteCharacteristicsDataStore::GetReaderForOrigin(
     const std::string& origin_str) {
   internal::LocalSiteCharacteristicsDataImpl* impl =
       GetOrCreateFeatureImpl(origin_str);
-  DCHECK_NE(nullptr, impl);
+  DCHECK(impl);
   SiteCharacteristicsDataReader* data_reader =
       new LocalSiteCharacteristicsDataReader(impl);
   return base::WrapUnique(data_reader);
 }
 
+std::unique_ptr<LocalSiteCharacteristicsDataWriter>
+LocalSiteCharacteristicsDataStore::GetWriterForOrigin(
+    const std::string& origin_str) {
+  internal::LocalSiteCharacteristicsDataImpl* impl =
+      GetOrCreateFeatureImpl(origin_str);
+  DCHECK(impl);
+  LocalSiteCharacteristicsDataWriter* data_writer =
+      new LocalSiteCharacteristicsDataWriter(impl);
+  return base::WrapUnique(data_writer);
+}
+
 internal::LocalSiteCharacteristicsDataImpl*
 LocalSiteCharacteristicsDataStore::GetOrCreateFeatureImpl(
     const std::string& origin_str) {
@@ -57,7 +68,7 @@ LocalSiteCharacteristicsDataStore::GetOrCreateFeatureImpl(
 void LocalSiteCharacteristicsDataStore::
     OnLocalSiteCharacteristicsDataImplDestroyed(
         internal::LocalSiteCharacteristicsDataImpl* impl) {
-  DCHECK_NE(nullptr, impl);
+  DCHECK(impl);
   DCHECK(base::ContainsKey(origin_data_map_, impl->origin_str()));
   // Remove the entry for this origin as this is about to get destroyed.
   auto num_erased = origin_data_map_.erase(impl->origin_str());

chrome/browser/resource_coordinator/local_site_characteristics_data_store.h

@@ -5,11 +5,14 @@
 #ifndef CHROME_BROWSER_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATA_STORE_H_
 #define CHROME_BROWSER_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATA_STORE_H_
 
+#include <string>
+
 #include "base/containers/flat_map.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h"
+#include "chrome/browser/resource_coordinator/local_site_characteristics_data_writer.h"
 #include "chrome/browser/resource_coordinator/site_characteristics_data_store.h"
 #include "components/history/core/browser/history_service_observer.h"
 
@@ -34,6 +37,9 @@ class LocalSiteCharacteristicsDataStore
   std::unique_ptr<SiteCharacteristicsDataReader> GetReaderForOrigin(
       const std::string& origin_str) override;
 
+  std::unique_ptr<LocalSiteCharacteristicsDataWriter> GetWriterForOrigin(
+      const std::string& origin_str);
+
   const LocalSiteCharacteristicsMap& origin_data_map_for_testing() const {
     return origin_data_map_;
   }

chrome/browser/resource_coordinator/local_site_characteristics_data_store_unittest.cc

@@ -40,20 +40,19 @@ class LocalSiteCharacteristicsDataStoreTest : public ::testing::Test {
 
 TEST_F(LocalSiteCharacteristicsDataStoreTest, EndToEnd) {
   auto reader = data_store_.GetReaderForOrigin(kTestOrigin);
-  EXPECT_NE(nullptr, reader.get());
-  EXPECT_EQ(1U, data_store_.origin_data_map_for_testing().size());
+  EXPECT_TRUE(reader);
+  auto writer = data_store_.GetWriterForOrigin(kTestOrigin);
+  EXPECT_TRUE(writer);
 
-  internal::LocalSiteCharacteristicsDataImpl* data =
-      data_store_.origin_data_map_for_testing().find(kTestOrigin)->second;
-  EXPECT_NE(nullptr, data);
+  EXPECT_EQ(1U, data_store_.origin_data_map_for_testing().size());
 
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             reader->UpdatesTitleInBackground());
-  data->NotifySiteLoaded();
-  data->NotifyUpdatesTitleInBackground();
+  writer->NotifySiteLoaded();
+  writer->NotifyUpdatesTitleInBackground();
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             reader->UpdatesTitleInBackground());
-  data->NotifySiteUnloaded();
+  writer->NotifySiteUnloaded();
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             reader->UpdatesTitleInBackground());
 
@@ -66,6 +65,7 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, EndToEnd) {
   reader_copy.reset();
 
   reader.reset();
+  writer.reset();
   EXPECT_TRUE(data_store_.origin_data_map_for_testing().empty());
 
   data_store_.OnURLsDeleted(nullptr, history::DeletionTimeRange::AllTime(),
@@ -78,6 +78,9 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
   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);
+
   internal::LocalSiteCharacteristicsDataImpl* data =
       data_store_.origin_data_map_for_testing().find(kOrigin1Url)->second;
   EXPECT_NE(nullptr, data);
@@ -86,9 +89,9 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
 
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             reader->UpdatesTitleInBackground());
-  data->NotifySiteLoaded();
+  writer->NotifySiteLoaded();
   base::TimeDelta last_loaded_time = data->last_loaded_time_for_testing();
-  data->NotifyUpdatesTitleInBackground();
+  writer->NotifyUpdatesTitleInBackground();
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             reader->UpdatesTitleInBackground());
   test_clock_.Advance(kDelay);
@@ -97,11 +100,13 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
   const std::string kOrigin2Url = GURL(kTestOrigin2).GetOrigin().GetContent();
   auto reader2 = data_store_.GetReaderForOrigin(kOrigin2Url);
   EXPECT_TRUE(reader2);
+  auto writer2 = data_store_.GetWriterForOrigin(kOrigin2Url);
+  EXPECT_TRUE(writer2);
   internal::LocalSiteCharacteristicsDataImpl* data2 =
       data_store_.origin_data_map_for_testing().find(kOrigin2Url)->second;
   EXPECT_NE(nullptr, data2);
-  data2->NotifySiteLoaded();
-  data2->NotifyUpdatesFaviconInBackground();
+  writer2->NotifySiteLoaded();
+  writer2->NotifyUpdatesFaviconInBackground();
 
   // This site hasn'be been unloaded yet, so the last loaded time shouldn't have
   // changed.
@@ -138,8 +143,8 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
   EXPECT_EQ(data2->last_loaded_time_for_testing(),
             test_clock_.NowTicks() - base::TimeTicks::UnixEpoch());
 
-  data->NotifySiteUnloaded();
-  data2->NotifySiteUnloaded();
+  writer->NotifySiteUnloaded();
+  writer2->NotifySiteUnloaded();
 }
 
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_writer.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/local_site_characteristics_data_writer.h"
+
+#include "chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h"
+
+namespace resource_coordinator {
+
+LocalSiteCharacteristicsDataWriter::~LocalSiteCharacteristicsDataWriter() =
+    default;
+
+void LocalSiteCharacteristicsDataWriter::NotifySiteLoaded() {
+  impl_->NotifySiteLoaded();
+}
+
+void LocalSiteCharacteristicsDataWriter::NotifySiteUnloaded() {
+  impl_->NotifySiteUnloaded();
+}
+
+void LocalSiteCharacteristicsDataWriter::NotifyUpdatesFaviconInBackground() {
+  impl_->NotifyUpdatesFaviconInBackground();
+}
+
+void LocalSiteCharacteristicsDataWriter::NotifyUpdatesTitleInBackground() {
+  impl_->NotifyUpdatesTitleInBackground();
+}
+
+void LocalSiteCharacteristicsDataWriter::NotifyUsesAudioInBackground() {
+  impl_->NotifyUsesAudioInBackground();
+}
+
+void LocalSiteCharacteristicsDataWriter::NotifyUsesNotificationsInBackground() {
+  impl_->NotifyUsesNotificationsInBackground();
+}
+
+LocalSiteCharacteristicsDataWriter::LocalSiteCharacteristicsDataWriter(
+    scoped_refptr<internal::LocalSiteCharacteristicsDataImpl> impl)
+    : impl_(std::move(impl)) {}
+
+}  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_writer.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_DATA_WRITER_H_
+#define CHROME_BROWSER_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATA_WRITER_H_
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+
+namespace resource_coordinator {
+
+namespace internal {
+class LocalSiteCharacteristicsDataImpl;
+}  // namespace internal
+
+// Used to record local characteristics usage observations in the local
+// database.
+class LocalSiteCharacteristicsDataWriter {
+ public:
+  ~LocalSiteCharacteristicsDataWriter();
+
+  // Records tab load/unload events.
+  void NotifySiteLoaded();
+  void NotifySiteUnloaded();
+
+  // Records feature usage.
+  void NotifyUpdatesFaviconInBackground();
+  void NotifyUpdatesTitleInBackground();
+  void NotifyUsesAudioInBackground();
+  void NotifyUsesNotificationsInBackground();
+
+ private:
+  friend class LocalSiteCharacteristicsDataWriterTest;
+  friend class LocalSiteCharacteristicsDataStoreTest;
+  friend class LocalSiteCharacteristicsDataStore;
+
+  // Private constructor, these objects are meant to be created by a site
+  // characteristics data store.
+  explicit LocalSiteCharacteristicsDataWriter(
+      scoped_refptr<internal::LocalSiteCharacteristicsDataImpl> impl);
+
+  // The LocalSiteCharacteristicDataInternal object we delegate to.
+  const scoped_refptr<internal::LocalSiteCharacteristicsDataImpl> impl_;
+
+  DISALLOW_COPY_AND_ASSIGN(LocalSiteCharacteristicsDataWriter);
+};
+
+}  // namespace resource_coordinator
+
+#endif  // CHROME_BROWSER_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATA_WRITER_H_

chrome/browser/resource_coordinator/local_site_characteristics_data_writer_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/local_site_characteristics_data_writer.h"
+
+#include "base/macros.h"
+#include "base/memory/ptr_util.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/local_site_characteristics_feature_usage.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace resource_coordinator {
+
+class LocalSiteCharacteristicsDataWriterTest : public ::testing::Test {
+ protected:
+  // The constructors needs to call 'new' directly rather than using the
+  // base::MakeRefCounted helper function because the constructor of
+  // LocalSiteCharacteristicsDataImpl is protected and not visible to
+  // base::MakeRefCounted.
+  LocalSiteCharacteristicsDataWriterTest()
+      : test_impl_(base::WrapRefCounted(
+            new internal::LocalSiteCharacteristicsDataImpl("foo.com",
+                                                           &delegate_))) {
+    LocalSiteCharacteristicsDataWriter* writer =
+        new LocalSiteCharacteristicsDataWriter(test_impl_.get());
+    writer_ = base::WrapUnique(writer);
+  }
+
+  ~LocalSiteCharacteristicsDataWriterTest() override = default;
+
+  // 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.
+  ::testing::NiceMock<
+      testing::MockLocalSiteCharacteristicsDataImplOnDestroyDelegate>
+      delegate_;
+
+  // The LocalSiteCharacteristicsDataImpl object used in these tests.
+  scoped_refptr<internal::LocalSiteCharacteristicsDataImpl> test_impl_;
+
+  // A LocalSiteCharacteristicsDataWriter object associated with the origin used
+  // to create this object.
+  std::unique_ptr<LocalSiteCharacteristicsDataWriter> writer_;
+
+  bool TabIsLoaded() { return test_impl_->IsLoaded(); }
+
+  DISALLOW_COPY_AND_ASSIGN(LocalSiteCharacteristicsDataWriterTest);
+};
+
+TEST_F(LocalSiteCharacteristicsDataWriterTest, TestModifiers) {
+  // Make sure that we initially have no information about any of the features
+  // and that the site is in an unloaded state.
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UpdatesFaviconInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UpdatesTitleInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UsesAudioInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UsesNotificationsInBackground());
+
+  // Test the OnTabLoaded function.
+  EXPECT_FALSE(TabIsLoaded());
+  writer_->NotifySiteLoaded();
+  EXPECT_TRUE(TabIsLoaded());
+
+  // Test all the modifiers.
+
+  writer_->NotifyUpdatesFaviconInBackground();
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UpdatesFaviconInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UpdatesTitleInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UsesAudioInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UsesNotificationsInBackground());
+
+  writer_->NotifyUpdatesTitleInBackground();
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UpdatesFaviconInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UpdatesTitleInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UsesAudioInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UsesNotificationsInBackground());
+
+  writer_->NotifyUsesAudioInBackground();
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UpdatesFaviconInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UpdatesTitleInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UsesAudioInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
+            test_impl_->UsesNotificationsInBackground());
+
+  writer_->NotifyUsesNotificationsInBackground();
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UpdatesFaviconInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UpdatesTitleInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UsesAudioInBackground());
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
+            test_impl_->UsesNotificationsInBackground());
+
+  writer_->NotifySiteUnloaded();
+}
+
+}  // namespace resource_coordinator

chrome/test/BUILD.gn

@@ -2940,6 +2940,7 @@ test("unit_tests") {
       "../browser/resource_coordinator/local_site_characteristics_data_store_unittest.cc",
       "../browser/resource_coordinator/local_site_characteristics_data_unittest_utils.cc",
       "../browser/resource_coordinator/local_site_characteristics_data_unittest_utils.h",
+      "../browser/resource_coordinator/local_site_characteristics_data_writer_unittest.cc",
       "../browser/resource_coordinator/tab_activity_watcher_unittest.cc",
       "../browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc",
       "../browser/resource_coordinator/tab_lifecycle_unit_unittest.cc",