Commit 255feb99 authored by alexclarke's avatar alexclarke Committed by Commit Bot

Allow tasks scheduled for the instant virtual time budges expires to run

It feels strange that if a task is due to run at time t=1.0 and we gave
1s of virtual time budget that that task doesn't run.  This patch changes
that so the task will run by using a fence instead of disabling the queue.

BUG=696001

Review-Url: https://codereview.chromium.org/2909293002
Cr-Commit-Position: refs/heads/master@{#476225}
parent 0caff95c
...@@ -1422,10 +1422,10 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, VirtualTimeTest) { ...@@ -1422,10 +1422,10 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, VirtualTimeTest) {
params->SetString("expression", "console.log('done')"); params->SetString("expression", "console.log('done')");
SendCommand("Runtime.evaluate", std::move(params), true); SendCommand("Runtime.evaluate", std::move(params), true);
// The second timer shold not fire. // The third timer should not fire.
EXPECT_THAT(console_messages_, ElementsAre("before", "done")); EXPECT_THAT(console_messages_, ElementsAre("before", "at", "done"));
// Let virtual time advance for another second, which should make the second // Let virtual time advance for another second, which should make the third
// timer fire. // timer fire.
params.reset(new base::DictionaryValue()); params.reset(new base::DictionaryValue());
params->SetString("policy", "advance"); params->SetString("policy", "advance");
...@@ -1434,7 +1434,7 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, VirtualTimeTest) { ...@@ -1434,7 +1434,7 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, VirtualTimeTest) {
WaitForNotification("Emulation.virtualTimeBudgetExpired"); WaitForNotification("Emulation.virtualTimeBudgetExpired");
EXPECT_THAT(console_messages_, ElementsAre("before", "done", "at", "after")); EXPECT_THAT(console_messages_, ElementsAre("before", "at", "done", "after"));
} }
// Tests that the Security.showCertificateViewer command shows the // Tests that the Security.showCertificateViewer command shows the
......
...@@ -219,6 +219,7 @@ RendererSchedulerImpl::MainThreadOnly::MainThreadOnly( ...@@ -219,6 +219,7 @@ RendererSchedulerImpl::MainThreadOnly::MainThreadOnly(
in_idle_period_for_testing(false), in_idle_period_for_testing(false),
use_virtual_time(false), use_virtual_time(false),
is_audio_playing(false), is_audio_playing(false),
virtual_time_paused(false),
rail_mode_observer(nullptr), rail_mode_observer(nullptr),
wake_up_budget_pool(nullptr), wake_up_budget_pool(nullptr),
task_duration_reporter("RendererScheduler.TaskDurationPerQueueType2"), task_duration_reporter("RendererScheduler.TaskDurationPerQueueType2"),
...@@ -369,6 +370,8 @@ scoped_refptr<TaskQueue> RendererSchedulerImpl::NewTimerTaskQueue( ...@@ -369,6 +370,8 @@ scoped_refptr<TaskQueue> RendererSchedulerImpl::NewTimerTaskQueue(
TimeDomainType::THROTTLED) { TimeDomainType::THROTTLED) {
task_queue_throttler_->IncreaseThrottleRefCount(timer_task_queue.get()); task_queue_throttler_->IncreaseThrottleRefCount(timer_task_queue.get());
} }
if (GetMainThreadOnly().virtual_time_paused)
timer_task_queue->InsertFence(TaskQueue::InsertFencePosition::NOW);
timer_task_queue->AddTaskObserver( timer_task_queue->AddTaskObserver(
&GetMainThreadOnly().timer_task_cost_estimator); &GetMainThreadOnly().timer_task_cost_estimator);
AddQueueToWakeUpBudgetPool(timer_task_queue.get()); AddQueueToWakeUpBudgetPool(timer_task_queue.get());
...@@ -1404,6 +1407,25 @@ void RendererSchedulerImpl::ResumeTimerQueue() { ...@@ -1404,6 +1407,25 @@ void RendererSchedulerImpl::ResumeTimerQueue() {
ForceUpdatePolicy(); ForceUpdatePolicy();
} }
void RendererSchedulerImpl::VirtualTimePaused() {
DCHECK(!GetMainThreadOnly().virtual_time_paused);
GetMainThreadOnly().virtual_time_paused = true;
for (const auto& pair : timer_task_runners_) {
DCHECK(!task_queue_throttler_->IsThrottled(pair.first.get()));
DCHECK(!pair.first->HasFence());
pair.first->InsertFence(TaskQueue::InsertFencePosition::NOW);
}
}
void RendererSchedulerImpl::VirtualTimeResumed() {
DCHECK(GetMainThreadOnly().virtual_time_paused);
GetMainThreadOnly().virtual_time_paused = false;
for (const auto& pair : timer_task_runners_) {
DCHECK(!task_queue_throttler_->IsThrottled(pair.first.get()));
pair.first->RemoveFence();
}
}
void RendererSchedulerImpl::SetTimerQueueSuspensionWhenBackgroundedEnabled( void RendererSchedulerImpl::SetTimerQueueSuspensionWhenBackgroundedEnabled(
bool enabled) { bool enabled) {
// Note that this will only take effect for the next backgrounded signal. // Note that this will only take effect for the next backgrounded signal.
...@@ -1518,6 +1540,8 @@ RendererSchedulerImpl::AsValueLocked(base::TimeTicks optional_now) const { ...@@ -1518,6 +1540,8 @@ RendererSchedulerImpl::AsValueLocked(base::TimeTicks optional_now) const {
.timer_task_cost_estimator.expected_task_duration() .timer_task_cost_estimator.expected_task_duration()
.InMillisecondsF()); .InMillisecondsF());
state->SetBoolean("is_audio_playing", GetMainThreadOnly().is_audio_playing); state->SetBoolean("is_audio_playing", GetMainThreadOnly().is_audio_playing);
state->SetBoolean("virtual_time_paused",
GetMainThreadOnly().virtual_time_paused);
state->BeginDictionary("web_view_schedulers"); state->BeginDictionary("web_view_schedulers");
for (WebViewSchedulerImpl* web_view_scheduler : for (WebViewSchedulerImpl* web_view_scheduler :
......
...@@ -117,6 +117,8 @@ class PLATFORM_EXPORT RendererSchedulerImpl ...@@ -117,6 +117,8 @@ class PLATFORM_EXPORT RendererSchedulerImpl
void Shutdown() override; void Shutdown() override;
void SuspendTimerQueue() override; void SuspendTimerQueue() override;
void ResumeTimerQueue() override; void ResumeTimerQueue() override;
void VirtualTimePaused() override;
void VirtualTimeResumed() override;
void SetTimerQueueSuspensionWhenBackgroundedEnabled(bool enabled) override; void SetTimerQueueSuspensionWhenBackgroundedEnabled(bool enabled) override;
void SetTopLevelBlameContext( void SetTopLevelBlameContext(
base::trace_event::BlameContext* blame_context) override; base::trace_event::BlameContext* blame_context) override;
...@@ -492,6 +494,7 @@ class PLATFORM_EXPORT RendererSchedulerImpl ...@@ -492,6 +494,7 @@ class PLATFORM_EXPORT RendererSchedulerImpl
bool in_idle_period_for_testing; bool in_idle_period_for_testing;
bool use_virtual_time; bool use_virtual_time;
bool is_audio_playing; bool is_audio_playing;
bool virtual_time_paused;
std::set<WebViewSchedulerImpl*> web_view_schedulers; // Not owned. std::set<WebViewSchedulerImpl*> web_view_schedulers; // Not owned.
RAILModeObserver* rail_mode_observer; // Not owned. RAILModeObserver* rail_mode_observer; // Not owned.
WakeUpBudgetPool* wake_up_budget_pool; // Not owned. WakeUpBudgetPool* wake_up_budget_pool; // Not owned.
......
...@@ -100,7 +100,7 @@ WebViewSchedulerImpl::WebViewSchedulerImpl( ...@@ -100,7 +100,7 @@ WebViewSchedulerImpl::WebViewSchedulerImpl(
should_throttle_frames_(false), should_throttle_frames_(false),
disable_background_timer_throttling_(disable_background_timer_throttling), disable_background_timer_throttling_(disable_background_timer_throttling),
allow_virtual_time_to_advance_(true), allow_virtual_time_to_advance_(true),
timers_suspended_(false), virtual_time_paused_(false),
have_seen_loading_task_(false), have_seen_loading_task_(false),
virtual_time_(false), virtual_time_(false),
is_audio_playing_(false), is_audio_playing_(false),
...@@ -189,15 +189,17 @@ void WebViewSchedulerImpl::DisableVirtualTimeForTesting() { ...@@ -189,15 +189,17 @@ void WebViewSchedulerImpl::DisableVirtualTimeForTesting() {
} }
void WebViewSchedulerImpl::ApplyVirtualTimePolicyToTimers() { void WebViewSchedulerImpl::ApplyVirtualTimePolicyToTimers() {
if (!virtual_time_ || allow_virtual_time_to_advance_) { bool virtual_time_should_be_paused =
if (timers_suspended_) { virtual_time_ && !allow_virtual_time_to_advance_;
renderer_scheduler_->ResumeTimerQueue(); if (virtual_time_should_be_paused == virtual_time_paused_)
timers_suspended_ = false; return;
}
} else if (!timers_suspended_) { if (virtual_time_should_be_paused) {
renderer_scheduler_->SuspendTimerQueue(); renderer_scheduler_->VirtualTimePaused();
timers_suspended_ = true; } else {
renderer_scheduler_->VirtualTimeResumed();
} }
virtual_time_paused_ = virtual_time_should_be_paused;
} }
void WebViewSchedulerImpl::SetAllowVirtualTimeToAdvance( void WebViewSchedulerImpl::SetAllowVirtualTimeToAdvance(
......
...@@ -112,7 +112,7 @@ class PLATFORM_EXPORT WebViewSchedulerImpl : public WebViewScheduler { ...@@ -112,7 +112,7 @@ class PLATFORM_EXPORT WebViewSchedulerImpl : public WebViewScheduler {
bool should_throttle_frames_; bool should_throttle_frames_;
bool disable_background_timer_throttling_; bool disable_background_timer_throttling_;
bool allow_virtual_time_to_advance_; bool allow_virtual_time_to_advance_;
bool timers_suspended_; bool virtual_time_paused_;
bool have_seen_loading_task_; bool have_seen_loading_task_;
bool virtual_time_; bool virtual_time_;
bool is_audio_playing_; bool is_audio_playing_;
......
...@@ -614,13 +614,13 @@ TEST_F(WebViewSchedulerImplTest, SuspendTimersWhileVirtualTimeIsPaused) { ...@@ -614,13 +614,13 @@ TEST_F(WebViewSchedulerImplTest, SuspendTimersWhileVirtualTimeIsPaused) {
std::unique_ptr<WebFrameSchedulerImpl> web_frame_scheduler = std::unique_ptr<WebFrameSchedulerImpl> web_frame_scheduler =
web_view_scheduler_->CreateWebFrameSchedulerImpl(nullptr); web_view_scheduler_->CreateWebFrameSchedulerImpl(nullptr);
web_frame_scheduler->TimerTaskRunner()->PostDelayedTask(
BLINK_FROM_HERE, WTF::Bind(&RunOrderTask, 1, WTF::Unretained(&run_order)),
TimeDelta());
web_view_scheduler_->SetVirtualTimePolicy(VirtualTimePolicy::PAUSE); web_view_scheduler_->SetVirtualTimePolicy(VirtualTimePolicy::PAUSE);
web_view_scheduler_->EnableVirtualTime(); web_view_scheduler_->EnableVirtualTime();
web_frame_scheduler->TimerTaskRunner()->PostTask(
BLINK_FROM_HERE,
WTF::Bind(&RunOrderTask, 1, WTF::Unretained(&run_order)));
mock_task_runner_->RunUntilIdle(); mock_task_runner_->RunUntilIdle();
EXPECT_TRUE(run_order.empty()); EXPECT_TRUE(run_order.empty());
...@@ -681,9 +681,11 @@ TEST_F(WebViewSchedulerImplTest, VirtualTimeBudgetExhaustedCallback) { ...@@ -681,9 +681,11 @@ TEST_F(WebViewSchedulerImplTest, VirtualTimeBudgetExhaustedCallback) {
// The timer that is scheduled for the exact point in time when virtual time // The timer that is scheduled for the exact point in time when virtual time
// expires will not run. // expires will not run.
EXPECT_THAT(real_times, ElementsAre(initial_real_time, initial_real_time)); EXPECT_THAT(real_times, ElementsAre(initial_real_time, initial_real_time,
initial_real_time));
EXPECT_THAT(virtual_times_ms, ElementsAre(initial_virtual_time_ms + 1, EXPECT_THAT(virtual_times_ms, ElementsAre(initial_virtual_time_ms + 1,
initial_virtual_time_ms + 2)); initial_virtual_time_ms + 2,
initial_virtual_time_ms + 5));
} }
namespace { namespace {
......
...@@ -106,6 +106,10 @@ void FakeRendererScheduler::SuspendTimerQueue() {} ...@@ -106,6 +106,10 @@ void FakeRendererScheduler::SuspendTimerQueue() {}
void FakeRendererScheduler::ResumeTimerQueue() {} void FakeRendererScheduler::ResumeTimerQueue() {}
void FakeRendererScheduler::VirtualTimePaused() {}
void FakeRendererScheduler::VirtualTimeResumed() {}
void FakeRendererScheduler::SetTimerQueueSuspensionWhenBackgroundedEnabled( void FakeRendererScheduler::SetTimerQueueSuspensionWhenBackgroundedEnabled(
bool enabled) {} bool enabled) {}
......
...@@ -208,10 +208,11 @@ TEST_F(VirtualTimeTest, ...@@ -208,10 +208,11 @@ TEST_F(VirtualTimeTest,
TEST_F(VirtualTimeTest, MAYBE_DOMTimersSuspended) { TEST_F(VirtualTimeTest, MAYBE_DOMTimersSuspended) {
WebView().Scheduler()->EnableVirtualTime(); WebView().Scheduler()->EnableVirtualTime();
// Schedule a normal DOM timer to run at 1s in the future. // Schedule normal DOM timers to run at 1s and 1.001s in the future.
ExecuteJavaScript( ExecuteJavaScript(
"var run_order = [];" "var run_order = [];"
"setTimeout(() => { run_order.push(1); }, 1000);"); "setTimeout(() => { run_order.push(1); }, 1000);"
"setTimeout(() => { run_order.push(2); }, 1001);");
RefPtr<WebTaskRunner> runner = RefPtr<WebTaskRunner> runner =
TaskRunnerHelper::Get(TaskType::kTimer, Window().GetExecutionContext()); TaskRunnerHelper::Get(TaskType::kTimer, Window().GetExecutionContext());
...@@ -226,13 +227,13 @@ TEST_F(VirtualTimeTest, MAYBE_DOMTimersSuspended) { ...@@ -226,13 +227,13 @@ TEST_F(VirtualTimeTest, MAYBE_DOMTimersSuspended) {
WTF::Unretained(WebView().Scheduler())), WTF::Unretained(WebView().Scheduler())),
TimeDelta::FromMilliseconds(1000)); TimeDelta::FromMilliseconds(1000));
// ALso schedule a second timer for the same point in time. // ALso schedule a third timer for the same point in time.
ExecuteJavaScript("setTimeout(() => { run_order.push(2); }, 1000);"); ExecuteJavaScript("setTimeout(() => { run_order.push(2); }, 1000);");
// The second DOM timer shouldn't have run because pausing virtual time also // The second DOM timer shouldn't have run because the virtual time budget
// atomically pauses DOM timers. // expired.
testing::RunPendingTasks(); testing::RunPendingTasks();
EXPECT_EQ("1", ExecuteJavaScript("run_order.join(', ')")); EXPECT_EQ("1, 2", ExecuteJavaScript("run_order.join(', ')"));
} }
} // namespace blink } // namespace blink
...@@ -159,14 +159,24 @@ class BLINK_PLATFORM_EXPORT RendererScheduler : public ChildScheduler { ...@@ -159,14 +159,24 @@ class BLINK_PLATFORM_EXPORT RendererScheduler : public ChildScheduler {
// Must be called from the main thread. // Must be called from the main thread.
virtual bool IsHighPriorityWorkAnticipated() = 0; virtual bool IsHighPriorityWorkAnticipated() = 0;
// Suspends the timer queue and increments the timer queue suspension count. // Suspends the timer queues and increments the timer queue suspension count.
// May only be called from the main thread. // May only be called from the main thread.
virtual void SuspendTimerQueue() = 0; virtual void SuspendTimerQueue() = 0;
// Decrements the timer queue suspension count and re-enables the timer queue // Decrements the timer queue suspension count and re-enables the timer queues
// if the suspension count is zero and the current schduler policy allows it. // if the suspension count is zero and the current schduler policy allows it.
virtual void ResumeTimerQueue() = 0; virtual void ResumeTimerQueue() = 0;
// Suspends the timer queues by inserting a fence that blocks any tasks posted
// after this point from running. Orthogonal to SuspendTimerQueue. Care must
// be taken when using this API to avoid fighting with the TaskQueueThrottler.
virtual void VirtualTimePaused() = 0;
// Removes the fence added by VirtualTimePaused allowing timers to execute
// normally. Care must be taken when using this API to avoid fighting with the
// TaskQueueThrottler.
virtual void VirtualTimeResumed() = 0;
// Sets whether to allow suspension of timers after the backgrounded signal is // Sets whether to allow suspension of timers after the backgrounded signal is
// received via OnRendererBackgrounded. Defaults to disabled. // received via OnRendererBackgrounded. Defaults to disabled.
virtual void SetTimerQueueSuspensionWhenBackgroundedEnabled(bool enabled) = 0; virtual void SetTimerQueueSuspensionWhenBackgroundedEnabled(bool enabled) = 0;
......
...@@ -53,6 +53,8 @@ class FakeRendererScheduler : public RendererScheduler { ...@@ -53,6 +53,8 @@ class FakeRendererScheduler : public RendererScheduler {
void Shutdown() override; void Shutdown() override;
void SuspendTimerQueue() override; void SuspendTimerQueue() override;
void ResumeTimerQueue() override; void ResumeTimerQueue() override;
void VirtualTimePaused() override;
void VirtualTimeResumed() override;
void SetTimerQueueSuspensionWhenBackgroundedEnabled(bool enabled) override; void SetTimerQueueSuspensionWhenBackgroundedEnabled(bool enabled) override;
void SetTopLevelBlameContext( void SetTopLevelBlameContext(
base::trace_event::BlameContext* blame_context) override; base::trace_event::BlameContext* blame_context) override;
......
...@@ -57,6 +57,8 @@ class MockRendererScheduler : public RendererScheduler { ...@@ -57,6 +57,8 @@ class MockRendererScheduler : public RendererScheduler {
MOCK_METHOD0(Shutdown, void()); MOCK_METHOD0(Shutdown, void());
MOCK_METHOD0(SuspendTimerQueue, void()); MOCK_METHOD0(SuspendTimerQueue, void());
MOCK_METHOD0(ResumeTimerQueue, void()); MOCK_METHOD0(ResumeTimerQueue, void());
MOCK_METHOD0(VirtualTimePaused, void());
MOCK_METHOD0(VirtualTimeResumed, void());
MOCK_METHOD1(SetTimerQueueSuspensionWhenBackgroundedEnabled, void(bool)); MOCK_METHOD1(SetTimerQueueSuspensionWhenBackgroundedEnabled, void(bool));
MOCK_METHOD1(SetTopLevelBlameContext, void(base::trace_event::BlameContext*)); MOCK_METHOD1(SetTopLevelBlameContext, void(base::trace_event::BlameContext*));
MOCK_METHOD1(SetRAILModeObserver, void(RAILModeObserver*)); MOCK_METHOD1(SetRAILModeObserver, void(RAILModeObserver*));
......
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