[Media History] Merge MEI into origin table

Merge media engagement fields into origin table since
it will be faster.

BUG=1024359

Change-Id: Icdb8cb9e9f7ea2965565c9157f4826110da60c16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036779Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738013}

chrome/browser/BUILD.gn

@@ -685,8 +685,6 @@ jumbo_static_library("browser") {
     "media/cast_remoting_connector.h",
     "media/history/media_history_contents_observer.cc",
     "media/history/media_history_contents_observer.h",
-    "media/history/media_history_engagement_table.cc",
-    "media/history/media_history_engagement_table.h",
     "media/history/media_history_keyed_service.cc",
     "media/history/media_history_keyed_service.h",
     "media/history/media_history_keyed_service_factory.cc",

chrome/browser/media/history/media_history_engagement_table.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/media/history/media_history_engagement_table.h"
-
-#include "base/strings/stringprintf.h"
-#include "sql/statement.h"
-
-namespace media_history {
-
-const char MediaHistoryEngagementTable::kTableName[] = "mediaEngagement";
-
-MediaHistoryEngagementTable::MediaHistoryEngagementTable(
-    scoped_refptr<base::UpdateableSequencedTaskRunner> db_task_runner)
-    : MediaHistoryTableBase(std::move(db_task_runner)) {}
-
-MediaHistoryEngagementTable::~MediaHistoryEngagementTable() = default;
-
-sql::InitStatus MediaHistoryEngagementTable::CreateTableIfNonExistent() {
-  if (!CanAccessDatabase())
-    return sql::INIT_FAILURE;
-
-  bool success =
-      DB()->Execute(base::StringPrintf("CREATE TABLE IF NOT EXISTS %s("
-                                       "origin_id INTEGER PRIMARY KEY,"
-                                       "last_updated INTEGER,"
-                                       "visits INTEGER,"
-                                       "playbacks INTEGER,"
-                                       "last_playback_time REAL,"
-                                       "has_high_score INTEGER,"
-                                       "CONSTRAINT fk_origin "
-                                       "FOREIGN KEY (origin_id) "
-                                       "REFERENCES origin(id) "
-                                       "ON DELETE CASCADE"
-                                       ")",
-                                       kTableName)
-                        .c_str());
-
-  if (!success) {
-    ResetDB();
-    LOG(ERROR) << "Failed to create media history engagement table.";
-    return sql::INIT_FAILURE;
-  }
-
-  return sql::INIT_OK;
-}
-
-}  // namespace media_history

chrome/browser/media/history/media_history_engagement_table.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_MEDIA_HISTORY_MEDIA_HISTORY_ENGAGEMENT_TABLE_H_
-#define CHROME_BROWSER_MEDIA_HISTORY_MEDIA_HISTORY_ENGAGEMENT_TABLE_H_
-
-#include "base/updateable_sequenced_task_runner.h"
-#include "chrome/browser/media/history/media_history_table_base.h"
-#include "sql/init_status.h"
-#include "url/origin.h"
-
-namespace media_history {
-
-class MediaHistoryEngagementTable : public MediaHistoryTableBase {
- public:
-  static const char kTableName[];
-
-  struct MediaEngagementScore {
-    MediaEngagementScore();
-
-    url::Origin& origin;
-    base::TimeDelta last_updated;  // timestamp (epoch seconds)
-    int visits;
-    int playbacks;
-    base::TimeDelta last_playback_time;
-    bool has_high_score;
-  };
-
- private:
-  friend class MediaHistoryStoreInternal;
-
-  explicit MediaHistoryEngagementTable(
-      scoped_refptr<base::UpdateableSequencedTaskRunner> db_task_runner);
-  ~MediaHistoryEngagementTable() override;
-
-  // MediaHistoryTableBase:
-  sql::InitStatus CreateTableIfNonExistent() override;
-
-  DISALLOW_COPY_AND_ASSIGN(MediaHistoryEngagementTable);
-};
-
-}  // namespace media_history
-
-#endif  // CHROME_BROWSER_MEDIA_HISTORY_MEDIA_HISTORY_ENGAGEMENT_TABLE_H_

chrome/browser/media/history/media_history_origin_table.cc

@@ -21,12 +21,18 @@ sql::InitStatus MediaHistoryOriginTable::CreateTableIfNonExistent() {
   if (!CanAccessDatabase())
     return sql::INIT_FAILURE;
 
-  bool success =
-      DB()->Execute(base::StringPrintf("CREATE TABLE IF NOT EXISTS %s("
-                                       "id INTEGER PRIMARY KEY AUTOINCREMENT,"
-                                       "origin TEXT NOT NULL UNIQUE)",
-                                       kTableName)
-                        .c_str());
+  bool success = DB()->Execute(
+      base::StringPrintf("CREATE TABLE IF NOT EXISTS %s("
+                         "id INTEGER PRIMARY KEY AUTOINCREMENT,"
+                         "origin TEXT NOT NULL UNIQUE, "
+                         "last_updated_time_s INTEGER,"
+                         "has_media_engagement INTEGER, "
+                         "media_engagement_visits INTEGER,"
+                         "media_engagement_playbacks INTEGER,"
+                         "media_engagement_last_playback_time REAL,"
+                         "media_engagement_has_high_score INTEGER)",
+                         kTableName)
+          .c_str());
 
   if (!success) {
     ResetDB();
@@ -44,11 +50,14 @@ bool MediaHistoryOriginTable::CreateOriginId(const std::string& origin) {
 
   // Insert the origin into the table if it does not exist.
   sql::Statement statement(DB()->GetCachedStatement(
-      SQL_FROM_HERE, base::StringPrintf("INSERT OR IGNORE INTO %s"
-                                        "(origin) VALUES (?)",
-                                        kTableName)
-                         .c_str()));
+      SQL_FROM_HERE,
+      base::StringPrintf("INSERT OR IGNORE INTO %s"
+                         "(origin, last_updated_time_s) VALUES (?, ?)",
+                         kTableName)
+          .c_str()));
   statement.BindString(0, origin);
+  statement.BindInt64(1,
+                      base::Time::Now().ToDeltaSinceWindowsEpoch().InSeconds());
   if (!statement.Run()) {
     LOG(ERROR) << "Failed to create the origin ID.";
     return false;

chrome/browser/media/history/media_history_store.cc

@@ -8,13 +8,13 @@
 #include "base/files/file_path.h"
 #include "base/strings/stringprintf.h"
 #include "base/task_runner_util.h"
-#include "chrome/browser/media/history/media_history_engagement_table.h"
 #include "chrome/browser/media/history/media_history_origin_table.h"
 #include "chrome/browser/media/history/media_history_playback_table.h"
 #include "chrome/browser/media/history/media_history_session_table.h"
 #include "content/public/browser/media_player_watch_time.h"
 #include "services/media_session/public/cpp/media_position.h"
 #include "sql/statement.h"
+#include "url/origin.h"
 
 namespace {
 
@@ -78,7 +78,6 @@ class MediaHistoryStoreInternal
   const base::FilePath db_path_;
   std::unique_ptr<sql::Database> db_;
   sql::MetaTable meta_table_;
-  scoped_refptr<MediaHistoryEngagementTable> engagement_table_;
   scoped_refptr<MediaHistoryOriginTable> origin_table_;
   scoped_refptr<MediaHistoryPlaybackTable> playback_table_;
   scoped_refptr<MediaHistorySessionTable> session_table_;
@@ -92,14 +91,12 @@ MediaHistoryStoreInternal::MediaHistoryStoreInternal(
     scoped_refptr<base::UpdateableSequencedTaskRunner> db_task_runner)
     : db_task_runner_(db_task_runner),
       db_path_(profile->GetPath().Append(kMediaHistoryDatabaseName)),
-      engagement_table_(new MediaHistoryEngagementTable(db_task_runner_)),
       origin_table_(new MediaHistoryOriginTable(db_task_runner_)),
       playback_table_(new MediaHistoryPlaybackTable(db_task_runner_)),
       session_table_(new MediaHistorySessionTable(db_task_runner_)),
       initialization_successful_(false) {}
 
 MediaHistoryStoreInternal::~MediaHistoryStoreInternal() {
-  db_task_runner_->ReleaseSoon(FROM_HERE, std::move(engagement_table_));
   db_task_runner_->ReleaseSoon(FROM_HERE, std::move(origin_table_));
   db_task_runner_->ReleaseSoon(FROM_HERE, std::move(playback_table_));
   db_task_runner_->ReleaseSoon(FROM_HERE, std::move(session_table_));
@@ -175,9 +172,7 @@ sql::InitStatus MediaHistoryStoreInternal::CreateOrUpgradeIfNeeded() {
 
 sql::InitStatus MediaHistoryStoreInternal::InitializeTables() {
   DCHECK(db_task_runner_->RunsTasksInCurrentSequence());
-  sql::InitStatus status = engagement_table_->Initialize(db_.get());
-  if (status == sql::INIT_OK)
-    status = origin_table_->Initialize(db_.get());
+  sql::InitStatus status = origin_table_->Initialize(db_.get());
   if (status == sql::INIT_OK)
     status = playback_table_->Initialize(db_.get());
   if (status == sql::INIT_OK)

chrome/browser/media/history/media_history_store.h

@@ -13,7 +13,6 @@
 #include "base/memory/ref_counted.h"
 #include "base/optional.h"
 #include "base/updateable_sequenced_task_runner.h"
-#include "chrome/browser/media/history/media_history_engagement_table.h"
 #include "chrome/browser/media/history/media_history_origin_table.h"
 #include "chrome/browser/media/history/media_history_playback_table.h"
 #include "chrome/browser/media/history/media_history_store.mojom.h"

chrome/browser/media/history/media_history_store_unittest.cc

@@ -86,9 +86,9 @@ class MediaHistoryStoreUnitTest : public testing::Test {
 };
 
 TEST_F(MediaHistoryStoreUnitTest, CreateDatabaseTables) {
-  ASSERT_TRUE(GetDB().DoesTableExist("mediaEngagement"));
   ASSERT_TRUE(GetDB().DoesTableExist("origin"));
   ASSERT_TRUE(GetDB().DoesTableExist("playback"));
+  ASSERT_TRUE(GetDB().DoesTableExist("playbackSession"));
 }
 
 TEST_F(MediaHistoryStoreUnitTest, SavePlayback) {
@@ -135,8 +135,8 @@ TEST_F(MediaHistoryStoreUnitTest, SavePlayback) {
   EXPECT_EQ(2, playback_row_count);
 
   // Verify that the origin table contains the expected number of items
-  sql::Statement select_from_origin_statement(
-      GetDB().GetUniqueStatement("SELECT id, origin FROM origin"));
+  sql::Statement select_from_origin_statement(GetDB().GetUniqueStatement(
+      "SELECT id, origin, last_updated_time_s FROM origin"));
   ASSERT_TRUE(select_from_origin_statement.is_valid());
   int origin_row_count = 0;
   while (select_from_origin_statement.Step()) {
@@ -144,6 +144,10 @@ TEST_F(MediaHistoryStoreUnitTest, SavePlayback) {
     EXPECT_EQ(1, select_from_origin_statement.ColumnInt(0));
     EXPECT_EQ("http://google.com/",
               select_from_origin_statement.ColumnString(1));
+    EXPECT_LE(now_in_seconds_before,
+              select_from_origin_statement.ColumnInt64(2));
+    EXPECT_GE(now_in_seconds_after,
+              select_from_origin_statement.ColumnInt64(2));
   }
 
   EXPECT_EQ(1, origin_row_count);
@@ -156,8 +160,6 @@ TEST_F(MediaHistoryStoreUnitTest, GetStats) {
     EXPECT_EQ(0, stats->table_row_counts[MediaHistoryOriginTable::kTableName]);
     EXPECT_EQ(0,
               stats->table_row_counts[MediaHistoryPlaybackTable::kTableName]);
-    EXPECT_EQ(0,
-              stats->table_row_counts[MediaHistoryEngagementTable::kTableName]);
     EXPECT_EQ(0, stats->table_row_counts[MediaHistorySessionTable::kTableName]);
   }
 
@@ -176,8 +178,6 @@ TEST_F(MediaHistoryStoreUnitTest, GetStats) {
     EXPECT_EQ(1, stats->table_row_counts[MediaHistoryOriginTable::kTableName]);
     EXPECT_EQ(1,
               stats->table_row_counts[MediaHistoryPlaybackTable::kTableName]);
-    EXPECT_EQ(0,
-              stats->table_row_counts[MediaHistoryEngagementTable::kTableName]);
     EXPECT_EQ(0, stats->table_row_counts[MediaHistorySessionTable::kTableName]);
   }
 }

chrome/test/data/webui/media/media_history_webui_browsertest.js

@@ -42,12 +42,12 @@ TEST_F('MediaHistoryWebUIBrowserTest', 'MAYBE_All', function() {
   test('check stats table is loaded', function() {
     let statsRows =
         Array.from(document.getElementById('stats-table-body').children);
-    assertEquals(5, statsRows.length);
+    assertEquals(4, statsRows.length);
 
     assertDeepEquals(
         [
-          ['mediaEngagement', '0'], ['meta', '3'], ['origin', '0'],
-          ['playback', '0'], ['playbackSession', '0']
+          ['meta', '3'], ['origin', '0'], ['playback', '0'],
+          ['playbackSession', '0']
         ],
         statsRows.map(
             x => [x.children[0].textContent, x.children[1].textContent]));