[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)

Change-Id: I2f20d94762a2908c7531ce35cc2df110e5ba13aa
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#690096}
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/branch-heads/3865@{#600}
Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>

6 files changed, 61 insertions, 11 deletions

diff --git a/chromium/content/browser/media/session/media_session_controller.cc b/chromium/content/browser/media/session/media_session_controller.cc
index fa1a8a50405..7475453edd0 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_);
 }
 
@@ -62,10 +60,8 @@ bool MediaSessionController::Initialize(
   // we already have a session from a previous call, release it.
   if (!has_audio_ || is_remote ||
       media_web_contents_observer_->web_contents()->IsAudioMuted()) {
-    if (has_session_) {
-      has_session_ = false;
-      media_session_->RemovePlayer(this, player_id_);
-    }
+    has_session_ = false;
+    media_session_->RemovePlayer(this, player_id_);
     return true;
   }
 
@@ -135,7 +131,7 @@ void MediaSessionController::WebContentsMutedStateChanged(bool muted) {
   if (!muted && !has_session_) {
     if (media_session_->AddPlayer(this, player_id_, media_content_type_))
       has_session_ = true;
-  } else if (muted && has_session_) {
+  } else if (muted) {
     has_session_ = false;
     media_session_->RemovePlayer(this, player_id_);
   }
diff --git a/chromium/content/browser/media/session/media_session_controller.h b/chromium/content/browser/media/session/media_session_controller.h
index cff0dc78c0e..3f5c73af0cd 100644
--- a/chromium/content/browser/media/session/media_session_controller.h
+++ b/chromium/content/browser/media/session/media_session_controller.h
@@ -58,6 +58,8 @@ class CONTENT_EXPORT MediaSessionController
   void WebContentsMutedStateChanged(bool muted);
 
  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 9b4e96063a9..27abe648ba2 100644
--- a/chromium/content/browser/media/session/media_session_controller_unittest.cc
+++ b/chromium/content/browser/media/session/media_session_controller_unittest.cc
@@ -71,6 +71,8 @@ class MediaSessionControllerTest : public RenderViewHostImplTestHarness {
                                        multiplier);
   }
 
+  void ResetHasSessionBit() { controller_->has_session_ = false; }
+
   template <typename T>
   bool ReceivedMessagePlayPause() {
     const IPC::Message* msg = test_sink().GetUniqueMessageMatching(T::ID);
@@ -265,4 +267,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 25867580366..b33442850e7 100644
--- a/chromium/content/browser/media/session/media_session_impl.cc
+++ b/chromium/content/browser/media/session/media_session_impl.cc
@@ -807,7 +807,7 @@ void MediaSessionImpl::FinishSystemAudioFocusRequest(
 
   OnSystemAudioFocusRequested(result);
 
-  if (!result) {
+  if (!result && !HasPepper()) {
     switch (audio_focus_type) {
       case AudioFocusType::kGain:
         // If the gain audio focus request failed then we should suspend the
@@ -913,7 +913,9 @@ bool MediaSessionImpl::AddPepperPlayer(MediaSessionPlayerObserver* observer,
                                        int player_id) {
   AudioFocusDelegate::AudioFocusResult result =
       RequestSystemAudioFocus(AudioFocusType::kGain);
-  DCHECK_NE(AudioFocusDelegate::AudioFocusResult::kFailed, result);
+
+  if (result == AudioFocusDelegate::AudioFocusResult::kFailed)
+    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 84c26918664..a7cd8c15aee 100644
--- a/chromium/content/browser/media/session/media_session_impl.h
+++ b/chromium/content/browser/media/session/media_session_impl.h
@@ -130,7 +130,7 @@ class MediaSessionImpl : public MediaSession,
   CONTENT_EXPORT bool IsSuspended() const;
 
   // Returns whether the session has Pepper instances.
-  bool HasPepper() const;
+  CONTENT_EXPORT bool HasPepper() const;
 
   // WebContentsObserver implementation
   void WebContentsDestroyed() override;
diff --git a/chromium/content/browser/media/session/media_session_impl_browsertest.cc b/chromium/content/browser/media/session/media_session_impl_browsertest.cc
index e7a23dcc40c..087ee69ef94 100644
--- a/chromium/content/browser/media/session/media_session_impl_browsertest.cc
+++ b/chromium/content/browser/media/session/media_session_impl_browsertest.cc
@@ -70,7 +70,7 @@ class MockAudioFocusDelegate : public AudioFocusDelegate {
       return AudioFocusDelegate::AudioFocusResult::kDelayed;
     } else {
       audio_focus_type_ = audio_focus_type;
-      return AudioFocusDelegate::AudioFocusResult::kSuccess;
+      return sync_result_;
     }
   }
 
@@ -94,7 +94,14 @@ class MockAudioFocusDelegate : public AudioFocusDelegate {
 
   bool HasRequests() const { return !requests_.empty(); }
 
+  void SetSyncResult(AudioFocusDelegate::AudioFocusResult result) {
+    sync_result_ = result;
+  }
+
  private:
+  AudioFocusDelegate::AudioFocusResult sync_result_ =
+      AudioFocusDelegate::AudioFocusResult::kSuccess;
+
   MediaSessionImpl* media_session_;
   const bool async_mode_ = false;
 
@@ -219,6 +226,10 @@ class MediaSessionImplBrowserTest : public content::ContentBrowserTest {
     mock_audio_focus_delegate()->ResolveRequest(false /* result */);
   }
 
+  void SetSyncAudioFocusResult(AudioFocusDelegate::AudioFocusResult result) {
+    mock_audio_focus_delegate()->SetSyncResult(result);
+  }
+
   bool HasUnresolvedAudioFocusRequest() {
     return mock_audio_focus_delegate()->HasRequests();
   }
@@ -278,6 +289,18 @@ class MediaSessionImplParamBrowserTest
   }
 };
 
+class MediaSessionImplSyncBrowserTest : public MediaSessionImplBrowserTest {
+ protected:
+  MediaSessionImplSyncBrowserTest() = default;
+
+  void SetUpOnMainThread() override {
+    MediaSessionImplBrowserTest::SetUpOnMainThread();
+
+    SetAudioFocusDelegateForTests(new NiceMock<MockAudioFocusDelegate>(
+        media_session_, false /* async_mode */));
+  }
+};
+
 INSTANTIATE_TEST_CASE_P(, MediaSessionImplParamBrowserTest, testing::Bool());
 
 IN_PROC_BROWSER_TEST_P(MediaSessionImplParamBrowserTest,
@@ -1902,6 +1925,19 @@ IN_PROC_BROWSER_TEST_P(MediaSessionImplParamBrowserTest,
   }
 }
 
+IN_PROC_BROWSER_TEST_F(MediaSessionImplSyncBrowserTest,
+                       PepperPlayerNotAddedIfFocusFailed) {
+  SetSyncAudioFocusResult(AudioFocusDelegate::AudioFocusResult::kFailed);
+
+  auto player_observer = std::make_unique<MockMediaSessionPlayerObserver>();
+  int player_id = player_observer->StartNewPlayer();
+
+  EXPECT_FALSE(AddPlayer(player_observer.get(), player_id,
+                         media::MediaContentType::Pepper));
+
+  EXPECT_FALSE(media_session_->HasPepper());
+}
+
 IN_PROC_BROWSER_TEST_F(MediaSessionImplBrowserTest, Async_RequestFailure_Gain) {
   auto player_observer = std::make_unique<MockMediaSessionPlayerObserver>();