Revert "[Media History] Retain refs and remove fatal"

This reverts commit ae8f4133.

Reason for revert: Crashes

Original change's description:
> [Media History] Retain refs and remove fatal
> 
> Retains refs to the store when using callbacks
> and also change the fatal -> error for sql
> errors since it is making bots crash.
> 
> Change-Id: Ie27d43f2044700e1b299d2a34406b1eb2fc9412b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2149405
> Reviewed-by: Tommy Steimel <steimel@chromium.org>
> Commit-Queue: Becca Hughes <beccahughes@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#759814}

TBR=beccahughes@chromium.org,steimel@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Ib90f55a1176af4ace35d0e421fc5b0ba667ccadd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2157165Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760521}

chrome/browser/media/history/media_history_keyed_service.cc

@@ -108,8 +108,7 @@ void MediaHistoryKeyedService::Shutdown() {
 
   if (auto* store = store_->GetForWrite()) {
     store->db_task_runner_->PostTask(
-        FROM_HERE,
-        base::BindOnce(&MediaHistoryStore::Close, base::RetainedRef(store)));
+        FROM_HERE, base::BindOnce(&MediaHistoryStore::Close, store));
   }
 }
 
@@ -125,9 +124,9 @@ void MediaHistoryKeyedService::OnURLsDeleted(
     // Destroy the old database and create a new one.
     auto db_task_runner = store->db_task_runner_;
 
-    db_task_runner->PostTask(
-        FROM_HERE, base::BindOnce(&MediaHistoryStore::RazeAndClose,
-                                  base::RetainedRef(store_->GetForDelete())));
+    db_task_runner->PostTask(FROM_HERE,
+                             base::BindOnce(&MediaHistoryStore::RazeAndClose,
+                                            store_->GetForDelete()));
 
     // Create a new internal store.
     store_ = std::make_unique<StoreHolder>(profile_, std::move(db_task_runner));
@@ -155,7 +154,7 @@ void MediaHistoryKeyedService::OnURLsDeleted(
   if (!deleted_origins.empty()) {
     store->db_task_runner_->PostTask(
         FROM_HERE, base::BindOnce(&MediaHistoryStore::DeleteAllOriginData,
-                                  base::RetainedRef(store), origins));
+                                  store, origins));
   }
 
   // Build a set of all urls in |deleted_rows| that do not have their origin in
@@ -172,8 +171,8 @@ void MediaHistoryKeyedService::OnURLsDeleted(
 
   if (!deleted_urls.empty()) {
     store->db_task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(&MediaHistoryStore::DeleteAllURLData,
-                                  base::RetainedRef(store), deleted_urls));
+        FROM_HERE, base::BindOnce(&MediaHistoryStore::DeleteAllURLData, store,
+                                  deleted_urls));
   }
 }
 
@@ -181,8 +180,8 @@ void MediaHistoryKeyedService::SavePlayback(
     const content::MediaPlayerWatchTime& watch_time) {
   if (auto* store = store_->GetForWrite()) {
     store->db_task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(&MediaHistoryStore::SavePlayback,
-                                  base::RetainedRef(store), watch_time));
+        FROM_HERE,
+        base::BindOnce(&MediaHistoryStore::SavePlayback, store, watch_time));
   }
 }
 
@@ -191,7 +190,7 @@ void MediaHistoryKeyedService::GetMediaHistoryStats(
   base::PostTaskAndReplyWithResult(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
       base::BindOnce(&MediaHistoryStore::GetMediaHistoryStats,
-                     base::RetainedRef(store_->GetForRead())),
+                     store_->GetForRead()),
       std::move(callback));
 }
 
@@ -201,7 +200,7 @@ void MediaHistoryKeyedService::GetOriginRowsForDebug(
   base::PostTaskAndReplyWithResult(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
       base::BindOnce(&MediaHistoryStore::GetOriginRowsForDebug,
-                     base::RetainedRef(store_->GetForRead())),
+                     store_->GetForRead()),
       std::move(callback));
 }
 
@@ -211,7 +210,7 @@ void MediaHistoryKeyedService::GetMediaHistoryPlaybackRowsForDebug(
   base::PostTaskAndReplyWithResult(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
       base::BindOnce(&MediaHistoryStore::GetMediaHistoryPlaybackRowsForDebug,
-                     base::RetainedRef(store_->GetForRead())),
+                     store_->GetForRead()),
       std::move(callback));
 }
 
@@ -223,8 +222,7 @@ void MediaHistoryKeyedService::GetPlaybackSessions(
   base::PostTaskAndReplyWithResult(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
       base::BindOnce(&MediaHistoryStore::GetPlaybackSessions,
-                     base::RetainedRef(store_->GetForRead()), num_sessions,
-                     std::move(filter)),
+                     store_->GetForRead(), num_sessions, std::move(filter)),
       std::move(callback));
 }
 
@@ -236,8 +234,7 @@ void MediaHistoryKeyedService::SavePlaybackSession(
   if (auto* store = store_->GetForWrite()) {
     store->db_task_runner_->PostTask(
         FROM_HERE, base::BindOnce(&MediaHistoryStore::SavePlaybackSession,
-                                  base::RetainedRef(store), url, metadata,
-                                  position, artwork));
+                                  store, url, metadata, position, artwork));
   }
 }
 
