Media Session: do not notify image observers for no-op image clearing.

It avoids triggering callbacks that end up being no-ops and limit it to
situations where the observer would actually do something about it.

Bug: 1028822
Change-Id: I7982f74d5791277e181e63e91c0f9f076586717a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1949561Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721711}

services/media_session/media_controller.cc

@@ -63,11 +63,15 @@ class MediaController::ImageObserverHolder {
   }
 
   void ClearImage() {
+    if (!did_send_image_last_)
+      return;
+    did_send_image_last_ = false;
     observer_->MediaControllerImageChanged(type_, SkBitmap());
   }
 
  private:
   void OnImage(const SkBitmap& image) {
+    did_send_image_last_ = true;
     observer_->MediaControllerImageChanged(type_, image);
   }
 
@@ -83,6 +87,9 @@ class MediaController::ImageObserverHolder {
 
   mojo::Remote<mojom::MediaControllerImageObserver> observer_;
 
+  // Whether the last information sent to the observer was an image.
+  bool did_send_image_last_ = false;
+
   base::WeakPtrFactory<ImageObserverHolder> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ImageObserverHolder);

services/media_session/media_controller_unittest.cc

@@ -1091,9 +1091,7 @@ TEST_F(MediaControllerTest, ActiveController_SimulateImagesChanged) {
   {
     test::TestMediaControllerImageObserver observer(controller(), 0, 0);
 
-    // By default, we should receive an empty image.
-    observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
-                                        true);
+    // By default, the image is empty but no notification should be received.
     EXPECT_TRUE(media_session.last_image_src().is_empty());
 
     // Check that we receive the correct image and that it was requested from
@@ -1201,31 +1199,37 @@ TEST_F(MediaControllerTest,
   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);
+
+  media_session.SetImagesOfType(mojom::MediaSessionImageType::kArtwork,
+                                {image1});
 
   {
     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);
+    // we have is 1px so the observer will not be notified.
     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);
+                                  {image1, image2});
     observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
                                         false);
     EXPECT_EQ(image2.src, media_session.last_image_src());
+
+    // Use the first set of images again.
+    media_session.SetImagesOfType(mojom::MediaSessionImageType::kArtwork,
+                                  {image1});
+    // The observer requires as image that is at least 5px and should now be
+    // notified that the image was cleared.
+    observer.WaitForExpectedImageOfType(mojom::MediaSessionImageType::kArtwork,
+                                        true);
   }
 }