[Media History] Make store API private

This makes the store API private and moves
it to MediaHistoryKeyedService. This is so
we can create a read only version of MHKS
in the future for incognito.

BUG=1051239

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

chrome/browser/media/history/media_history_browsertest.cc

@@ -91,7 +91,7 @@ class MediaHistoryBrowserTest : public InProcessBrowserTest {
     base::RunLoop run_loop;
     std::vector<mojom::MediaHistoryPlaybackSessionRowPtr> out;
 
-    GetMediaHistoryStore()->GetPlaybackSessions(
+    GetMediaHistoryService()->GetPlaybackSessions(
         max_sessions, std::move(filter),
         base::BindLambdaForTesting(
             [&](std::vector<mojom::MediaHistoryPlaybackSessionRowPtr>
@@ -108,7 +108,7 @@ class MediaHistoryBrowserTest : public InProcessBrowserTest {
     base::RunLoop run_loop;
     mojom::MediaHistoryStatsPtr stats_out;
 
-    GetMediaHistoryStore()->GetMediaHistoryStats(
+    GetMediaHistoryService()->GetMediaHistoryStats(
         base::BindLambdaForTesting([&](mojom::MediaHistoryStatsPtr stats) {
           stats_out = std::move(stats);
           run_loop.Quit();
@@ -213,9 +213,8 @@ class MediaHistoryBrowserTest : public InProcessBrowserTest {
     return content::MediaSession::Get(GetWebContents());
   }
 
-  MediaHistoryStore* GetMediaHistoryStore() {
-    return MediaHistoryKeyedServiceFactory::GetForProfile(browser()->profile())
-        ->GetMediaHistoryStore();
+  MediaHistoryKeyedService* GetMediaHistoryService() {
+    return MediaHistoryKeyedServiceFactory::GetForProfile(browser()->profile());
   }
 
  private:

chrome/browser/media/history/media_history_contents_observer.cc

@@ -62,7 +62,7 @@ void MediaHistoryContentsObserver::WebContentsDestroyed() {
 void MediaHistoryContentsObserver::MediaWatchTimeChanged(
     const content::MediaPlayerWatchTime& watch_time) {
   if (service_)
-    service_->GetMediaHistoryStore()->SavePlayback(watch_time);
+    service_->SavePlayback(watch_time);
 }
 
 void MediaHistoryContentsObserver::MediaSessionInfoChanged(
@@ -112,8 +112,8 @@ void MediaHistoryContentsObserver::MaybeCommitMediaSession() {
     return;
   }
 
-  service_->GetMediaHistoryStore()->SavePlaybackSession(
-      current_url_, *cached_metadata_, cached_position_, cached_artwork_);
+  service_->SavePlaybackSession(current_url_, *cached_metadata_,
+                                cached_position_, cached_artwork_);
 }
 
 WEB_CONTENTS_USER_DATA_KEY_IMPL(MediaHistoryContentsObserver)

chrome/browser/media/history/media_history_keyed_service.cc

@@ -86,4 +86,54 @@ void MediaHistoryKeyedService::OnURLsDeleted(
   // should remove data by URL.
 }
 
+void MediaHistoryKeyedService::SavePlayback(
+    const content::MediaPlayerWatchTime& watch_time) {
+  media_history_store_->SavePlayback(watch_time);
+}
+
+void MediaHistoryKeyedService::GetMediaHistoryStats(
+    base::OnceCallback<void(mojom::MediaHistoryStatsPtr)> callback) {
+  media_history_store_->GetMediaHistoryStats(std::move(callback));
+}
+
+void MediaHistoryKeyedService::GetOriginRowsForDebug(
+    base::OnceCallback<void(std::vector<mojom::MediaHistoryOriginRowPtr>)>
+        callback) {
+  media_history_store_->GetOriginRowsForDebug(std::move(callback));
+}
+
+void MediaHistoryKeyedService::GetMediaHistoryPlaybackRowsForDebug(
+    base::OnceCallback<void(std::vector<mojom::MediaHistoryPlaybackRowPtr>)>
+        callback) {
+  media_history_store_->GetMediaHistoryPlaybackRowsForDebug(
+      std::move(callback));
+}
+
+void MediaHistoryKeyedService::GetPlaybackSessions(
+    base::Optional<unsigned int> num_sessions,
+    base::Optional<GetPlaybackSessionsFilter> filter,
+    base::OnceCallback<
+        void(std::vector<mojom::MediaHistoryPlaybackSessionRowPtr>)> callback) {
+  media_history_store_->GetPlaybackSessions(num_sessions, filter,
+                                            std::move(callback));
+}
+
+void MediaHistoryKeyedService::SavePlaybackSession(
+    const GURL& url,
+    const media_session::MediaMetadata& metadata,
+    const base::Optional<media_session::MediaPosition>& position,
+    const std::vector<media_session::MediaImage>& artwork) {
+  media_history_store_->SavePlaybackSession(url, metadata, position, artwork);
+}
+
+void MediaHistoryKeyedService::GetURLsInTableForTest(
+    const std::string& table,
+    base::OnceCallback<void(std::set<GURL>)> callback) {
+  media_history_store_->GetURLsInTableForTest(table, std::move(callback));
+}
+
+void MediaHistoryKeyedService::SaveMediaFeed(const GURL& url) {
+  media_history_store_->SaveMediaFeed(url);
+}
+
 }  // namespace media_history

chrome/browser/media/history/media_history_keyed_service.h

@@ -12,6 +12,12 @@
 
 class Profile;
 
+namespace media_session {
+struct MediaImage;
+struct MediaMetadata;
+struct MediaPosition;
+}  // namespace media_session
+
 namespace history {
 class HistoryService;
 }  // namespace history
@@ -29,10 +35,6 @@ class MediaHistoryKeyedService : public KeyedService,
   // Returns the instance attached to the given |profile|.
   static MediaHistoryKeyedService* Get(Profile* profile);
 
-  MediaHistoryStore* GetMediaHistoryStore() {
-    return media_history_store_.get();
-  }
-
   // Overridden from KeyedService:
   void Shutdown() override;
 
@@ -40,6 +42,53 @@ class MediaHistoryKeyedService : public KeyedService,
   void OnURLsDeleted(history::HistoryService* history_service,
                      const history::DeletionInfo& deletion_info) override;
 
+  // Saves a playback from a single player in the media history store.
+  void SavePlayback(const content::MediaPlayerWatchTime& watch_time);
+
+  void GetMediaHistoryStats(
+      base::OnceCallback<void(mojom::MediaHistoryStatsPtr)> callback);
+
+  // Returns all the rows in the origin table. This should only be used for
+  // debugging because it is very slow.
+  void GetOriginRowsForDebug(
+      base::OnceCallback<void(std::vector<mojom::MediaHistoryOriginRowPtr>)>
+          callback);
+
+  // Returns all the rows in the playback table. This is only used for
+  // debugging because it loads all rows in the table.
+  void GetMediaHistoryPlaybackRowsForDebug(
+      base::OnceCallback<void(std::vector<mojom::MediaHistoryPlaybackRowPtr>)>
+          callback);
+
+  // Gets the playback sessions from the media history store. The results will
+  // be ordered by most recent first and be limited to the first |num_sessions|.
+  // For each session it calls |filter| and if that returns |true| then that
+  // session will be included in the results.
+  using GetPlaybackSessionsFilter =
+      base::RepeatingCallback<bool(const base::TimeDelta& duration,
+                                   const base::TimeDelta& position)>;
+  void GetPlaybackSessions(
+      base::Optional<unsigned int> num_sessions,
+      base::Optional<GetPlaybackSessionsFilter> filter,
+      base::OnceCallback<void(
+          std::vector<mojom::MediaHistoryPlaybackSessionRowPtr>)> callback);
+
+  // Saves a playback session in the media history store.
+  void SavePlaybackSession(
+      const GURL& url,
+      const media_session::MediaMetadata& metadata,
+      const base::Optional<media_session::MediaPosition>& position,
+      const std::vector<media_session::MediaImage>& artwork);
+
+  // Saves a newly discovered media feed in the media history store.
+  void SaveMediaFeed(const GURL& url);
+
+ protected:
+  friend class MediaHistoryKeyedServiceTest;
+
+  void GetURLsInTableForTest(const std::string& table,
+                             base::OnceCallback<void(std::set<GURL>)> callback);
+
  private:
   std::unique_ptr<MediaHistoryStore> media_history_store_;
 

chrome/browser/media/history/media_history_keyed_service_unittest.cc

@@ -114,7 +114,7 @@ class MediaHistoryKeyedServiceTest : public ChromeRenderViewHostTestHarness {
     base::RunLoop run_loop;
     mojom::MediaHistoryStatsPtr stats_out;
 
-    service()->GetMediaHistoryStore()->GetMediaHistoryStats(base::BindOnce(
+    service()->GetMediaHistoryStats(base::BindOnce(
         [](mojom::MediaHistoryStatsPtr* stats_out,
            base::RepeatingClosure callback, mojom::MediaHistoryStatsPtr stats) {
           stats_out->Swap(&stats);
@@ -130,7 +130,7 @@ class MediaHistoryKeyedServiceTest : public ChromeRenderViewHostTestHarness {
     base::RunLoop run_loop;
     std::set<GURL> out;
 
-    service()->GetMediaHistoryStore()->GetURLsInTableForTest(
+    service()->GetURLsInTableForTest(
         table, base::BindLambdaForTesting([&](std::set<GURL> urls) {
           out = urls;
           run_loop.Quit();
@@ -176,7 +176,7 @@ TEST_F(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenHistoryIsDeleted) {
         base::TimeDelta::FromMilliseconds(321), true, false);
 
     history->AddPage(url, base::Time::Now(), history::SOURCE_BROWSED);
-    service()->GetMediaHistoryStore()->SavePlayback(watch_time);
+    service()->SavePlayback(watch_time);
 
     // Wait until the playbacks have finished saving.
     content::RunAllTasksUntilIdle();
@@ -253,11 +253,11 @@ TEST_F(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenOriginIsDeleted) {
         base::TimeDelta::FromMilliseconds(321), true, false);
 
     history->AddPage(url1a, base::Time::Now(), history::SOURCE_BROWSED);
-    service()->GetMediaHistoryStore()->SavePlayback(watch_time);
+    service()->SavePlayback(watch_time);
 
-    service()->GetMediaHistoryStore()->SavePlaybackSession(
-        url1a, media_session::MediaMetadata(), base::nullopt,
-        CreateImageVector(url1a_image));
+    service()->SavePlaybackSession(url1a, media_session::MediaMetadata(),
+                                   base::nullopt,
+                                   CreateImageVector(url1a_image));
   }
 
   // Record a playback in the database for |url1b|.
@@ -267,11 +267,11 @@ TEST_F(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenOriginIsDeleted) {
         base::TimeDelta::FromMilliseconds(321), true, false);
 
     history->AddPage(url1b, base::Time::Now(), history::SOURCE_BROWSED);
-    service()->GetMediaHistoryStore()->SavePlayback(watch_time);
+    service()->SavePlayback(watch_time);
 
-    service()->GetMediaHistoryStore()->SavePlaybackSession(
-        url1b, media_session::MediaMetadata(), base::nullopt,
-        CreateImageVector(url1b_image));
+    service()->SavePlaybackSession(url1b, media_session::MediaMetadata(),
+                                   base::nullopt,
+                                   CreateImageVector(url1b_image));
   }
 
   // Record a playback in the database for |url1c|. However, we won't store it
@@ -281,11 +281,11 @@ TEST_F(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenOriginIsDeleted) {
     content::MediaPlayerWatchTime watch_time(
         url1c, url1c.GetOrigin(), base::TimeDelta::FromMilliseconds(123),
         base::TimeDelta::FromMilliseconds(321), true, false);
-    service()->GetMediaHistoryStore()->SavePlayback(watch_time);
+    service()->SavePlayback(watch_time);
 
-    service()->GetMediaHistoryStore()->SavePlaybackSession(
-        url1c, media_session::MediaMetadata(), base::nullopt,
-        CreateImageVector(shared_image));
+    service()->SavePlaybackSession(url1c, media_session::MediaMetadata(),
+                                   base::nullopt,
+                                   CreateImageVector(shared_image));
   }
 
   // Record a playback in the database for |url2a|.
@@ -295,11 +295,11 @@ TEST_F(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenOriginIsDeleted) {
         base::TimeDelta::FromMilliseconds(321), true, false);
 
     history->AddPage(url2a, base::Time::Now(), history::SOURCE_BROWSED);
-    service()->GetMediaHistoryStore()->SavePlayback(watch_time);
+    service()->SavePlayback(watch_time);
 
-    service()->GetMediaHistoryStore()->SavePlaybackSession(
-        url2a, media_session::MediaMetadata(), base::nullopt,
-        CreateImageVector(url2a_image));
+    service()->SavePlaybackSession(url2a, media_session::MediaMetadata(),
+                                   base::nullopt,
+                                   CreateImageVector(url2a_image));
   }
 
   // Record a playback in the database for |url2b|.
@@ -309,11 +309,11 @@ TEST_F(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenOriginIsDeleted) {
         base::TimeDelta::FromMilliseconds(321), true, false);
 
     history->AddPage(url2b, base::Time::Now(), history::SOURCE_BROWSED);
-    service()->GetMediaHistoryStore()->SavePlayback(watch_time);
+    service()->SavePlayback(watch_time);
 
-    service()->GetMediaHistoryStore()->SavePlaybackSession(
-        url2b, media_session::MediaMetadata(), base::nullopt,
-        CreateImageVector(shared_image));
+    service()->SavePlaybackSession(url2b, media_session::MediaMetadata(),
+                                   base::nullopt,
+                                   CreateImageVector(shared_image));
   }
 
   // Wait until the playbacks have finished saving.

chrome/browser/media/history/media_history_store.h

@@ -49,6 +49,13 @@ class MediaHistoryStore {
       scoped_refptr<base::UpdateableSequencedTaskRunner> db_task_runner);
   ~MediaHistoryStore();
 
+  using GetPlaybackSessionsFilter =
+      base::RepeatingCallback<bool(const base::TimeDelta& duration,
+                                   const base::TimeDelta& position)>;
+
+ protected:
+  friend class MediaHistoryKeyedService;
+
   // Saves a playback from a single player in the media history store.
   void SavePlayback(const content::MediaPlayerWatchTime& watch_time);
 
@@ -71,9 +78,6 @@ class MediaHistoryStore {
   // be ordered by most recent first and be limited to the first |num_sessions|.
   // For each session it calls |filter| and if that returns |true| then that
   // session will be included in the results.
-  using GetPlaybackSessionsFilter =
-      base::RepeatingCallback<bool(const base::TimeDelta& duration,
-                                   const base::TimeDelta& position)>;
   void GetPlaybackSessions(
       base::Optional<unsigned int> num_sessions,
       base::Optional<GetPlaybackSessionsFilter> filter,
@@ -92,10 +96,6 @@ class MediaHistoryStore {
 
   scoped_refptr<base::UpdateableSequencedTaskRunner> GetDBTaskRunnerForTest();
 
- protected:
-  friend class MediaHistoryKeyedService;
-  friend class MediaHistoryKeyedServiceTest;
-
   void EraseDatabaseAndCreateNew();
   void DeleteAllOriginData(const std::set<url::Origin>& origins);
 

chrome/browser/ui/webui/media/media_history_ui.cc

@@ -49,33 +49,33 @@ void MediaHistoryUI::BindInterface(
 
 void MediaHistoryUI::GetMediaHistoryStats(
     GetMediaHistoryStatsCallback callback) {
-  return GetMediaHistoryStore()->GetMediaHistoryStats(std::move(callback));
+  return GetMediaHistoryService()->GetMediaHistoryStats(std::move(callback));
 }
 
 void MediaHistoryUI::GetMediaHistoryOriginRows(
     GetMediaHistoryOriginRowsCallback callback) {
-  return GetMediaHistoryStore()->GetOriginRowsForDebug(std::move(callback));
+  return GetMediaHistoryService()->GetOriginRowsForDebug(std::move(callback));
 }
 
 void MediaHistoryUI::GetMediaHistoryPlaybackRows(
     GetMediaHistoryPlaybackRowsCallback callback) {
-  return GetMediaHistoryStore()->GetMediaHistoryPlaybackRowsForDebug(
+  return GetMediaHistoryService()->GetMediaHistoryPlaybackRowsForDebug(
       std::move(callback));
 }
 
 void MediaHistoryUI::GetMediaHistoryPlaybackSessionRows(
     GetMediaHistoryPlaybackSessionRowsCallback callback) {
-  return GetMediaHistoryStore()->GetPlaybackSessions(
+  return GetMediaHistoryService()->GetPlaybackSessions(
       base::nullopt, base::nullopt, std::move(callback));
 }
 
-media_history::MediaHistoryStore* MediaHistoryUI::GetMediaHistoryStore() {
+media_history::MediaHistoryKeyedService*
+MediaHistoryUI::GetMediaHistoryService() {
   Profile* profile = Profile::FromWebUI(web_ui());
   DCHECK(profile);
 
   media_history::MediaHistoryKeyedService* service =
       media_history::MediaHistoryKeyedServiceFactory::GetForProfile(profile);
   DCHECK(service);
-
-  return service->GetMediaHistoryStore();
+  return service;
 }

chrome/browser/ui/webui/media/media_history_ui.h

@@ -11,7 +11,7 @@
 #include "ui/webui/mojo_web_ui_controller.h"
 
 namespace media_history {
-class MediaHistoryStore;
+class MediaHistoryKeyedService;
 }  // namespace media_history
 
 // The UI for chrome://media-history.
@@ -38,7 +38,7 @@ class MediaHistoryUI : public ui::MojoWebUIController,
       GetMediaHistoryPlaybackSessionRowsCallback callback) override;
 
  private:
-  media_history::MediaHistoryStore* GetMediaHistoryStore();
+  media_history::MediaHistoryKeyedService* GetMediaHistoryService();
 
   mojo::ReceiverSet<media_history::mojom::MediaHistoryStore> receivers_;