diff options
author | Michael BrĂ¼ning <michael.bruning@qt.io> | 2019-12-04 11:32:55 +0100 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2019-12-05 13:24:18 +0000 |
commit | e76f8b22419403db7ab4bb43cc6c165c11088484 (patch) | |
tree | 58a1596d101a20675eaea5373e19a5804845df93 | |
parent | 4f7d6ea2e7961a0b688202f65194915d4d1af12f (diff) | |
download | qtwebengine-chromium-e76f8b22419403db7ab4bb43cc6c165c11088484.tar.gz |
[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; |