[Chromeshine] Persist events to LevelDB.

Implement the remainder of the UsageStatsDatabase functions on WebsiteEvents.

Bug: 902572
Change-Id: Ic4420acd9c19adb383af556aa89c3621e89a4e39
Reviewed-on: https://chromium-review.googlesource.com/c/1437556
Commit-Queue: Natalie Chouinard <chouinard@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628289}

chrome/browser/android/usage_stats/usage_stats_database.cc

@@ -66,9 +66,9 @@ bool KeyContainsDomainFilter(const base::flat_set<std::string>& domains,
   return domains.contains(key.substr(kFqdnPosition));
 }
 
-UsageStatsDatabase::Error ToError(bool success) {
-  return success ? UsageStatsDatabase::Error::kNoError
-                 : UsageStatsDatabase::Error::kUnknownError;
+UsageStatsDatabase::Error ToError(bool isSuccess) {
+  return isSuccess ? UsageStatsDatabase::Error::kNoError
+                   : UsageStatsDatabase::Error::kUnknownError;
 }
 
 std::string CreateWebsiteEventKey(int64_t seconds, const std::string& fqdn) {
@@ -95,6 +95,20 @@ void UsageStatsDatabase::GetAllEvents(EventsCallback callback) {
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
+void UsageStatsDatabase::QueryEventsInRange(int64_t start,
+                                            int64_t end,
+                                            EventsCallback callback) {
+  // Load all UsageStats with the website events prefix, where the timestamp is
+  // in the specified range. Function accepts a half-open range [start, end) as
+  // input, but the database operates on fully-closed ranges. Because the
+  // timestamps are represented by integers, [start, end) is equivalent to
+  // [start, end - 1].
+  proto_db_->LoadKeysAndEntriesInRange(
+      CreateWebsiteEventKey(start, ""), CreateWebsiteEventKey(end - 1, ""),
+      base::BindOnce(&UsageStatsDatabase::OnLoadEntriesForQueryEventsInRange,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
 void UsageStatsDatabase::AddEvents(std::vector<WebsiteEvent> events,
                                    StatusCallback callback) {
   auto entries = std::make_unique<
@@ -119,6 +133,34 @@ void UsageStatsDatabase::AddEvents(std::vector<WebsiteEvent> events,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
+void UsageStatsDatabase::DeleteAllEvents(StatusCallback callback) {
+  // Remove all entries with the website event prefix.
+  proto_db_->UpdateEntriesWithRemoveFilter(
+      std::make_unique<
+          leveldb_proto::ProtoDatabase<UsageStat>::KeyEntryVector>(),
+      base::BindRepeating([](const std::string& key) { return true; }),
+      kWebsiteEventPrefix,
+      base::BindOnce(&UsageStatsDatabase::OnUpdateEntries,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+void UsageStatsDatabase::DeleteEventsInRange(int64_t start,
+                                             int64_t end,
+                                             StatusCallback callback) {
+  // TODO(crbug/927655): If/when leveldb_proto adds an UpdateEntriesInRange
+  // function, we should consolidate these two proto_db_ calls into a single
+  // call.
+
+  // Load all keys with the website events prefix, where the timestamp is
+  // in the specified range, passing a callback to delete those entries.
+  // Function accepts a half-open range [start, end) as input, but the database
+  // operates on fully-closed ranges. Because the timestamps are represented by
+  // integers, [start, end) is equivalent to [start, end - 1].
+  proto_db_->LoadKeysAndEntriesInRange(
+      CreateWebsiteEventKey(start, ""), CreateWebsiteEventKey(end - 1, ""),
+      base::BindOnce(&UsageStatsDatabase::OnLoadEntriesForDeleteEventsInRange,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
 void UsageStatsDatabase::DeleteEventsWithMatchingDomains(
     base::flat_set<std::string> domains,
     StatusCallback callback) {
@@ -212,13 +254,13 @@ void UsageStatsDatabase::SetTokenMappings(TokenMap mappings,
 }
 
 void UsageStatsDatabase::OnUpdateEntries(StatusCallback callback,
-                                         bool success) {
-  std::move(callback).Run(ToError(success));
+                                         bool isSuccess) {
+  std::move(callback).Run(ToError(isSuccess));
 }
 
 void UsageStatsDatabase::OnLoadEntriesForGetAllEvents(
     EventsCallback callback,
-    bool success,
+    bool isSuccess,
     std::unique_ptr<std::vector<UsageStat>> stats) {
   std::vector<WebsiteEvent> results;
 
@@ -229,12 +271,53 @@ void UsageStatsDatabase::OnLoadEntriesForGetAllEvents(
     }
   }
 
-  std::move(callback).Run(ToError(success), std::move(results));
+  std::move(callback).Run(ToError(isSuccess), std::move(results));
+}
+
+void UsageStatsDatabase::OnLoadEntriesForQueryEventsInRange(
+    EventsCallback callback,
+    bool isSuccess,
+    std::unique_ptr<UsageStatMap> stat_map) {
+  std::vector<WebsiteEvent> results;
+
+  if (stat_map) {
+    results.reserve(stat_map->size());
+    for (const auto& key_stat : *stat_map) {
+      results.emplace_back(key_stat.second.website_event());
+    }
+  }
+
+  std::move(callback).Run(ToError(isSuccess), std::move(results));
+}
+
+void UsageStatsDatabase::OnLoadEntriesForDeleteEventsInRange(
+    StatusCallback callback,
+    bool isSuccess,
+    std::unique_ptr<UsageStatMap> stat_map) {
+  if (isSuccess && stat_map) {
+    // Collect keys found in range to be deleted.
+    auto keys_to_delete = std::make_unique<std::vector<std::string>>();
+    keys_to_delete->reserve(stat_map->size());
+
+    for (const auto& key_stat : *stat_map) {
+      keys_to_delete->emplace_back(key_stat.first);
+    }
+
+    // Remove all entries found in range.
+    proto_db_->UpdateEntries(
+        std::make_unique<
+            leveldb_proto::ProtoDatabase<UsageStat>::KeyEntryVector>(),
+        std::move(keys_to_delete),
+        base::BindOnce(&UsageStatsDatabase::OnUpdateEntries,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+  } else {
+    std::move(callback).Run(ToError(isSuccess));
+  }
 }
 
 void UsageStatsDatabase::OnLoadEntriesForGetAllSuspensions(
     SuspensionsCallback callback,
-    bool success,
+    bool isSuccess,
     std::unique_ptr<std::vector<UsageStat>> stats) {
   std::vector<std::string> results;
 
@@ -245,12 +328,12 @@ void UsageStatsDatabase::OnLoadEntriesForGetAllSuspensions(
     }
   }
 
-  std::move(callback).Run(ToError(success), std::move(results));
+  std::move(callback).Run(ToError(isSuccess), std::move(results));
 }
 
 void UsageStatsDatabase::OnLoadEntriesForGetAllTokenMappings(
     TokenMappingsCallback callback,
-    bool success,
+    bool isSuccess,
     std::unique_ptr<std::vector<UsageStat>> stats) {
   TokenMap results;
 
@@ -261,7 +344,7 @@ void UsageStatsDatabase::OnLoadEntriesForGetAllTokenMappings(
     }
   }
 
-  std::move(callback).Run(ToError(success), std::move(results));
+  std::move(callback).Run(ToError(isSuccess), std::move(results));
 }
 
 }  // namespace usage_stats

chrome/browser/android/usage_stats/usage_stats_database.h

@@ -36,6 +36,8 @@ class UsageStatsDatabase {
 
   using StatusCallback = base::OnceCallback<void(Error)>;
 
+  using UsageStatMap = std::map<std::string, UsageStat>;
+
   // Initializes the database with user |profile|.
   explicit UsageStatsDatabase(Profile* profile);
 
@@ -47,16 +49,17 @@ class UsageStatsDatabase {
 
   void GetAllEvents(EventsCallback callback);
 
-  // |start| and |end| are timestamps representing milliseconds since the
-  // beginning of the Unix Epoch.
+  // Get all events in range between |start| (inclusive) and |end| (exclusive),
+  // where |start| and |end| are represented by milliseconds since Unix epoch.
   void QueryEventsInRange(int64_t start, int64_t end, EventsCallback callback);
 
   void AddEvents(std::vector<WebsiteEvent> events, StatusCallback callback);
 
   void DeleteAllEvents(StatusCallback callback);
 
-  // |start| and |end| are timestamps representing milliseconds since the
-  // beginning of the Unix Epoch.
+  // Delete all events in range between |start| (inclusive) and |end|
+  // (exclusive), where |start| and |end| are represented by milliseconds since
+  // Unix epoch.
   void DeleteEventsInRange(int64_t start, int64_t end, StatusCallback callback);
 
   void DeleteEventsWithMatchingDomains(base::flat_set<std::string> domains,
@@ -76,21 +79,31 @@ class UsageStatsDatabase {
   void SetTokenMappings(TokenMap mappings, StatusCallback callback);
 
  private:
-  void OnUpdateEntries(StatusCallback callback, bool success);
+  void OnUpdateEntries(StatusCallback callback, bool isSuccess);
 
   void OnLoadEntriesForGetAllEvents(
       EventsCallback callback,
-      bool success,
+      bool isSuccess,
       std::unique_ptr<std::vector<UsageStat>> stats);
 
+  void OnLoadEntriesForQueryEventsInRange(
+      EventsCallback callback,
+      bool isSuccess,
+      std::unique_ptr<UsageStatMap> stat_map);
+
+  void OnLoadEntriesForDeleteEventsInRange(
+      StatusCallback callback,
+      bool isSuccess,
+      std::unique_ptr<UsageStatMap> stat_map);
+
   void OnLoadEntriesForGetAllSuspensions(
       SuspensionsCallback callback,
-      bool success,
+      bool isSuccess,
       std::unique_ptr<std::vector<UsageStat>> stats);
 
   void OnLoadEntriesForGetAllTokenMappings(
       TokenMappingsCallback callback,
-      bool success,
+      bool isSuccess,
       std::unique_ptr<std::vector<UsageStat>> stats);
 
   std::unique_ptr<leveldb_proto::ProtoDatabase<UsageStat>> proto_db_;

chrome/browser/android/usage_stats/usage_stats_database_unittest.cc

@@ -63,7 +63,7 @@ class UsageStatsDatabaseTest : public testing::Test {
   FakeDB<UsageStat>* fake_db() { return fake_db_unowned_; }
 
   MOCK_METHOD1(OnUpdateDone, void(UsageStatsDatabase::Error));
-  MOCK_METHOD2(OnGetAllEventsDone,
+  MOCK_METHOD2(OnGetEventsDone,
                void(UsageStatsDatabase::Error, std::vector<WebsiteEvent>));
   MOCK_METHOD2(OnGetAllSuspensionsDone,
                void(UsageStatsDatabase::Error, std::vector<std::string>));
@@ -85,22 +85,21 @@ TEST_F(UsageStatsDatabaseTest, Initialization) {
 
 // Website Event Tests
 TEST_F(UsageStatsDatabaseTest, GetAllEventsSuccess) {
-  EXPECT_CALL(*this, OnGetAllEventsDone(UsageStatsDatabase::Error::kNoError,
-                                        ElementsAre()));
+  EXPECT_CALL(*this, OnGetEventsDone(UsageStatsDatabase::Error::kNoError,
+                                     ElementsAre()));
 
   usage_stats_database()->GetAllEvents(base::BindOnce(
-      &UsageStatsDatabaseTest::OnGetAllEventsDone, base::Unretained(this)));
+      &UsageStatsDatabaseTest::OnGetEventsDone, base::Unretained(this)));
 
   fake_db()->LoadCallback(true);
 }
 
 TEST_F(UsageStatsDatabaseTest, GetAllEventsFailure) {
-  EXPECT_CALL(*this,
-              OnGetAllEventsDone(UsageStatsDatabase::Error::kUnknownError,
-                                 ElementsAre()));
+  EXPECT_CALL(*this, OnGetEventsDone(UsageStatsDatabase::Error::kUnknownError,
+                                     ElementsAre()));
 
   usage_stats_database()->GetAllEvents(base::BindOnce(
-      &UsageStatsDatabaseTest::OnGetAllEventsDone, base::Unretained(this)));
+      &UsageStatsDatabaseTest::OnGetEventsDone, base::Unretained(this)));
 
   fake_db()->LoadCallback(false);
 }
@@ -118,6 +117,7 @@ TEST_F(UsageStatsDatabaseTest, AddEventsEmpty) {
 }
 
 TEST_F(UsageStatsDatabaseTest, AddAndGetOneEvent) {
+  // Add 1 event.
   WebsiteEvent event1 =
       CreateWebsiteEvent(kFqdn1, 1, WebsiteEvent::START_BROWSING);
   std::vector<WebsiteEvent> events({event1});
@@ -130,18 +130,119 @@ TEST_F(UsageStatsDatabaseTest, AddAndGetOneEvent) {
 
   fake_db()->UpdateCallback(true);
 
-  EXPECT_CALL(*this,
-              OnGetAllEventsDone(UsageStatsDatabase::Error::kNoError,
-                                 ElementsAre(EqualsWebsiteEvent(event1))));
+  // Get 1 event.
+  EXPECT_CALL(*this, OnGetEventsDone(UsageStatsDatabase::Error::kNoError,
+                                     ElementsAre(EqualsWebsiteEvent(event1))));
 
   usage_stats_database()->GetAllEvents(base::BindOnce(
-      &UsageStatsDatabaseTest::OnGetAllEventsDone, base::Unretained(this)));
+      &UsageStatsDatabaseTest::OnGetEventsDone, base::Unretained(this)));
+
+  fake_db()->LoadCallback(true);
+}
+
+TEST_F(UsageStatsDatabaseTest, AddAndQueryEventsInRange) {
+  // Add 2 events at time 5 and 10.
+  WebsiteEvent event1 =
+      CreateWebsiteEvent(kFqdn1, 5, WebsiteEvent::START_BROWSING);
+  WebsiteEvent event2 =
+      CreateWebsiteEvent(kFqdn2, 10, WebsiteEvent::STOP_BROWSING);
+  std::vector<WebsiteEvent> events({event1, event2});
+
+  EXPECT_CALL(*this, OnUpdateDone(UsageStatsDatabase::Error::kNoError));
+
+  usage_stats_database()->AddEvents(
+      events, base::BindOnce(&UsageStatsDatabaseTest::OnUpdateDone,
+                             base::Unretained(this)));
+
+  fake_db()->UpdateCallback(true);
+
+  // Get events between time 0 (inclusive) and 9 (exclusive).
+  // This test validates the correct lexicographic ordering of timestamps such
+  // that key(0) <= key(5) < key(9) <= key(10).
+  EXPECT_CALL(*this, OnGetEventsDone(UsageStatsDatabase::Error::kNoError,
+                                     ElementsAre(EqualsWebsiteEvent(event1))));
+
+  usage_stats_database()->QueryEventsInRange(
+      0, 9,
+      base::BindOnce(&UsageStatsDatabaseTest::OnGetEventsDone,
+                     base::Unretained(this)));
+
+  fake_db()->LoadCallback(true);
+}
+
+TEST_F(UsageStatsDatabaseTest, AddAndDeleteAllEvents) {
+  // Add 1 event.
+  WebsiteEvent event1 =
+      CreateWebsiteEvent(kFqdn1, 1, WebsiteEvent::START_BROWSING);
+  std::vector<WebsiteEvent> events({event1});
+
+  EXPECT_CALL(*this, OnUpdateDone(UsageStatsDatabase::Error::kNoError));
+
+  usage_stats_database()->AddEvents(
+      events, base::BindOnce(&UsageStatsDatabaseTest::OnUpdateDone,
+                             base::Unretained(this)));
+
+  fake_db()->UpdateCallback(true);
+
+  // Delete all events.
+  EXPECT_CALL(*this, OnUpdateDone(UsageStatsDatabase::Error::kNoError));
+
+  usage_stats_database()->DeleteAllEvents(base::BindOnce(
+      &UsageStatsDatabaseTest::OnUpdateDone, base::Unretained(this)));
+
+  fake_db()->UpdateCallback(true);
+
+  // Get all events (expecting none).
+  EXPECT_CALL(*this, OnGetEventsDone(UsageStatsDatabase::Error::kNoError,
+                                     ElementsAre()));
+
+  usage_stats_database()->GetAllEvents(base::BindOnce(
+      &UsageStatsDatabaseTest::OnGetEventsDone, base::Unretained(this)));
+
+  fake_db()->LoadCallback(true);
+}
+
+TEST_F(UsageStatsDatabaseTest, AddAndDeleteEventsInRange) {
+  // Add 3 events.
+  WebsiteEvent event1 =
+      CreateWebsiteEvent(kFqdn1, 1, WebsiteEvent::START_BROWSING);
+  WebsiteEvent event2 =
+      CreateWebsiteEvent(kFqdn1, 2, WebsiteEvent::START_BROWSING);
+  WebsiteEvent event3 =
+      CreateWebsiteEvent(kFqdn1, 10, WebsiteEvent::START_BROWSING);
+  std::vector<WebsiteEvent> events({event1, event2, event3});
+
+  EXPECT_CALL(*this, OnUpdateDone(UsageStatsDatabase::Error::kNoError));
+
+  usage_stats_database()->AddEvents(
+      events, base::BindOnce(&UsageStatsDatabaseTest::OnUpdateDone,
+                             base::Unretained(this)));
+
+  fake_db()->UpdateCallback(true);
+
+  // Delete events between time 1 (inclusive) and 10 (exclusive).
+  EXPECT_CALL(*this, OnUpdateDone(UsageStatsDatabase::Error::kNoError));
+
+  usage_stats_database()->DeleteEventsInRange(
+      1, 10,
+      base::BindOnce(&UsageStatsDatabaseTest::OnUpdateDone,
+                     base::Unretained(this)));
+
+  fake_db()->LoadCallback(true);
+  fake_db()->UpdateCallback(true);
+
+  // Get 1 remaining event outside range (at time 10).
+  EXPECT_CALL(*this, OnGetEventsDone(UsageStatsDatabase::Error::kNoError,
+                                     ElementsAre(EqualsWebsiteEvent(event3))));
+
+  usage_stats_database()->GetAllEvents(base::BindOnce(
+      &UsageStatsDatabaseTest::OnGetEventsDone, base::Unretained(this)));
 
   fake_db()->LoadCallback(true);
 }
 
 TEST_F(UsageStatsDatabaseTest, AddAndDeleteEventsMatchingDomain) {
-  // Add 3 events
+  // Add 3 events.
   WebsiteEvent event1 =
       CreateWebsiteEvent(kFqdn1, 1, WebsiteEvent::START_BROWSING);
   WebsiteEvent event2 =
@@ -158,7 +259,7 @@ TEST_F(UsageStatsDatabaseTest, AddAndDeleteEventsMatchingDomain) {
 
   fake_db()->UpdateCallback(true);
 
-  // Delete 2 events by FQDN
+  // Delete 2 events by FQDN.
   base::flat_set<std::string> domains({kFqdn1});
   EXPECT_CALL(*this, OnUpdateDone(UsageStatsDatabase::Error::kNoError));
 
@@ -168,13 +269,12 @@ TEST_F(UsageStatsDatabaseTest, AddAndDeleteEventsMatchingDomain) {
 
   fake_db()->UpdateCallback(true);
 
-  // Get 1 remaining event with non-matching FQDN
-  EXPECT_CALL(*this,
-              OnGetAllEventsDone(UsageStatsDatabase::Error::kNoError,
-                                 ElementsAre(EqualsWebsiteEvent(event3))));
+  // Get 1 remaining event with non-matching FQDN.
+  EXPECT_CALL(*this, OnGetEventsDone(UsageStatsDatabase::Error::kNoError,
+                                     ElementsAre(EqualsWebsiteEvent(event3))));
 
   usage_stats_database()->GetAllEvents(base::BindOnce(
-      &UsageStatsDatabaseTest::OnGetAllEventsDone, base::Unretained(this)));
+      &UsageStatsDatabaseTest::OnGetEventsDone, base::Unretained(this)));
 
   fake_db()->LoadCallback(true);
 }