[Backport] CVE-2019-5876

[Media Session] Fix issues in media session

For more context, please see the bug. This CL
is two part:

1) Unconditionally remove the player from
   the media session
2) Do not add a pepper player if focus fails

BUG=997190

(cherry picked from commit e30383d507bb7f94a42a32c42d98ff2dd4811166)

Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/branch-heads/3865@{#600}
Cr-Branched-From:
0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}

chromium/content/browser/media/session/media_session_controller.cc
chromium/content/browser/media/session/media_session_impl_browsertest.cc

Change-Id: I8a90ffddcd3fd6a3f55c1b6036b89bb1b5dcd020
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

5 files changed, 20 insertions, 8 deletions

diff --git a/chromium/content/browser/media/session/media_session_controller.cc b/chromium/content/browser/media/session/media_session_controller.cc
index 970bb4cb738..ac4a4958386 100644
--- a/chromium/content/browser/media/session/media_session_controller.cc
+++ b/chromium/content/browser/media/session/media_session_controller.cc
@@ -23,8 +23,6 @@ MediaSessionController::MediaSessionController(
 }
 
 MediaSessionController::~MediaSessionController() {
-  if (!has_session_)
-    return;
   media_session_->RemovePlayer(this, player_id_);
 }
 
@@ -56,10 +54,8 @@ bool MediaSessionController::Initialize(
   // Don't bother with a MediaSession for remote players or without audio.  If
   // we already have a session from a previous call, release it.
   if (!has_audio || is_remote) {
-    if (has_session_) {
-      has_session_ = false;
-      media_session_->RemovePlayer(this, player_id_);
-    }
+    has_session_ = false;
+    media_session_->RemovePlayer(this, player_id_);
     return true;
   }
 
diff --git a/chromium/content/browser/media/session/media_session_controller.h b/chromium/content/browser/media/session/media_session_controller.h
index f709828bd2b..fa2c181d43d 100644
--- a/chromium/content/browser/media/session/media_session_controller.h
+++ b/chromium/content/browser/media/session/media_session_controller.h
@@ -55,6 +55,8 @@ class CONTENT_EXPORT MediaSessionController
   int get_player_id_for_testing() const { return player_id_; }
 
  private:
+  friend class MediaSessionControllerTest;
+
   const WebContentsObserver::MediaPlayerId id_;
 
   // Non-owned pointer; |media_web_contents_observer_| is the owner of |this|.
diff --git a/chromium/content/browser/media/session/media_session_controller_unittest.cc b/chromium/content/browser/media/session/media_session_controller_unittest.cc
index c2c2868ab12..dedee0f4215 100644
--- a/chromium/content/browser/media/session/media_session_controller_unittest.cc
+++ b/chromium/content/browser/media/session/media_session_controller_unittest.cc
@@ -53,6 +53,7 @@ class MediaSessionControllerTest : public RenderViewHostImplTestHarness {
     controller_->OnSetVolumeMultiplier(controller_->get_player_id_for_testing(),
                                        multiplier);
   }
+  void ResetHasSessionBit() { controller_->has_session_ = false; }
 
   template <typename T>
   bool ReceivedMessagePlayPause() {
@@ -214,4 +215,16 @@ TEST_F(MediaSessionControllerTest, Reinitialize) {
   EXPECT_EQ(current_player_id, controller_->get_player_id_for_testing());
 }
 
+TEST_F(MediaSessionControllerTest, RemovePlayerIfSessionReset) {
+  ASSERT_TRUE(controller_->Initialize(
+      true, false, media::MediaContentType::Persistent, nullptr));
+  EXPECT_TRUE(media_session()->IsActive());
+
+  ResetHasSessionBit();
+  EXPECT_TRUE(media_session()->IsActive());
+
+  controller_.reset();
+  EXPECT_FALSE(media_session()->IsActive());
+}
+
 }  // namespace content
diff --git a/chromium/content/browser/media/session/media_session_impl.cc b/chromium/content/browser/media/session/media_session_impl.cc
index 1b5e706c47a..733c72cd60b 100644
--- a/chromium/content/browser/media/session/media_session_impl.cc
+++ b/chromium/content/browser/media/session/media_session_impl.cc
@@ -531,7 +531,8 @@ bool MediaSessionImpl::AddPepperPlayer(MediaSessionPlayerObserver* observer,
                                        int player_id) {
   bool success =
       RequestSystemAudioFocus(AudioFocusManager::AudioFocusType::Gain);
-  DCHECK(success);
+  if (!success)
+    return false;
 
   pepper_players_.insert(PlayerIdentifier(observer, player_id));
 
diff --git a/chromium/content/browser/media/session/media_session_impl.h b/chromium/content/browser/media/session/media_session_impl.h
index 1f1a0fc85d1..018b1fce020 100644
--- a/chromium/content/browser/media/session/media_session_impl.h
+++ b/chromium/content/browser/media/session/media_session_impl.h
@@ -167,7 +167,7 @@ class MediaSessionImpl : public MediaSession,
   }
 
   // Returns whether the session has Pepper instances.
-  bool HasPepper() const;
+  CONTENT_EXPORT bool HasPepper() const;
 
   // WebContentsObserver implementation
   void WebContentsDestroyed() override;