RC: Expose the dirty bit for DB entries in chrome://discards.

Bug: 874968
Change-Id: Id87498398908bfe8cf7e6c7ab596074b0575325a
Reviewed-on: https://chromium-review.googlesource.com/1236499
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593178}

chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h

@@ -128,6 +128,7 @@ class LocalSiteCharacteristicsDataImpl
   }
 
   const url::Origin& origin() const { return origin_; }
+  bool is_dirty() const { return is_dirty_; }
 
   void ExpireAllObservationWindowsForTesting();
 

chrome/browser/resource_coordinator/local_site_characteristics_data_store.cc

@@ -93,8 +93,9 @@ void LocalSiteCharacteristicsDataStore::GetDatabaseSize(
   std::move(on_have_data).Run(base::nullopt, base::nullopt);
 }
 
-bool LocalSiteCharacteristicsDataStore::GetaDataForOrigin(
+bool LocalSiteCharacteristicsDataStore::GetDataForOrigin(
     const url::Origin& origin,
+    bool* is_dirty,
     std::unique_ptr<SiteCharacteristicsProto>* data) {
   DCHECK_NE(nullptr, data);
   const auto it = origin_data_map_.find(origin);
@@ -104,6 +105,7 @@ bool LocalSiteCharacteristicsDataStore::GetaDataForOrigin(
   std::unique_ptr<SiteCharacteristicsProto> ret =
       std::make_unique<SiteCharacteristicsProto>();
   ret->CopyFrom(it->second->FlushStateToProto());
+  *is_dirty = it->second->is_dirty();
   *data = std::move(ret);
   return true;
 }

chrome/browser/resource_coordinator/local_site_characteristics_data_store.h

@@ -63,8 +63,9 @@ class LocalSiteCharacteristicsDataStore
   const char* GetDataStoreName() override;
   std::vector<url::Origin> GetAllInMemoryOrigins() override;
   void GetDatabaseSize(DatabaseSizeCallback on_have_data) override;
-  bool GetaDataForOrigin(
+  bool GetDataForOrigin(
       const url::Origin& origin,
+      bool* is_dirty,
       std::unique_ptr<SiteCharacteristicsProto>* data) override;
   LocalSiteCharacteristicsDataStore* GetDataStore() override;
 

chrome/browser/resource_coordinator/local_site_characteristics_data_store_inspector.h

@@ -50,11 +50,12 @@ class LocalSiteCharacteristicsDataStoreInspector {
   virtual void GetDatabaseSize(DatabaseSizeCallback on_have_data) = 0;
 
   // Retrieves the in-memory data for a given origin.
-  // On return |data| contains the available data for |origin|, or nullptr
-  // if no data is available for this origin.
-  virtual bool GetaDataForOrigin(
+  // On return |data| contains the available data for |origin| if available,
+  // and |is_dirty| is true if the entry needs flushing to disk.
+  // Returns true if an entry exists for |origin|.
+  virtual bool GetDataForOrigin(
       const url::Origin& origin,
-      /* TODO(siggi): more properties, like "dirty bit" */
+      bool* is_dirty,
       std::unique_ptr<SiteCharacteristicsProto>* data) = 0;
 
   // Retrieves the data store this inspector is associated with.

chrome/browser/resource_coordinator/local_site_characteristics_data_store_unittest.cc

@@ -211,7 +211,9 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, InspectorWorks) {
   // We expect an empty data store at the outset.
   EXPECT_EQ(0U, inspector->GetAllInMemoryOrigins().size());
   std::unique_ptr<SiteCharacteristicsProto> data;
-  EXPECT_FALSE(inspector->GetaDataForOrigin(kTestOrigin, &data));
+  bool is_dirty = false;
+  EXPECT_FALSE(inspector->GetDataForOrigin(kTestOrigin, &is_dirty, &data));
+  EXPECT_FALSE(is_dirty);
   EXPECT_EQ(nullptr, data.get());
 
   {
@@ -220,8 +222,14 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, InspectorWorks) {
                                                   TabVisibility::kBackground);
 
     EXPECT_EQ(1U, inspector->GetAllInMemoryOrigins().size());
-    EXPECT_TRUE(inspector->GetaDataForOrigin(kTestOrigin, &data));
+    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));
+    EXPECT_TRUE(is_dirty);
   }
 
   // Make sure the interface is unregistered from the profile on destruction.

chrome/browser/resource_coordinator/local_site_characteristics_non_recording_data_store.cc

@@ -68,13 +68,14 @@ void LocalSiteCharacteristicsNonRecordingDataStore::GetDatabaseSize(
   data_store_inspector_->GetDatabaseSize(std::move(on_have_data));
 }
 
-bool LocalSiteCharacteristicsNonRecordingDataStore::GetaDataForOrigin(
+bool LocalSiteCharacteristicsNonRecordingDataStore::GetDataForOrigin(
     const url::Origin& origin,
+    bool* is_dirty,
     std::unique_ptr<SiteCharacteristicsProto>* data) {
   if (!data_store_inspector_)
     return false;
 
-  return data_store_inspector_->GetaDataForOrigin(origin, data);
+  return data_store_inspector_->GetDataForOrigin(origin, is_dirty, data);
 }
 
 SiteCharacteristicsDataStore*

chrome/browser/resource_coordinator/local_site_characteristics_non_recording_data_store.h

@@ -44,8 +44,9 @@ class LocalSiteCharacteristicsNonRecordingDataStore
   const char* GetDataStoreName() override;
   std::vector<url::Origin> GetAllInMemoryOrigins() override;
   void GetDatabaseSize(DatabaseSizeCallback on_have_data) override;
-  bool GetaDataForOrigin(
+  bool GetDataForOrigin(
       const url::Origin& origin,
+      bool* is_dirty,
       std::unique_ptr<SiteCharacteristicsProto>* data) override;
   SiteCharacteristicsDataStore* GetDataStore() override;
 

chrome/browser/resource_coordinator/local_site_characteristics_non_recording_data_store_unittest.cc

@@ -109,7 +109,9 @@ TEST_F(LocalSiteCharacteristicsNonRecordingDataStoreTest, InspectorWorks) {
   // We expect an empty data store at the outset.
   EXPECT_EQ(0U, inspector->GetAllInMemoryOrigins().size());
   std::unique_ptr<SiteCharacteristicsProto> data;
-  EXPECT_FALSE(inspector->GetaDataForOrigin(kTestOrigin, &data));
+  bool is_dirty = false;
+  EXPECT_FALSE(inspector->GetDataForOrigin(kTestOrigin, &is_dirty, &data));
+  EXPECT_FALSE(is_dirty);
   EXPECT_EQ(nullptr, data.get());
 
   {
@@ -119,8 +121,13 @@ TEST_F(LocalSiteCharacteristicsNonRecordingDataStoreTest, InspectorWorks) {
         kTestOrigin, TabVisibility::kBackground);
 
     EXPECT_EQ(1U, inspector->GetAllInMemoryOrigins().size());
-    EXPECT_TRUE(inspector->GetaDataForOrigin(kTestOrigin, &data));
+    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 profile on destruction.

chrome/browser/ui/webui/discards/discards_ui.cc

@@ -341,11 +341,11 @@ void DiscardsDetailsProviderImpl::GetSiteCharacteristicsDatabase(
     // Get the data for this origin and convert it from proto to the
     // corresponding mojo structure.
     std::unique_ptr<SiteCharacteristicsProto> proto;
-    if (data_store_inspector_->GetaDataForOrigin(origin, &proto)) {
+    bool is_dirty = false;
+    if (data_store_inspector_->GetDataForOrigin(origin, &is_dirty, &proto)) {
       auto entry = ConvertEntryFromProto(proto.get());
       entry->origin = origin.Serialize();
-      // TODO(siggi): Get the dirty bit.
-      entry->is_dirty = false;
+      entry->is_dirty = is_dirty;
       result->db_rows.push_back(std::move(entry));
     }
   }