Skip to content

GitLab

T
tangled
Commit 5f0076f2 authored by Becca Hughes's avatar Becca Hughes Committed by Commit Bot
Browse files
Options

Fix position timing when paused 

When we pause the media session we
should update the position to the
current position so we know were
the media was paused.

BUG=1049530

Change-Id: If651edc4a4be1bdefeaa2638289f7b0399e1094e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2047606Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarTommy Steimel <steimel@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740438}
parent 4cf696f5
Hide whitespace changes
Inline Side-by-side
Showing with 84 additions and 22 deletions
third_party/blink/renderer/modules/mediasession/DEPS
View file @ 5f0076f2
include_rules = [
"+base/test/simple_test_tick_clock.h",
"+services/media_session/public/mojom",
"-third_party/blink/renderer/modules",
"+third_party/blink/renderer/modules/event_target_modules.h",
......
third_party/blink/renderer/modules/mediasession/media_session.cc
View file @ 5f0076f2
......@@ -5,7 +5,9 @@
#include "third_party/blink/renderer/modules/mediasession/media_session.h"
#include <memory>
#include "base/optional.h"
#include "base/time/default_tick_clock.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_media_position_state.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_media_session_action_details.h"
......@@ -125,6 +127,7 @@ mojom::blink::MediaSessionPlaybackState StringToMediaSessionPlaybackState(
MediaSession::MediaSession(ExecutionContext* execution_context)
: ContextClient(execution_context),
clock_(base::DefaultTickClock::GetInstance()),
playback_state_(mojom::blink::MediaSessionPlaybackState::NONE) {}
void MediaSession::Dispose() {
......@@ -134,7 +137,7 @@ void MediaSession::Dispose() {
void MediaSession::setPlaybackState(const String& playback_state) {
playback_state_ = StringToMediaSessionPlaybackState(playback_state);
RecalculatePositionState(false /* notify */);
RecalculatePositionState(/*was_set=*/false);
mojom::blink::MediaSessionService* service = GetService();
if (service)
......@@ -263,7 +266,7 @@ void MediaSession::setPositionState(MediaPositionState* position_state,
declared_playback_rate_ = position_state_->playback_rate;
RecalculatePositionState(true /* notify */);
RecalculatePositionState(/*was_set=*/true);
}
void MediaSession::NotifyActionChange(const String& action,
......@@ -285,7 +288,25 @@ void MediaSession::NotifyActionChange(const String& action,
}
}
void MediaSession::RecalculatePositionState(bool notify) {
base::TimeDelta MediaSession::GetPositionNow() const {
const base::TimeTicks now = clock_->NowTicks();
const base::TimeDelta elapsed_time =
position_state_->playback_rate *
(now - position_state_->last_updated_time);
const base::TimeDelta updated_position =
position_state_->position + elapsed_time;
const base::TimeDelta start = base::TimeDelta::FromSeconds(0);
if (updated_position <= start)
return start;
else if (updated_position >= position_state_->duration)
return position_state_->duration;
else
return updated_position;
}
void MediaSession::RecalculatePositionState(bool was_set) {
if (!position_state_)
return;
......@@ -294,12 +315,18 @@ void MediaSession::RecalculatePositionState(bool notify) {
? 0.0
: declared_playback_rate_;
notify = notify || new_playback_rate != position_state_->playback_rate;
position_state_->playback_rate = new_playback_rate;
if (!notify)
if (!was_set && new_playback_rate == position_state_->playback_rate)
return;
// If we updated the position state because of the playback rate then we
// should update the time.
if (!was_set) {
position_state_->position = GetPositionNow();
}
position_state_->playback_rate = new_playback_rate;
position_state_->last_updated_time = clock_->NowTicks();
if (auto* service = GetService())
service->SetPositionState(position_state_.Clone());
}
......
third_party/blink/renderer/modules/mediasession/media_session.h
View file @ 5f0076f2
......@@ -15,6 +15,10 @@
#include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
namespace base {
class TickClock;
} // namespace base
namespace blink {
class ExecutionContext;
......@@ -65,7 +69,9 @@ class MODULES_EXPORT MediaSession final
void NotifyActionChange(const String& action, ActionChangeType);
void RecalculatePositionState(bool notify);
base::TimeDelta GetPositionNow() const;
void RecalculatePositionState(bool was_set);
// blink::mojom::blink::MediaSessionClient implementation.
void DidReceiveAction(media_session::mojom::blink::MediaSessionAction,
......@@ -74,6 +80,8 @@ class MODULES_EXPORT MediaSession final
// Returns null when the ExecutionContext is not document.
mojom::blink::MediaSessionService* GetService();
const base::TickClock* clock_ = nullptr;
mojom::blink::MediaSessionPlaybackState playback_state_;
media_session::mojom::blink::MediaPositionPtr position_state_;
double declared_playback_rate_ = 0.0;
......
third_party/blink/renderer/modules/mediasession/media_session_test.cc
View file @ 5f0076f2
......@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/modules/mediasession/media_session.h"
#include "base/macros.h"
#include "base/test/simple_test_tick_clock.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -57,6 +58,7 @@ class MediaSessionTest : public PageTestBase {
media_session_ =
MakeGarbageCollected<MediaSession>(GetDocument().ToExecutionContext());
media_session_->service_ = mock_service_->CreateRemoteAndBind();
media_session_->clock_ = &test_clock_;
}
void SetPositionState(double duration,
......@@ -83,7 +85,11 @@ class MediaSessionTest : public PageTestBase {
MockMediaSessionService& service() { return *mock_service_.get(); }
base::SimpleTestTickClock& clock() { return test_clock_; }
private:
base::SimpleTestTickClock test_clock_;
std::unique_ptr<MockMediaSessionService> mock_service_;
Persistent<MediaSession> media_session_;
......@@ -98,6 +104,7 @@ TEST_F(MediaSessionTest, PlaybackPositionState_None) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(1.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -114,6 +121,7 @@ TEST_F(MediaSessionTest, PlaybackPositionState_Paused) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(0.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -130,6 +138,7 @@ TEST_F(MediaSessionTest, PlaybackPositionState_Playing) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(1.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -147,6 +156,7 @@ TEST_F(MediaSessionTest, PlaybackPositionState_Paused_Clear) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(0.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -176,6 +186,7 @@ TEST_F(MediaSessionTest, PositionPlaybackState_None) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(1.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -190,24 +201,28 @@ TEST_F(MediaSessionTest, PositionPlaybackState_Paused_None) {
base::RunLoop loop;
EXPECT_CALL(service(), SetPositionState(_))
.WillOnce(testing::Invoke([&](auto position_state) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(base::TimeDelta::FromMinutes(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromMinutes(1), position_state->position);
EXPECT_EQ(1.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
SetPositionState(10, 5, 1.0);
SetPositionState(600, 60, 1.0);
loop.Run();
}
clock().Advance(base::TimeDelta::FromMinutes(1));
{
base::RunLoop loop;
EXPECT_CALL(service(), SetPositionState(_))
.WillOnce(testing::Invoke([&](auto position_state) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(base::TimeDelta::FromMinutes(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromMinutes(2), position_state->position);
EXPECT_EQ(0.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -216,13 +231,16 @@ TEST_F(MediaSessionTest, PositionPlaybackState_Paused_None) {
loop.Run();
}
clock().Advance(base::TimeDelta::FromMinutes(1));
{
base::RunLoop loop;
EXPECT_CALL(service(), SetPositionState(_))
.WillOnce(testing::Invoke([&](auto position_state) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(base::TimeDelta::FromMinutes(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromMinutes(2), position_state->position);
EXPECT_EQ(1.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -237,24 +255,28 @@ TEST_F(MediaSessionTest, PositionPlaybackState_Paused_Playing) {
base::RunLoop loop;
EXPECT_CALL(service(), SetPositionState(_))
.WillOnce(testing::Invoke([&](auto position_state) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(base::TimeDelta::FromMinutes(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromMinutes(1), position_state->position);
EXPECT_EQ(1.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
SetPositionState(10, 5, 1.0);
SetPositionState(600, 60, 1.0);
loop.Run();
}
clock().Advance(base::TimeDelta::FromMinutes(1));
{
base::RunLoop loop;
EXPECT_CALL(service(), SetPositionState(_))
.WillOnce(testing::Invoke([&](auto position_state) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(base::TimeDelta::FromMinutes(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromMinutes(2), position_state->position);
EXPECT_EQ(0.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -263,13 +285,16 @@ TEST_F(MediaSessionTest, PositionPlaybackState_Paused_Playing) {
loop.Run();
}
clock().Advance(base::TimeDelta::FromMinutes(1));
{
base::RunLoop loop;
EXPECT_CALL(service(), SetPositionState(_))
.WillOnce(testing::Invoke([&](auto position_state) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(base::TimeDelta::FromMinutes(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromMinutes(2), position_state->position);
EXPECT_EQ(1.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......@@ -286,6 +311,7 @@ TEST_F(MediaSessionTest, PositionPlaybackState_Playing) {
EXPECT_EQ(base::TimeDelta::FromSeconds(10), position_state->duration);
EXPECT_EQ(base::TimeDelta::FromSeconds(5), position_state->position);
EXPECT_EQ(1.0, position_state->playback_rate);
EXPECT_EQ(clock().NowTicks(), position_state->last_updated_time);
loop.Quit();
}));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment