Skip to content

Commit

Permalink
MetronomeSource: introduce TickProvider.
Browse files Browse the repository at this point in the history
This CL prepares the MetronomeSource to be able to accept other types
of tick providers like those sourced by VSync signals or explicitly
switchable tick providers.

Bug: chromium:1381982
Change-Id: I7a1962495d3a271f05a38b8c1ae9ba16d7eb3be2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4004709
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Markus Handell <handellm@google.com>
Reviewed-by: Francois Pierre Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1073355}
  • Loading branch information
handellm authored and Chromium LUCI CQ committed Nov 18, 2022
1 parent 6fe608f commit 9e01876
Show file tree
Hide file tree
Showing 16 changed files with 393 additions and 226 deletions.
4 changes: 2 additions & 2 deletions base/task/sequenced_task_runner.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

namespace blink {
class LowPrecisionTimer;
class MetronomeSource;
class TimerBase;
class TimerBasedTickProvider;
class WebRtcTaskQueue;
}
namespace webrtc {
Expand Down Expand Up @@ -55,8 +55,8 @@ class PostDelayedTaskPassKey {
friend class base::DeadlineTimer;
friend class base::MetronomeTimer;
friend class blink::LowPrecisionTimer;
friend class blink::MetronomeSource;
friend class blink::TimerBase;
friend class blink::TimerBasedTickProvider;
friend class blink::WebRtcTaskQueue;
friend class PostDelayedTaskPassKeyForTesting;
friend class webrtc::ThreadWrapper;
Expand Down
5 changes: 4 additions & 1 deletion components/webrtc/thread_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/memory/raw_ptr.h"
#include "base/sequence_checker.h"
#include "base/synchronization/waitable_event.h"
#include "base/task/sequenced_task_runner.h"
#include "base/thread_annotations.h"
#include "base/threading/thread_local.h"
#include "base/threading/thread_task_runner_handle.h"
Expand All @@ -23,6 +24,7 @@
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/webrtc/rtc_base/physical_socket_server.h"
#include "third_party/webrtc_overrides/metronome_source.h"
#include "third_party/webrtc_overrides/timer_based_tick_provider.h"

namespace webrtc {
namespace {
Expand Down Expand Up @@ -237,7 +239,8 @@ void ThreadWrapper::PostDelayedTask(absl::AnyInvocable<void() &&> task,
base::TimeTicks::Now() + base::Microseconds(delay.us());
// Coalesce low precision tasks onto the metronome.
base::TimeTicks snapped_target_time =
blink::MetronomeSource::TimeSnappedToNextTick(target_time);
blink::TimerBasedTickProvider::TimeSnappedToNextTick(
target_time, blink::TimerBasedTickProvider::kDefaultPeriod);
if (coalesced_tasks_.QueueDelayedTask(target_time, std::move(task),
snapped_target_time)) {
task_runner_->PostDelayedTaskAt(
Expand Down
7 changes: 5 additions & 2 deletions components/webrtc/thread_wrapper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "third_party/webrtc/api/task_queue/task_queue_test.h"
#include "third_party/webrtc_overrides/metronome_source.h"
#include "third_party/webrtc_overrides/test/metronome_like_task_queue_test.h"
#include "third_party/webrtc_overrides/timer_based_tick_provider.h"

namespace webrtc {
namespace {
Expand Down Expand Up @@ -176,10 +177,12 @@ class ThreadWrapperProvider : public blink::MetronomeLikeTaskQueueProvider {

base::TimeDelta DeltaToNextTick() const override {
base::TimeTicks now = base::TimeTicks::Now();
return blink::MetronomeSource::TimeSnappedToNextTick(now) - now;
return blink::TimerBasedTickProvider::TimeSnappedToNextTick(
now, blink::TimerBasedTickProvider::kDefaultPeriod) -
now;
}
base::TimeDelta MetronomeTick() const override {
return blink::MetronomeSource::Tick();
return blink::TimerBasedTickProvider::kDefaultPeriod;
}
webrtc::TaskQueueBase* TaskQueue() const override { return thread_; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "base/metrics/histogram_macros.h"
#include "base/synchronization/waitable_event.h"
#include "base/system/sys_info.h"
#include "base/task/sequenced_task_runner.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "components/webrtc/thread_wrapper.h"
Expand Down Expand Up @@ -74,6 +75,7 @@
#include "third_party/webrtc/rtc_base/ref_counted_object.h"
#include "third_party/webrtc/rtc_base/ssl_adapter.h"
#include "third_party/webrtc_overrides/task_queue_factory.h"
#include "third_party/webrtc_overrides/timer_based_tick_provider.h"

namespace WTF {
template <>
Expand Down Expand Up @@ -190,7 +192,8 @@ class PeerConnectionStaticDeps {
webrtc::ThreadWrapper::current()->set_send_allowed(true);
if (!metronome_source_) {
metronome_source_ = std::make_unique<MetronomeSource>(
chrome_worker_thread_.task_runner());
std::make_unique<TimerBasedTickProvider>(
TimerBasedTickProvider::kDefaultPeriod));
}
}

Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/platform/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2145,6 +2145,7 @@ source_set("blink_platform_unittests_sources") {
"peerconnection/stats_collecting_encoder_test.cc",
"peerconnection/stats_collector_test.cc",
"peerconnection/task_queue_factory_test.cc",
"peerconnection/timer_based_tick_provider_test.cc",
"peerconnection/two_keys_adapter_map_unittest.cc",
"peerconnection/webrtc_audio_sink_test.cc",
"peerconnection/webrtc_decoding_info_handler_test.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@
#include "base/time/time.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/webrtc_overrides/metronome_source.h"
#include "third_party/webrtc_overrides/timer_based_tick_provider.h"

namespace blink {

namespace {

base::TimeDelta TickPeriod() {
return TimerBasedTickProvider::kDefaultPeriod;
}

class LowPrecisionTimerTest : public ::testing::Test {
public:
LowPrecisionTimerTest()
Expand All @@ -23,7 +28,9 @@ class LowPrecisionTimerTest : public ::testing::Test {
// Ensure mock time is aligned with metronome tick.
base::TimeTicks now = base::TimeTicks::Now();
task_environment_.FastForwardBy(
MetronomeSource::TimeSnappedToNextTick(now) - now);
TimerBasedTickProvider::TimeSnappedToNextTick(
now, TimerBasedTickProvider::kDefaultPeriod) -
now);
}

protected:
Expand Down Expand Up @@ -135,37 +142,36 @@ TEST_F(LowPrecisionTimerTest, StartOneShot) {
base::Unretained(&listener)));

// Schedule to fire on the first tick.
timer.StartOneShot(MetronomeSource::Tick());
task_environment_.FastForwardBy(MetronomeSource::Tick());
timer.StartOneShot(TickPeriod());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 1u);

// The task does not repeat automatically.
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 1u);

// Schedule to fire a millisecond before the next tick. Advancing to that
// time does not result in a callback.
timer.StartOneShot(MetronomeSource::Tick() - base::Milliseconds(1));
task_environment_.FastForwardBy(MetronomeSource::Tick() -
base::Milliseconds(1));
timer.StartOneShot(TickPeriod() - base::Milliseconds(1));
task_environment_.FastForwardBy(TickPeriod() - base::Milliseconds(1));
EXPECT_EQ(listener.callback_count(), 1u);
// But it fires on the next tick.
task_environment_.FastForwardBy(base::Milliseconds(1));
EXPECT_EQ(listener.callback_count(), 2u);

// Fire a little after the next tick. Two ticks has to pass before anything
// happens.
timer.StartOneShot(MetronomeSource::Tick() + base::Milliseconds(1));
task_environment_.FastForwardBy(MetronomeSource::Tick());
timer.StartOneShot(TickPeriod() + base::Milliseconds(1));
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 2u);
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 3u);

// Schedule to fire but shutdown the timer before it has time to fire.
timer.StartOneShot(MetronomeSource::Tick());
timer.StartOneShot(TickPeriod());
timer.Shutdown();

task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 3u);
}

Expand All @@ -176,7 +182,7 @@ TEST_F(LowPrecisionTimerTest, RecursiveStartOneShot) {
// A full tick is needed before the callback fires.
task_environment_.FastForwardBy(delay);
EXPECT_EQ(recursive_shotter.callback_count(), 0u);
task_environment_.FastForwardBy(MetronomeSource::Tick() - delay);
task_environment_.FastForwardBy(TickPeriod() - delay);
EXPECT_EQ(recursive_shotter.callback_count(), 1u);

// The same is true the second time it fires. This is not a high precision
Expand All @@ -185,11 +191,11 @@ TEST_F(LowPrecisionTimerTest, RecursiveStartOneShot) {
// higher precision.
task_environment_.FastForwardBy(delay);
EXPECT_EQ(recursive_shotter.callback_count(), 1u);
task_environment_.FastForwardBy(MetronomeSource::Tick() - delay);
task_environment_.FastForwardBy(TickPeriod() - delay);
EXPECT_EQ(recursive_shotter.callback_count(), 2u);

// It is not repeated a third time.
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(recursive_shotter.callback_count(), 2u);
}

Expand All @@ -200,9 +206,8 @@ TEST_F(LowPrecisionTimerTest, MoveToNewTaskRunner) {
base::Unretained(&listener)));

// Schedule on the next tick, and advance time close to that.
timer.StartOneShot(MetronomeSource::Tick());
task_environment_.FastForwardBy(MetronomeSource::Tick() -
base::Milliseconds(3));
timer.StartOneShot(TickPeriod());
task_environment_.FastForwardBy(TickPeriod() - base::Milliseconds(3));
EXPECT_EQ(listener.callback_count(), 0u);

// Move to a new task runner. The CallbackListener will EXPECT_TRUE that the
Expand All @@ -229,17 +234,16 @@ TEST_F(LowPrecisionTimerTest, StartRepeating) {
task_environment_.FastForwardBy(base::Milliseconds(10));
EXPECT_EQ(listener.callback_count(), 0u);
// But it does repeat on every tick.
task_environment_.FastForwardBy(MetronomeSource::Tick() -
base::Milliseconds(10));
task_environment_.FastForwardBy(TickPeriod() - base::Milliseconds(10));
EXPECT_EQ(listener.callback_count(), 1u);
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 2u);
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 3u);
timer.Shutdown();

// The timer stops on shutdown.
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 3u);
}

Expand All @@ -250,25 +254,25 @@ TEST_F(LowPrecisionTimerTest, StopRepeatingTimer) {
base::Unretained(&listener)));

// Repeat every tick.
timer.StartRepeating(MetronomeSource::Tick());
task_environment_.FastForwardBy(MetronomeSource::Tick());
timer.StartRepeating(TickPeriod());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 1u);
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 2u);

// Stop the timer and ensure it stops repeating.
timer.Stop();
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 2u);

