Commit 1bfa23fd authored by Alexander Timin's avatar Alexander Timin Committed by Commit Bot

[scheduler] Issue RequestBeginMainFrameNotExpected call asynchronously.

Post a control task to call RequestBeginMainFrameNotExpected
asynchronously to avoid calling it when frame tree is dirty.

R=yutak@chromium.org,dcheng@chromium.org,haraken@chromium.org
BUG=820893

Change-Id: I375aa46180f20f053c13563df910d3813c22938f
Reviewed-on: https://chromium-review.googlesource.com/962787Reviewed-by: default avatarYuta Kitamura <yutak@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543683}
parent 1efc2094
......@@ -802,15 +802,6 @@ void Page::RequestBeginMainFrameNotExpected(bool new_state) {
return;
if (LocalFrame* main_frame = DeprecatedLocalMainFrame()) {
if (!main_frame->Client()) {
// This can happen while swapping out web frames because the scheduler can
// be invoked in the middle of Oilpan allocation.
// TODO(https://crbug.com/820893): Figure out how to avoid this
// complicated corner case. As written, it's not clear PageScheduler
// does this correctly for OOPIF anyway since it only talks to the root
// WebLayerTreeView, but a page with OOPIFs also has local roots.
return;
}
if (WebLayerTreeView* layer_tree_view =
chrome_client_->GetWebLayerTreeView(main_frame)) {
layer_tree_view->RequestBeginMainFrameNotExpected(new_state);
......
......@@ -2228,11 +2228,29 @@ void RendererSchedulerImpl::OnPendingTasksChanged(bool has_tasks) {
main_thread_only().compositor_will_send_main_frame_not_expected.get())
return;
// Dispatch RequestBeginMainFrameNotExpectedSoon notifications asynchronously.
// This is needed because idle task can be posted (and OnPendingTasksChanged
// called) at any moment, including in the middle of allocating an object,
// when state is not consistent. Posting a task to dispatch notifications
// minimizes the amount of code that runs and sees an inconsistent state .
control_task_queue_->PostTask(
FROM_HERE,
base::BindOnce(
&RendererSchedulerImpl::DispatchRequestBeginMainFrameNotExpected,
weak_factory_.GetWeakPtr(), has_tasks));
}
void RendererSchedulerImpl::DispatchRequestBeginMainFrameNotExpected(
bool has_tasks) {
if (has_tasks ==
main_thread_only().compositor_will_send_main_frame_not_expected.get())
return;
main_thread_only().compositor_will_send_main_frame_not_expected = has_tasks;
TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"),
"RendererSchedulerImpl::OnPendingTasksChanged", "has_tasks",
has_tasks);
TRACE_EVENT1(
TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"),
"RendererSchedulerImpl::DispatchRequestBeginMainFrameNotExpected",
"has_tasks", has_tasks);
for (PageSchedulerImpl* page_scheduler : main_thread_only().page_schedulers) {
page_scheduler->RequestBeginMainFrameNotExpected(has_tasks);
}
......
......@@ -484,6 +484,7 @@ class PLATFORM_EXPORT RendererSchedulerImpl
void OnIdlePeriodEnded() override;
void OnPendingTasksChanged(bool has_tasks) override;
void DispatchRequestBeginMainFrameNotExpected(bool has_tasks);
void EndIdlePeriod();
......
......@@ -34,6 +34,8 @@ namespace scheduler {
// To avoid symbol collisions in jumbo builds.
namespace renderer_scheduler_impl_unittest {
using ::testing::Mock;
class FakeInputEvent : public blink::WebInputEvent {
public:
explicit FakeInputEvent(blink::WebInputEvent::Type event_type,
......@@ -207,9 +209,10 @@ class ScopedAutoAdvanceNowEnabler {
class RendererSchedulerImplForTest : public RendererSchedulerImpl {
public:
using RendererSchedulerImpl::EstimateLongestJankFreeTaskDuration;
using RendererSchedulerImpl::OnIdlePeriodEnded;
using RendererSchedulerImpl::OnIdlePeriodStarted;
using RendererSchedulerImpl::EstimateLongestJankFreeTaskDuration;
using RendererSchedulerImpl::OnPendingTasksChanged;
RendererSchedulerImplForTest(std::unique_ptr<TaskQueueManager> manager,
base::Optional<base::Time> initial_virtual_time)
......@@ -3258,6 +3261,8 @@ class PageSchedulerImplForTest : public PageSchedulerImpl {
return interventions_;
}
MOCK_METHOD1(RequestBeginMainFrameNotExpected, void(bool));
private:
std::vector<std::string> interventions_;
......@@ -4161,6 +4166,40 @@ TEST_F(RendererSchedulerImplTest, LoadingControlTasks) {
std::string("L5"), std::string("L6")));
}
TEST_F(RendererSchedulerImplTest, RequestBeginMainFrameNotExpected) {
std::unique_ptr<PageSchedulerImplForTest> page_scheduler =
std::make_unique<PageSchedulerImplForTest>(scheduler_.get());
scheduler_->AddPageScheduler(page_scheduler.get());
scheduler_->OnPendingTasksChanged(true);
EXPECT_CALL(*page_scheduler, RequestBeginMainFrameNotExpected(true)).Times(1);
RunUntilIdle();
Mock::VerifyAndClearExpectations(page_scheduler.get());
scheduler_->OnPendingTasksChanged(false);
EXPECT_CALL(*page_scheduler, RequestBeginMainFrameNotExpected(false))
.Times(1);
RunUntilIdle();
Mock::VerifyAndClearExpectations(page_scheduler.get());
}
TEST_F(RendererSchedulerImplTest,
RequestBeginMainFrameNotExpected_MultipleCalls) {
std::unique_ptr<PageSchedulerImplForTest> page_scheduler =
std::make_unique<PageSchedulerImplForTest>(scheduler_.get());
scheduler_->AddPageScheduler(page_scheduler.get());
scheduler_->OnPendingTasksChanged(true);
scheduler_->OnPendingTasksChanged(true);
// Multiple calls should result in only one call.
EXPECT_CALL(*page_scheduler, RequestBeginMainFrameNotExpected(true)).Times(1);
RunUntilIdle();
Mock::VerifyAndClearExpectations(page_scheduler.get());
}
#if defined(OS_ANDROID)
TEST_F(RendererSchedulerImplTest, PauseTimersForAndroidWebView) {
ScopedAutoAdvanceNowEnabler enable_auto_advance_now(mock_task_runner_);
......
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