Split KaleidoscopeDataProvider into NTP variant

As part of our security review we were asked to split
the KaleidoscopeDataProvider interface into two to reduce
the attack surface from the NTP.

BUG=1114862

Change-Id: Ie37472dfcda44cbea00b3e5b9b610b485d41d5fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577899
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834843}

chrome/browser/chrome_browser_interface_binders.cc

@@ -811,7 +811,11 @@ void PopulateChromeWebUIFrameBinders(
 
 #if !defined(OS_ANDROID)
   RegisterWebUIControllerInterfaceBinder<media::mojom::KaleidoscopeDataProvider,
-                                         KaleidoscopeUI, NewTabPageUI>(map);
+                                         KaleidoscopeUI>(map);
+
+  RegisterWebUIControllerInterfaceBinder<
+      media::mojom::KaleidoscopeNTPDataProvider, NewTabPageUI>(map);
+
   RegisterWebUIControllerInterfaceBinder<
       media::mojom::KaleidoscopeIdentityManager, KaleidoscopeUI, NewTabPageUI>(
       map);

chrome/browser/media/kaleidoscope/kaleidoscope_data_provider_impl.cc

@@ -63,11 +63,27 @@ base::Optional<media_feeds::mojom::MediaFeedItemType> GetFeedItemTypeForTab(
 
 KaleidoscopeDataProviderImpl::KaleidoscopeDataProviderImpl(
     mojo::PendingReceiver<media::mojom::KaleidoscopeDataProvider> receiver,
+    Profile* profile,
+    KaleidoscopeMetricsRecorder* metrics_recorder)
+    : KaleidoscopeDataProviderImpl(profile, metrics_recorder) {
+  receiver_.Bind(std::move(receiver));
+}
+
+KaleidoscopeDataProviderImpl::KaleidoscopeDataProviderImpl(
+    mojo::PendingReceiver<media::mojom::KaleidoscopeNTPDataProvider> receiver,
+    Profile* profile,
+    KaleidoscopeMetricsRecorder* metrics_recorder)
+    : KaleidoscopeDataProviderImpl(profile, metrics_recorder) {
+  ntp_receiver_.Bind(std::move(receiver));
+}
+
+KaleidoscopeDataProviderImpl::KaleidoscopeDataProviderImpl(
     Profile* profile,
     KaleidoscopeMetricsRecorder* metrics_recorder)
     : profile_(profile),
       metrics_recorder_(metrics_recorder),
-      receiver_(this, std::move(receiver)) {
+      receiver_(this),
+      ntp_receiver_(this) {
   DCHECK(profile);
 
   identity_manager_ = IdentityManagerFactory::GetForProfile(profile_);
@@ -164,7 +180,7 @@ void KaleidoscopeDataProviderImpl::GetHighWatchTimeOrigins(
 
 void KaleidoscopeDataProviderImpl::GetTopMediaFeeds(
     media::mojom::KaleidoscopeTab tab,
-    GetTopMediaFeedsCallback callback) {
+    media::mojom::KaleidoscopeDataProvider::GetTopMediaFeedsCallback callback) {
   GetMediaHistoryService()->GetMediaFeeds(
       media_history::MediaHistoryKeyedService::GetMediaFeedsRequest::
           CreateTopFeedsForDisplay(
@@ -178,7 +194,8 @@ void KaleidoscopeDataProviderImpl::GetTopMediaFeeds(
 void KaleidoscopeDataProviderImpl::GetMediaFeedContents(
     int64_t feed_id,
     media::mojom::KaleidoscopeTab tab,
-    GetMediaFeedContentsCallback callback) {
+    media::mojom::KaleidoscopeDataProvider::GetMediaFeedContentsCallback
+        callback) {
   GetMediaHistoryService()->GetMediaFeedItems(
       media_history::MediaHistoryKeyedService::GetMediaFeedItemsRequest::
           CreateItemsForFeed(
@@ -193,7 +210,8 @@ void KaleidoscopeDataProviderImpl::GetMediaFeedContents(
 
 void KaleidoscopeDataProviderImpl::GetContinueWatchingMediaFeedItems(
     media::mojom::KaleidoscopeTab tab,
-    GetContinueWatchingMediaFeedItemsCallback callback) {
+    media::mojom::KaleidoscopeDataProvider::
+        GetContinueWatchingMediaFeedItemsCallback callback) {
   GetMediaHistoryService()->GetMediaFeedItems(
       media_history::MediaHistoryKeyedService::GetMediaFeedItemsRequest::
           CreateItemsForContinueWatching(
@@ -227,7 +245,7 @@ void KaleidoscopeDataProviderImpl::GetCollections(
 }
 
 void KaleidoscopeDataProviderImpl::GetSignedOutProviders(
-    GetSignedOutProvidersCallback cb) {
+    media::mojom::KaleidoscopeDataProvider::GetSignedOutProvidersCallback cb) {
   DCHECK(!identity_manager_->HasPrimaryAccount(
       signin::ConsentLevel::kNotRequired));
 
@@ -313,7 +331,8 @@ void KaleidoscopeDataProviderImpl::OnGotMediaFeedContents(
 }
 
 void KaleidoscopeDataProviderImpl::OnGotContinueWatchingMediaFeedItems(
-    GetContinueWatchingMediaFeedItemsCallback callback,
+    media::mojom::KaleidoscopeDataProvider::
+        GetContinueWatchingMediaFeedItemsCallback callback,
     std::vector<media_feeds::mojom::MediaFeedItemPtr> items) {
   std::set<int64_t> ids;
   for (auto& item : items)

chrome/browser/media/kaleidoscope/kaleidoscope_data_provider_impl.h

@@ -26,12 +26,17 @@ class KaleidoscopeMetricsRecorder;
 class Profile;
 
 class KaleidoscopeDataProviderImpl
-    : public media::mojom::KaleidoscopeDataProvider {
+    : public media::mojom::KaleidoscopeDataProvider,
+      public media::mojom::KaleidoscopeNTPDataProvider {
  public:
   KaleidoscopeDataProviderImpl(
       mojo::PendingReceiver<media::mojom::KaleidoscopeDataProvider> receiver,
       Profile* profile,
       KaleidoscopeMetricsRecorder* metrics_recorder);
+  KaleidoscopeDataProviderImpl(
+      mojo::PendingReceiver<media::mojom::KaleidoscopeNTPDataProvider> receiver,
+      Profile* profile,
+      KaleidoscopeMetricsRecorder* metrics_recorder);
 
   KaleidoscopeDataProviderImpl(const KaleidoscopeDataProviderImpl&) = delete;
   KaleidoscopeDataProviderImpl& operator=(const KaleidoscopeDataProviderImpl&) =
@@ -39,14 +44,19 @@ class KaleidoscopeDataProviderImpl
   ~KaleidoscopeDataProviderImpl() override;
 
   // media::mojom::KaleidoscopeDataProvider implementation.
-  void GetTopMediaFeeds(media::mojom::KaleidoscopeTab tab,
-                        GetTopMediaFeedsCallback callback) override;
-  void GetMediaFeedContents(int64_t feed_id,
-                            media::mojom::KaleidoscopeTab tab,
-                            GetMediaFeedContentsCallback callback) override;
+  void GetTopMediaFeeds(
+      media::mojom::KaleidoscopeTab tab,
+      media::mojom::KaleidoscopeDataProvider::GetTopMediaFeedsCallback callback)
+      override;
+  void GetMediaFeedContents(
+      int64_t feed_id,
+      media::mojom::KaleidoscopeTab tab,
+      media::mojom::KaleidoscopeDataProvider::GetMediaFeedContentsCallback
+          callback) override;
   void GetContinueWatchingMediaFeedItems(
       media::mojom::KaleidoscopeTab tab,
-      GetContinueWatchingMediaFeedItemsCallback callback) override;
+      media::mojom::KaleidoscopeDataProvider::
+          GetContinueWatchingMediaFeedItemsCallback callback) override;
   void GetShouldShowFirstRunExperience(
       GetShouldShowFirstRunExperienceCallback cb) override;
   void SetFirstRunExperienceStep(
@@ -64,7 +74,9 @@ class KaleidoscopeDataProviderImpl
   void GetCollections(media::mojom::CredentialsPtr credentials,
                       const std::string& request,
                       GetCollectionsCallback cb) override;
-  void GetSignedOutProviders(GetSignedOutProvidersCallback cb) override;
+  void GetSignedOutProviders(
+      media::mojom::KaleidoscopeDataProvider::GetSignedOutProvidersCallback cb)
+      override;
   void SetSignedOutProviders(
       const std::vector<std::string>& providers) override;
   void RecordTimeTakenToStartWatchHistogram(base::TimeDelta time) override;
@@ -76,12 +88,16 @@ class KaleidoscopeDataProviderImpl
  private:
   media_history::MediaHistoryKeyedService* GetMediaHistoryService();
 
+  KaleidoscopeDataProviderImpl(Profile* profile,
+                               KaleidoscopeMetricsRecorder* metrics_recorder);
+
   void OnGotMediaFeedContents(
       GetMediaFeedContentsCallback callback,
       const int64_t feed_id,
       std::vector<media_feeds::mojom::MediaFeedItemPtr> items);
   void OnGotContinueWatchingMediaFeedItems(
-      GetContinueWatchingMediaFeedItemsCallback callback,
+      media::mojom::KaleidoscopeDataProvider::
+          GetContinueWatchingMediaFeedItemsCallback callback,
       std::vector<media_feeds::mojom::MediaFeedItemPtr> items);
 
   signin::IdentityManager* identity_manager_;
@@ -92,6 +108,8 @@ class KaleidoscopeDataProviderImpl
 
   mojo::Receiver<media::mojom::KaleidoscopeDataProvider> receiver_;
 
+  mojo::Receiver<media::mojom::KaleidoscopeNTPDataProvider> ntp_receiver_;
+
   base::WeakPtrFactory<KaleidoscopeDataProviderImpl> weak_ptr_factory{this};
 };
 

chrome/browser/media/kaleidoscope/mojom/kaleidoscope.mojom

@@ -90,12 +90,6 @@ interface KaleidoscopeDataProvider {
   // Will trigger a feedback dialog to be displayed.
   SendFeedback();
 
-  // Gets the collections from the backend to be displayed. Takes a credential
-  // instance and a string that contains the request to be sent to the server.
-  // The request is the GetCollectionsRequest proto here: go/ks-media-proto.
-  GetCollections(Credentials credentials, string request)
-      => (GetCollectionsResponse response);
-
   // Gets a list of stored providers that will be used to filter the
   // recommendations on the server if the user is signed out. They are arbitary
   // strings that are provided by the server, stored locally and then retrieved
@@ -124,6 +118,29 @@ interface KaleidoscopeDataProvider {
   UpdateFeedUserStatus(int64 feed_id, media_feeds.mojom.FeedUserStatus status);
 };
 
+// Provides data for the Kaleidoscope NTP module.
+interface KaleidoscopeNTPDataProvider {
+  // Returns all the Media Feeds that Kaleidoscope might decide to show.
+  // Returned feeds will be appropriate for the |tab|.
+  GetTopMediaFeeds(KaleidoscopeTab tab) => (array<media_feeds.mojom.MediaFeed> feeds);
+
+  // Returns all the items from the Media Feeds Store that can be displayed for
+  // continue watching. Returned items will be appropriate for the |tab|.
+  GetContinueWatchingMediaFeedItems(KaleidoscopeTab tab) => (array<media_feeds.mojom.MediaFeedItem> items);
+
+  // Gets the collections from the backend to be displayed. Takes a credential
+  // instance and a string that contains the request to be sent to the server.
+  // The request is the GetCollectionsRequest proto here: go/ks-media-proto.
+  GetCollections(Credentials credentials, string request)
+      => (GetCollectionsResponse response);
+
+  // Gets a list of stored providers that will be used to filter the
+  // recommendations on the server if the user is signed out. They are arbitary
+  // strings that are provided by the server, stored locally and then retrieved
+  // when Kaleidoscope is loaded.
+  GetSignedOutProviders() => (array<string> providers);
+};
+
 // Handles identity related tasks.
 interface KaleidoscopeIdentityManager {
   // Retrieves the current credentials.

chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.cc

@@ -387,7 +387,7 @@ void NewTabPageUI::BindInterface(
 }
 
 void NewTabPageUI::BindInterface(
-    mojo::PendingReceiver<media::mojom::KaleidoscopeDataProvider>
+    mojo::PendingReceiver<media::mojom::KaleidoscopeNTPDataProvider>
         pending_page_handler) {
   kaleidoscope_data_provider_ = std::make_unique<KaleidoscopeDataProviderImpl>(
       std::move(pending_page_handler), profile_, nullptr);

chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.h

@@ -76,7 +76,7 @@ class NewTabPageUI
   // media::mojom::KaleidoscopeNTPDataProvider mojo interface passing the
   // pending receiver that will be internally bound.
   void BindInterface(
-      mojo::PendingReceiver<media::mojom::KaleidoscopeDataProvider>
+      mojo::PendingReceiver<media::mojom::KaleidoscopeNTPDataProvider>
           pending_receiver);
 
   // Instantiates the implementor of the