[Media Image] Use ranking to select image

Use media_session::MediaImageManager to rank the
images based on the specified minimum and desired
sizes.

BUG=914430

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

services/media_session/media_controller.cc

@@ -6,6 +6,8 @@
 
 #include <set>
 
+#include "services/media_session/public/cpp/media_image_manager.h"
+
 namespace media_session {
 
 // ImageObserverHolder will hold each mojo image observer with the image
@@ -18,7 +20,8 @@ class MediaController::ImageObserverHolder {
                       int desired_size_px,
                       mojom::MediaControllerImageObserverPtr observer,
                       const std::vector<MediaImage>& current_images)
-      : owner_(owner),
+      : manager_(minimum_size_px, desired_size_px),
+        owner_(owner),
         type_(type),
         minimum_size_px_(minimum_size_px),
         desired_size_px_(desired_size_px),
@@ -39,18 +42,18 @@ class MediaController::ImageObserverHolder {
   mojom::MediaSessionImageType type() const { return type_; }
 
   void ImagesChanged(const std::vector<MediaImage>& images) {
-    // If we do not have any images then we should call with an empty image to
+    base::Optional<MediaImage> image = manager_.SelectImage(images);
+
+    // If we could not find an image then we should call with an empty image to
     // flush the observer.
-    if (images.empty()) {
+    if (!image) {
       ClearImage();
       return;
     }
 
-    // TODO(beccahughes): Pick the best image based on ranking.
-
     DCHECK(owner_->session_);
     owner_->session_->GetMediaImageBitmap(
-        images.at(0), minimum_size_px_, desired_size_px_,
+        *image, minimum_size_px_, desired_size_px_,
         base::BindOnce(&MediaController::ImageObserverHolder::OnImage,
                        base::Unretained(this)));
   }
@@ -64,6 +67,8 @@ class MediaController::ImageObserverHolder {
     observer_->MediaControllerImageChanged(type_, image);
   }
 
+  media_session::MediaImageManager manager_;
+
   MediaController* const owner_;
 
   mojom::MediaSessionImageType const type_;

services/media_session/media_controller_unittest.cc

@@ -881,7 +881,7 @@ TEST_F(MediaControllerTest, ClearImageObserverOnError) {
   EXPECT_EQ(0u, GetImageObserverCount(controller));
 
   {
-    test::TestMediaControllerImageObserver observer(controller_ptr);
+    test::TestMediaControllerImageObserver observer(controller_ptr, 0, 0);
     EXPECT_EQ(1u, GetImageObserverCount(controller));
   }
 
@@ -908,7 +908,7 @@ TEST_F(MediaControllerTest, ActiveController_SimulateImagesChanged) {
   images.push_back(image);
 
   {
-    test::TestMediaControllerImageObserver observer(controller());
+    test::TestMediaControllerImageObserver observer(controller(), 0, 0);
 
     // By default, we should receive an empty image.
     observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
@@ -952,7 +952,7 @@ TEST_F(MediaControllerTest,
   media_session.SetImagesOfType(mojom::MediaSessionImageType::kArtwork, images);
 
   {
-    test::TestMediaControllerImageObserver observer(controller());
+    test::TestMediaControllerImageObserver observer(controller(), 0, 0);
     observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
                                         false);
     EXPECT_EQ(image.src, media_session.last_image_src());
@@ -991,7 +991,7 @@ TEST_F(MediaControllerTest,
   media_session.SetImagesOfType(mojom::MediaSessionImageType::kArtwork, images);
 
   {
-    test::TestMediaControllerImageObserver observer(controller());
+    test::TestMediaControllerImageObserver observer(controller(), 0, 0);
     observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
                                         false);
     EXPECT_EQ(image.src, media_session.last_image_src());
@@ -1005,4 +1005,85 @@ TEST_F(MediaControllerTest,
   }
 }
 
+TEST_F(MediaControllerTest,
+       ActiveController_SimulateImagesChanged_MinSizeCutoff) {
+  test::MockMediaSession media_session;
+  media_session.SetIsControllable(true);
+
+  {
+    test::MockMediaSessionMojoObserver observer(media_session);
+    RequestAudioFocus(media_session, mojom::AudioFocusType::kGain);
+    observer.WaitForState(mojom::MediaSessionInfo::SessionState::kActive);
+  }
+
+  std::vector<MediaImage> images;
+  MediaImage image1;
+  image1.src = GURL("https://www.google.com");
+  image1.sizes.push_back(gfx::Size(1, 1));
+  images.push_back(image1);
+  media_session.SetImagesOfType(mojom::MediaSessionImageType::kArtwork, images);
+
+  {
+    test::TestMediaControllerImageObserver observer(controller(), 5, 10);
+
+    // The observer requires an image that is at least 5px but the only image
+    // we have is 1px so the observer will receive a null image.
+    observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
+                                        true);
+    EXPECT_TRUE(media_session.last_image_src().is_empty());
+
+    MediaImage image2;
+    image2.src = GURL("https://www.example.com");
+    image2.sizes.push_back(gfx::Size(10, 10));
+    images.push_back(image2);
+
+    // Update the media session with two images, one that is too small and one
+    // that is the right size. We should receive the second image through the
+    // observer.
+    media_session.SetImagesOfType(mojom::MediaSessionImageType::kArtwork,
+                                  images);
+    observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
+                                        false);
+    EXPECT_EQ(image2.src, media_session.last_image_src());
+  }
+}
+
+TEST_F(MediaControllerTest,
+       ActiveController_SimulateImagesChanged_DesiredSize) {
+  test::MockMediaSession media_session;
+  media_session.SetIsControllable(true);
+
+  {
+    test::MockMediaSessionMojoObserver observer(media_session);
+    RequestAudioFocus(media_session, mojom::AudioFocusType::kGain);
+    observer.WaitForState(mojom::MediaSessionInfo::SessionState::kActive);
+  }
+
+  std::vector<MediaImage> images;
+  MediaImage image1;
+  image1.src = GURL("https://www.google.com");
+  image1.sizes.push_back(gfx::Size(10, 10));
+  images.push_back(image1);
+
+  MediaImage image2;
+  image2.src = GURL("https://www.example.com");
+  image2.sizes.push_back(gfx::Size(9, 9));
+  images.push_back(image2);
+
+  media_session.SetImagesOfType(mojom::MediaSessionImageType::kArtwork, images);
+
+  {
+    test::TestMediaControllerImageObserver observer(controller(), 5, 10);
+
+    // The media session has two images, but the first one is closer to the 10px
+    // desired size that the observer has specified. Therefore, the observer
+    // should receive that image.
+    media_session.SetImagesOfType(mojom::MediaSessionImageType::kArtwork,
+                                  images);
+    observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
+                                        false);
+    EXPECT_EQ(image1.src, media_session.last_image_src());
+  }
+}
+
 }  // namespace media_session

services/media_session/public/cpp/test/test_media_controller.cc

@@ -8,12 +8,14 @@ namespace media_session {
 namespace test {
 
 TestMediaControllerImageObserver::TestMediaControllerImageObserver(
-    mojom::MediaControllerPtr& controller) {
+    mojom::MediaControllerPtr& controller,
+    int minimum_size_px,
+    int desired_size_px) {
   mojom::MediaControllerImageObserverPtr ptr;
   binding_.Bind(mojo::MakeRequest(&ptr));
 
-  controller->ObserveImages(mojom::MediaSessionImageType::kArtwork, 0, 0,
-                            std::move(ptr));
+  controller->ObserveImages(mojom::MediaSessionImageType::kArtwork,
+                            minimum_size_px, desired_size_px, std::move(ptr));
   controller.FlushForTesting();
 }
 

services/media_session/public/cpp/test/test_media_controller.h

@@ -22,8 +22,9 @@ class COMPONENT_EXPORT(MEDIA_SESSION_TEST_SUPPORT_CPP)
     TestMediaControllerImageObserver
     : public mojom::MediaControllerImageObserver {
  public:
-  explicit TestMediaControllerImageObserver(
-      mojom::MediaControllerPtr& controller);
+  TestMediaControllerImageObserver(mojom::MediaControllerPtr& controller,
+                                   int minimum_size_px,
+                                   int desired_size_px);
   ~TestMediaControllerImageObserver() override;
 
   // mojom::MediaControllerImageObserver overrides.