// The timer is reusable - can start and stop again.
timer.StartRepeating(MetronomeSource::Tick());
task_environment_.FastForwardBy(MetronomeSource::Tick());
timer.StartRepeating(TickPeriod());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 3u);
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 4u);
timer.Stop();
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(listener.callback_count(), 4u);

// Cleanup.
Expand All @@ -278,12 +282,12 @@ TEST_F(LowPrecisionTimerTest, StopRepeatingTimer) {
// Ensures stopping inside the timer callback does not deadlock.
TEST_F(LowPrecisionTimerTest, StopTimerFromInsideCallback) {
// Stops its own timer from inside the callback after a tick.
RecursiveStopper recursive_stopper(MetronomeSource::Tick());
task_environment_.FastForwardBy(MetronomeSource::Tick());
RecursiveStopper recursive_stopper(TickPeriod());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(recursive_stopper.callback_count(), 1u);

// Ensure we are stopped, the callback count does not increase.
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_EQ(recursive_stopper.callback_count(), 1u);
}

Expand Down Expand Up @@ -324,18 +328,18 @@ TEST_F(LowPrecisionTimerTest, IsActive) {

// StartOneShot() makes the timer temporarily active.
EXPECT_FALSE(is_active_checker.timer().IsActive());
is_active_checker.timer().StartOneShot(MetronomeSource::Tick());
is_active_checker.timer().StartOneShot(TickPeriod());
EXPECT_TRUE(is_active_checker.timer().IsActive());
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_FALSE(is_active_checker.timer().IsActive());
// The timer is said to be inactive inside the one-shot callback.
EXPECT_FALSE(is_active_checker.was_active_in_last_callback());

// StartRepeating() makes the timer active until stopped.
EXPECT_FALSE(is_active_checker.timer().IsActive());
is_active_checker.timer().StartRepeating(MetronomeSource::Tick());
is_active_checker.timer().StartRepeating(TickPeriod());
EXPECT_TRUE(is_active_checker.timer().IsActive());
task_environment_.FastForwardBy(MetronomeSource::Tick());
task_environment_.FastForwardBy(TickPeriod());
EXPECT_TRUE(is_active_checker.timer().IsActive());
// The timer is said to be active inside the repeating callback.
EXPECT_TRUE(is_active_checker.was_active_in_last_callback());
Expand Down

0 comments on commit 9e01876

Please sign in to comment.