@@ -248,7 +245,7 @@ void MediaHistoryKeyedService::GetItemsForMediaFeedForDebug(
   base::PostTaskAndReplyWithResult(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
       base::BindOnce(&MediaHistoryStore::GetItemsForMediaFeedForDebug,
-                     base::RetainedRef(store_->GetForRead()), feed_id),
+                     store_->GetForRead(), feed_id),
       std::move(callback));
 }
 
@@ -263,9 +260,9 @@ void MediaHistoryKeyedService::StoreMediaFeedFetchResult(
   if (auto* store = store_->GetForWrite()) {
     store->db_task_runner_->PostTaskAndReply(
         FROM_HERE,
-        base::BindOnce(&MediaHistoryStore::StoreMediaFeedFetchResult,
-                       base::RetainedRef(store), feed_id, std::move(items),
-                       result, was_fetched_from_cache, logos, display_name),
+        base::BindOnce(&MediaHistoryStore::StoreMediaFeedFetchResult, store,
+                       feed_id, std::move(items), result,
+                       was_fetched_from_cache, logos, display_name),
         std::move(callback));
   }
 }
@@ -276,15 +273,15 @@ void MediaHistoryKeyedService::GetURLsInTableForTest(
   base::PostTaskAndReplyWithResult(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
       base::BindOnce(&MediaHistoryStore::GetURLsInTableForTest,
-                     base::RetainedRef(store_->GetForRead()), table),
+                     store_->GetForRead(), table),
       std::move(callback));
 }
 
 void MediaHistoryKeyedService::DiscoverMediaFeed(const GURL& url) {
   if (auto* store = store_->GetForWrite()) {
     store->db_task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(&MediaHistoryStore::DiscoverMediaFeed,
-                                  base::RetainedRef(store), url));
+        FROM_HERE,
+        base::BindOnce(&MediaHistoryStore::DiscoverMediaFeed, store, url));
   }
 }
 
@@ -301,7 +298,7 @@ void MediaHistoryKeyedService::GetPendingSafeSearchCheckMediaFeedItems(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
       base::BindOnce(
           &MediaHistoryStore::GetPendingSafeSearchCheckMediaFeedItems,
-          base::RetainedRef(store_->GetForRead())),
+          store_->GetForRead()),
       std::move(callback));
 }
 
@@ -311,7 +308,7 @@ void MediaHistoryKeyedService::StoreMediaFeedItemSafeSearchResults(
     store->db_task_runner_->PostTask(
         FROM_HERE,
         base::BindOnce(&MediaHistoryStore::StoreMediaFeedItemSafeSearchResults,
-                       base::RetainedRef(store), results));
+                       store, results));
   }
 }
 
@@ -341,8 +338,8 @@ void MediaHistoryKeyedService::GetMediaFeeds(
         callback) {
   base::PostTaskAndReplyWithResult(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
-      base::BindOnce(&MediaHistoryStore::GetMediaFeeds,
-                     base::RetainedRef(store_->GetForRead()), request),
+      base::BindOnce(&MediaHistoryStore::GetMediaFeeds, store_->GetForRead(),
+                     request),
       std::move(callback));
 }
 

chrome/browser/media/history/media_history_store.cc

@@ -24,7 +24,6 @@
 #include "sql/recovery.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
-#include "third_party/sqlite/sqlite3.h"
 #include "url/origin.h"
 
 namespace {
@@ -47,7 +46,7 @@ void DatabaseErrorCallback(sql::Database* db,
     // return errors until the handle is re-opened.
     sql::Recovery::RecoverDatabase(db, db_path);
 
-    // The DLOG(ERROR) below is intended to draw immediate attention to errors
+    // The DLOG(FATAL) below is intended to draw immediate attention to errors
     // in newly-written code.  Database corruption is generally a result of OS
     // or hardware issues, not coding errors at the client level, so displaying
     // the error would probably lead to confusion.  The ignored call signals the
@@ -56,9 +55,9 @@ void DatabaseErrorCallback(sql::Database* db,
     return;
   }
 
-  // The default handling is to log on debug and to ignore on release.
+  // The default handling is to assert on debug and to ignore on release.
   if (!sql::Database::IsExpectedSqliteError(extended_error))
-    DLOG(ERROR) << db->GetErrorMessage();
+    DLOG(FATAL) << db->GetErrorMessage();
 }
 
 }  // namespace
@@ -101,7 +100,7 @@ MediaHistoryStore::MediaHistoryStore(
       initialization_successful_(false) {
   db_task_runner_->PostTask(
       FROM_HERE,
-      base::BindOnce(&MediaHistoryStore::Initialize, base::RetainedRef(this)));
+      base::BindOnce(&MediaHistoryStore::Initialize, base::Unretained(this)));
 }
 
 MediaHistoryStore::~MediaHistoryStore() {

chrome/browser/media/history/media_history_table_base.cc

@@ -25,10 +25,6 @@ sql::InitStatus MediaHistoryTableBase::Initialize(sql::Database* db) {
   DCHECK(db);
 
   db_ = db;
-
-  if (!db->is_open())
-    return sql::InitStatus::INIT_FAILURE;
-
   return CreateTableIfNonExistent();
 }