Wrap MediaMetadata in base::Optional in content and chrome

Since base::Optional<> has already been in use. It's helpful to wrap
MediaMetadata in base::Optional<>, so that the browser can know whether
the page has set the metadata or not.

BUG=643195

Review-Url: https://codereview.chromium.org/2300083002
Cr-Commit-Position: refs/heads/master@{#417255}

chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java

@@ -104,11 +104,11 @@ public class MediaSessionTabHelper {
 
                 // The page's title is used as a placeholder if no title is specified in the
                 // metadata.
-                if (TextUtils.isEmpty(metadata.getTitle())) {
+                if (metadata == null || TextUtils.isEmpty(metadata.getTitle())) {
                     mFallbackMetadata = new MediaMetadata(
                             sanitizeMediaTitle(mTab.getTitle()),
-                            metadata.getArtist(),
-                            metadata.getAlbum());
+                            metadata == null ? "" : metadata.getArtist(),
+                            metadata == null ? "" : metadata.getAlbum());
                     metadata = mFallbackMetadata;
                 }
 

content/browser/android/web_contents_observer_proxy.cc

@@ -9,12 +9,14 @@
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
+#include "base/optional.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/android/media_metadata_android.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
+#include "content/public/common/media_metadata.h"
 #include "jni/WebContentsObserverProxy_jni.h"
 
 using base::android::AttachCurrentThread;
@@ -307,12 +309,14 @@ void WebContentsObserverProxy::DidStartNavigationToPendingEntry(
 void WebContentsObserverProxy::MediaSessionStateChanged(
     bool is_controllable,
     bool is_suspended,
-    const MediaMetadata& metadata) {
+    const base::Optional<MediaMetadata>& metadata) {
   JNIEnv* env = AttachCurrentThread();
 
   ScopedJavaLocalRef<jobject> obj(java_observer_);
-  ScopedJavaLocalRef<jobject> j_metadata =
-      MediaMetadataAndroid::CreateJavaObject(env, metadata);
+  ScopedJavaLocalRef<jobject> j_metadata;
+
+  if (metadata.has_value())
+    j_metadata = MediaMetadataAndroid::CreateJavaObject(env, metadata.value());
 
   Java_WebContentsObserverProxy_mediaSessionStateChanged(
       env, obj, is_controllable, is_suspended, j_metadata);

content/browser/android/web_contents_observer_proxy.h

@@ -72,9 +72,10 @@ class WebContentsObserverProxy : public WebContentsObserver {
   void DidStartNavigationToPendingEntry(
       const GURL& url,
       NavigationController::ReloadType reload_type) override;
-  void MediaSessionStateChanged(bool is_controllable,
-                                bool is_suspended,
-                                const MediaMetadata& metadata) override;
+  void MediaSessionStateChanged(
+      bool is_controllable,
+      bool is_suspended,
+      const base::Optional<MediaMetadata>& metadata) override;
   void SetToBaseURLForDataURLIfNeeded(std::string* url);
 
   void DidFailLoadInternal(bool is_provisional_load,

content/browser/media/android/browser_media_session_manager.cc

@@ -4,6 +4,7 @@
 
 #include "content/browser/media/android/browser_media_session_manager.h"
 
+#include "base/optional.h"
 #include "content/browser/media/session/media_session.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/media/media_metadata_sanitizer.h"
@@ -30,10 +31,11 @@ void BrowserMediaSessionManager::OnDeactivate(int session_id, int request_id) {
 
 void BrowserMediaSessionManager::OnSetMetadata(
     int session_id,
-    const MediaMetadata& insecure_metadata) {
+    const base::Optional<MediaMetadata>& insecure_metadata) {
   // When receiving a MediaMetadata, the browser process can't trust that it is
   // coming from a known and secure source. It must be processed accordingly.
-  if (!MediaMetadataSanitizer::CheckSanity(insecure_metadata)) {
+  if (insecure_metadata.has_value() &&
+      !MediaMetadataSanitizer::CheckSanity(insecure_metadata.value())) {
     render_frame_host_->GetProcess()->ShutdownForBadMessage();
     return;
   }

content/browser/media/android/browser_media_session_manager.h

@@ -6,11 +6,12 @@
 #define CONTENT_BROWSER_MEDIA_ANDROID_BROWSER_MEDIA_SESSION_MANAGER_H_
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "content/common/content_export.h"
 
 namespace IPC {
 class Message;
-}
+}  // namespace IPC
 
 namespace content {
 
@@ -24,7 +25,8 @@ class CONTENT_EXPORT BrowserMediaSessionManager {
   // Message handlers.
   virtual void OnActivate(int session_id, int request_id);
   virtual void OnDeactivate(int session_id, int request_id);
-  virtual void OnSetMetadata(int session_id, const MediaMetadata& metadata);
+  virtual void OnSetMetadata(
+      int session_id, const base::Optional<MediaMetadata>& metadata);
 
   int GetRoutingID() const;
 

content/browser/media/android/browser_media_session_manager_browsertest.cc

@@ -14,6 +14,7 @@
 #include "content/browser/media/android/media_web_contents_observer_android.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/media_metadata.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/test_utils.h"
@@ -30,19 +31,28 @@ namespace content {
 namespace {
 
 // Helper function for build test javascripts.
-std::string BuildSetMetadataScript(const MediaMetadata& metadata) {
+std::string BuildSetMetadataScript(
+    const base::Optional<MediaMetadata>& metadata) {
   std::ostringstream generated_script;
 
-  generated_script << "var audio = document.createElement(\'audio\');"
-     << "audio.session = new MediaSession();"
-     << "audio.session.metadata = new MediaMetadata({"
-     << "title: \"" << metadata.title << "\", "
-     << "artist: \"" << metadata.artist << "\", "
-     << "album: \"" << metadata.album << "\", "
-     << "artwork: [";
+  generated_script
+      << "var audio = document.createElement(\'audio\');"
+      << "audio.session = new MediaSession();";
+
+  if (!metadata.has_value()) {
+    generated_script << "audio.session.metadata = null;";
+    return generated_script.str();
+  }
+
+  generated_script
+      << "audio.session.metadata = new MediaMetadata({"
+      << "title: \"" << metadata->title << "\", "
+      << "artist: \"" << metadata->artist << "\", "
+      << "album: \"" << metadata->album << "\", "
+      << "artwork: [";
 
   std::string artwork_separator = "";
-  for (const auto& artwork : metadata.artwork) {
+  for (const auto& artwork : metadata->artwork) {
     generated_script << artwork_separator << "{"
        << "src: \"" << artwork.src.spec() << "\", "
        << "type: \"" << artwork.type.string() << "\", "
@@ -61,12 +71,17 @@ std::string BuildSetMetadataScript(const MediaMetadata& metadata) {
 }  // anonymous namespace
 
 // Helper function to be pretty-print error messages by GMock.
-void PrintTo(const MediaMetadata& metadata, std::ostream* os) {
-  *os << "{ title=" << metadata.title << ", ";
-  *os << "artist=" << metadata.artist << ", ";
-  *os << "album=" << metadata.album << ", ";
+void PrintTo(const base::Optional<MediaMetadata>& metadata, std::ostream* os) {
+  if (!metadata.has_value()) {
+    *os << "<null MediaMetadata>";
+    return;
+  }
+
+  *os << "{ title=" << metadata->title << ", ";
+  *os << "artist=" << metadata->artist << ", ";
+  *os << "album=" << metadata->album << ", ";
   *os << "artwork=[";
-  for (const auto& artwork : metadata.artwork) {
+  for (const auto& artwork : metadata->artwork) {
     *os << "{ src=" << artwork.src.spec() << ", ";
     *os << "type=" << artwork.type.string() << ", ";
     *os << "sizes=[";
@@ -85,8 +100,8 @@ class MockBrowserMediaSessionManager : public BrowserMediaSessionManager {
 
   MOCK_METHOD2(OnActiveate, void(int session_id, int request_id));
   MOCK_METHOD2(OnDeactiveate, void(int session_id, int request_id));
-  MOCK_METHOD2(OnSetMetadata, void(int session_id,
-                                   const MediaMetadata& metadata));
+  MOCK_METHOD2(OnSetMetadata, void(
+      int session_id, const base::Optional<MediaMetadata>& metadata));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockBrowserMediaSessionManager);
@@ -129,15 +144,15 @@ class BrowserMediaSessionManagerBrowserTest : public ContentBrowserTest {
 
 IN_PROC_BROWSER_TEST_F(BrowserMediaSessionManagerBrowserTest,
                        TestMetadataPropagated) {
-  MediaMetadata expected;
-  expected.title = base::ASCIIToUTF16("title1");
-  expected.artist = base::ASCIIToUTF16("artist1");
-  expected.album = base::ASCIIToUTF16("album1");
+  base::Optional<MediaMetadata> expected = MediaMetadata();
+  expected->title = base::ASCIIToUTF16("title1");
+  expected->artist = base::ASCIIToUTF16("artist1");
+  expected->album = base::ASCIIToUTF16("album1");
   MediaMetadata::Artwork artwork;
   artwork.src = GURL("http://foo.com/bar.png");
   artwork.type = base::NullableString16(base::ASCIIToUTF16("image/png"), false);
   artwork.sizes.push_back(gfx::Size(128, 128));
-  expected.artwork.push_back(artwork);
+  expected->artwork.push_back(artwork);
 
   message_loop_runner_ = new MessageLoopRunner();
   EXPECT_CALL(*browser_media_session_manager_, OnSetMetadata(_, expected))
@@ -152,18 +167,46 @@ IN_PROC_BROWSER_TEST_F(BrowserMediaSessionManagerBrowserTest,
   // Make expectations ordered.
   InSequence s;
 
-  MediaMetadata dont_care_metadata;
+  base::Optional<MediaMetadata> dont_care_metadata = MediaMetadata();
 
-  MediaMetadata expected;
-  expected.title = base::ASCIIToUTF16("title2");
-  expected.artist = base::ASCIIToUTF16("artist2");
-  expected.album = base::ASCIIToUTF16("album2");
+  base::Optional<MediaMetadata> expected = MediaMetadata();
+  expected->title = base::ASCIIToUTF16("title2");
+  expected->artist = base::ASCIIToUTF16("artist2");
+  expected->album = base::ASCIIToUTF16("album2");
   MediaMetadata::Artwork artwork;
   artwork.src = GURL("http://foo.com/bar.jpg");
   artwork.type = base::NullableString16(
       base::ASCIIToUTF16("image/jpeg"), false);
   artwork.sizes.push_back(gfx::Size(256, 256));
-  expected.artwork.push_back(artwork);
+  expected->artwork.push_back(artwork);
+
+  // Set metadata for the first time.
+  message_loop_runner_ = new MessageLoopRunner();
+  EXPECT_CALL(*browser_media_session_manager_,
+              OnSetMetadata(_, dont_care_metadata))
+      .Times(1);
+  ASSERT_TRUE(ExecuteScript(web_contents_->GetMainFrame(),
+                            BuildSetMetadataScript(dont_care_metadata)));
+  message_loop_runner_->Run();
+
+  // Set metadata for the second time.
+  message_loop_runner_ = new MessageLoopRunner();
+  EXPECT_CALL(*browser_media_session_manager_, OnSetMetadata(_, expected))
+      .Times(1);
+  ASSERT_TRUE(ExecuteScript(web_contents_->GetMainFrame(),
+                            BuildSetMetadataScript(expected)));
+  message_loop_runner_->Run();
+}
+
+
+IN_PROC_BROWSER_TEST_F(BrowserMediaSessionManagerBrowserTest,
+                       TestNullMetadata) {
+  // Make expectations ordered.
+  InSequence s;
+
+  base::Optional<MediaMetadata> dont_care_metadata = MediaMetadata();
+
+  base::Optional<MediaMetadata> expected;
 
   // Set metadata for the first time.
   message_loop_runner_ = new MessageLoopRunner();
@@ -188,14 +231,14 @@ IN_PROC_BROWSER_TEST_F(BrowserMediaSessionManagerBrowserTest,
   // Make expectations ordered.
   InSequence s;
 
-  MediaMetadata dirty_metadata;
+  base::Optional<MediaMetadata> dirty_metadata = MediaMetadata();
   MediaMetadata::Artwork file_artwork;
   file_artwork.src = GURL("file:///foo/bar.jpg");
   file_artwork.type = base::NullableString16(
       base::ASCIIToUTF16("image/jpeg"), false);
-  dirty_metadata.artwork.push_back(file_artwork);
+  dirty_metadata->artwork.push_back(file_artwork);
 
-  MediaMetadata expected;
+  base::Optional<MediaMetadata> expected = MediaMetadata();
 
   // Set metadata for the first time.
   message_loop_runner_ = new MessageLoopRunner();

content/browser/media/session/media_session.cc

@@ -60,7 +60,7 @@ MediaSession::~MediaSession() {
   DCHECK(audio_focus_state_ == State::INACTIVE);
 }
 
-void MediaSession::SetMetadata(const MediaMetadata& metadata) {
+void MediaSession::SetMetadata(const base::Optional<MediaMetadata>& metadata) {
   metadata_ = metadata;
   // TODO(zqzhang): On Android, the metadata is sent though JNI everytime the
   // media session play/pause state changes. Need to find a way to seprate the

content/browser/media/session/media_session.h

@@ -10,6 +10,7 @@
 #include "base/callback_list.h"
 #include "base/id_map.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "content/browser/media/session/audio_focus_manager.h"
 #include "content/browser/media/session/media_session_uma_helper.h"
 #include "content/common/content_export.h"
@@ -63,8 +64,8 @@ class MediaSession : public WebContentsObserver,
 
   ~MediaSession() override;
 
-  void SetMetadata(const MediaMetadata& metadata);
-  const MediaMetadata& metadata() const { return metadata_; }
+  void SetMetadata(const base::Optional<MediaMetadata>& metadata);
+  const base::Optional<MediaMetadata>& metadata() const { return metadata_; }
 
   // Adds the given player to the current media session. Returns whether the
   // player was successfully added. If it returns false, AddPlayer() should be
@@ -218,7 +219,7 @@ class MediaSession : public WebContentsObserver,
   // StopDucking().
   bool is_ducking_;
 
-  MediaMetadata metadata_;
+  base::Optional<MediaMetadata> metadata_;
   base::CallbackList<void(State)> media_session_state_listeners_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaSession);

content/browser/media/session/media_session_browsertest.cc

@@ -54,7 +54,7 @@ class MockWebContentsObserver : public WebContentsObserver {
 
   MOCK_METHOD3(MediaSessionStateChanged,
                void(bool is_controllable, bool is_suspended,
-                    const content::MediaMetadata& metadata));
+                    const base::Optional<content::MediaMetadata>& metadata));
 };
 
 }  // namespace

content/common/media/media_metadata_sanitizer.cc

@@ -7,6 +7,8 @@
 #include <algorithm>
 #include <string>
 
+#include "content/public/common/media_metadata.h"
+
 namespace content {
 
 namespace {

content/common/media/media_metadata_sanitizer.h

@@ -5,10 +5,10 @@
 #ifndef CONTENT_COMMON_MEDIA_MEDIA_METADATA_SANITIZER_H_
 #define CONTENT_COMMON_MEDIA_MEDIA_METADATA_SANITIZER_H_
 
-#include "content/public/common/media_metadata.h"
-
 namespace content {
 
+struct MediaMetadata;
+
 class MediaMetadataSanitizer {
  public:
   // Check the sanity of |metadata|.

content/common/media/media_session_messages_android.h

@@ -5,6 +5,7 @@
 // IPC messages for the Media Session API.
 // Multiply-included message file, hence no include guard.
 
+#include "base/optional.h"
 #include "content/common/android/gin_java_bridge_errors.h"
 #include "content/common/content_export.h"
 #include "content/public/common/media_metadata.h"
@@ -41,4 +42,4 @@ IPC_MESSAGE_ROUTED2(MediaSessionHostMsg_Deactivate,
 
 IPC_MESSAGE_ROUTED2(MediaSessionHostMsg_SetMetadata,
                     int /* request_id*/,
-                    content::MediaMetadata /* metadata */)
+                    base::Optional<content::MediaMetadata> /* metadata */)

content/public/browser/web_contents_observer.h

@@ -8,12 +8,12 @@
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/process/kill.h"
 #include "base/process/process_handle.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/common/frame_navigate_params.h"
-#include "content/public/common/media_metadata.h"
 #include "content/public/common/resource_type.h"
 #include "content/public/common/security_style.h"
 #include "ipc/ipc_listener.h"
@@ -37,6 +37,7 @@ struct AXLocationChangeNotificationDetails;
 struct FaviconURL;
 struct FrameNavigateParams;
 struct LoadCommittedDetails;
+struct MediaMetadata;
 struct Referrer;
 struct ResourceRedirectDetails;
 struct ResourceRequestDetails;
@@ -469,9 +470,10 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
   virtual void MediaStoppedPlaying(const MediaPlayerId& id) {}
 
   // Invoked when media session has changed its state.
-  virtual void MediaSessionStateChanged(bool is_controllable,
-                                        bool is_suspended,
-                                        const MediaMetadata& metadata) {}
+  virtual void MediaSessionStateChanged(
+      bool is_controllable,
+      bool is_suspended,
+      const base::Optional<MediaMetadata>& metadata) {}
 
   // Invoked when the renderer process changes the page scale factor.
   virtual void OnPageScaleFactorChanged(float page_scale_factor) {}

content/renderer/media/android/renderer_media_session_manager.cc

@@ -5,6 +5,7 @@
 #include "content/renderer/media/android/renderer_media_session_manager.h"
 
 #include "base/logging.h"
+#include "base/optional.h"
 #include "content/common/media/media_metadata_sanitizer.h"
 #include "content/common/media/media_session_messages_android.h"
 #include "content/public/common/media_metadata.h"
@@ -64,15 +65,16 @@ void RendererMediaSessionManager::Deactivate(
 }
 
 void RendererMediaSessionManager::SetMetadata(
-    int session_id,
-    const MediaMetadata& metadata) {
+    int session_id, const base::Optional<MediaMetadata>& metadata) {
 
   // TODO(zqzhang): print a console warning when metadata is dirty. See
   // https://crbug.com/625244.
   Send(new MediaSessionHostMsg_SetMetadata(
       routing_id(), session_id,
-      MediaMetadataSanitizer::CheckSanity(metadata) ?
-      metadata : MediaMetadataSanitizer::Sanitize(metadata)));
+      (!metadata.has_value() ||
+       MediaMetadataSanitizer::CheckSanity(metadata.value()))
+          ? metadata
+          : MediaMetadataSanitizer::Sanitize(metadata.value())));
 }
 
 void RendererMediaSessionManager::OnDidActivate(int request_id, bool success) {

content/renderer/media/android/renderer_media_session_manager.h

@@ -10,6 +10,7 @@
 
 #include "base/id_map.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "content/common/content_export.h"
 #include "content/public/renderer/render_frame_observer.h"
 #include "third_party/WebKit/public/platform/modules/mediasession/WebMediaSession.h"
@@ -36,7 +37,8 @@ class CONTENT_EXPORT RendererMediaSessionManager : public RenderFrameObserver {
   void Deactivate(
       int session_id,
       std::unique_ptr<blink::WebMediaSessionDeactivateCallback> callback);
-  void SetMetadata(int session_id, const MediaMetadata& metadata);
+  void SetMetadata(
+      int session_id, const base::Optional<MediaMetadata>& metadata);
 
   void OnDidActivate(int request_id, bool success);
   void OnDidDeactivate(int request_id);

content/renderer/media/android/webmediasession_android.cc

@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/optional.h"
 #include "content/public/common/media_metadata.h"
 #include "content/renderer/media/android/renderer_media_session_manager.h"
 #include "third_party/WebKit/public/platform/WebIconSizesParser.h"
@@ -39,11 +40,12 @@ void WebMediaSessionAndroid::deactivate(
 
 void WebMediaSessionAndroid::setMetadata(
     const blink::WebMediaMetadata* web_metadata) {
-  MediaMetadata metadata;
+  base::Optional<MediaMetadata> metadata;
   if (web_metadata) {
-    metadata.title = web_metadata->title;
-    metadata.artist = web_metadata->artist;
-    metadata.album = web_metadata->album;
+    metadata = MediaMetadata();
+    metadata->title = web_metadata->title;
+    metadata->artist = web_metadata->artist;
+    metadata->album = web_metadata->album;
     for (const auto& web_artwork : web_metadata->artwork) {
       MediaMetadata::Artwork artwork;
       artwork.src = GURL(base::string16(web_artwork.src));
@@ -53,7 +55,7 @@ void WebMediaSessionAndroid::setMetadata(
       artwork.sizes.insert(artwork.sizes.end(),
                            web_sizes.begin(),
                            web_sizes.end());
-      metadata.artwork.push_back(artwork);
+      metadata->artwork.push_back(artwork);
     }
   }