Commit de7ce712 authored by Andrey Kosyakov's avatar Andrey Kosyakov Committed by Commit Bot

cc: fix a deadlock in scheduler state machine w/wait_for_all_pipeline_stages_before_draw

- we rasterize and activate a tree with active_tree_needs_first_draw;
- we don't draw the tree because we're not within the frame deadline [1]
- a commit happens meanwhile, which resets active_tree_is_ready_to_draw_ [2]

Now we can't draw, because we have a pending tree [3] and we can't
activate the pending tree, because active_tree_needs_first_draw [4];
So we don't produce a frame.
The next attempt to commit hangs the main thread, as it waits on the completion event [5].

1: https://source.chromium.org/chromium/chromium/src/+/master:cc/scheduler/scheduler_state_machine.cc;drc=51f3bdaec86288604ea075cf6e445e7ab3cbcc88;l=387
2: https://source.chromium.org/chromium/chromium/src/+/master:cc/scheduler/scheduler_state_machine.cc;drc=51f3bdaec86288604ea075cf6e445e7ab3cbcc88;l=877
3: https://source.chromium.org/chromium/chromium/src/+/master:cc/scheduler/scheduler_state_machine.cc;drc=51f3bdaec86288604ea075cf6e445e7ab3cbcc88;l=393
4: https://source.chromium.org/chromium/chromium/src/+/master:cc/scheduler/scheduler_state_machine.cc;drc=51f3bdaec86288604ea075cf6e445e7ab3cbcc88;l=420
5: https://source.chromium.org/chromium/chromium/src/+/master:cc/trees/proxy_main.cc;drc=cd2804d00d64031d9fd6397f6e9bc6093a2b2ef8;l=370

Bug: b/160032351
Change-Id: Ib89a513e2ab277742cea6c64ef14f945710c851a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2301202
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: default avatarEric Seckler <eseckler@chromium.org>
Reviewed-by: default avatarSunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789745}
parent 4df4669a
......@@ -872,9 +872,13 @@ void SchedulerStateMachine::WillCommit(bool commit_has_no_updates) {
has_pending_tree_ = true;
pending_tree_needs_first_draw_on_activation_ = true;
pending_tree_is_ready_for_activation_ = false;
// Wait for the new pending tree to become ready to draw, which may happen
// before or after activation.
active_tree_is_ready_to_draw_ = false;
if (!active_tree_needs_first_draw_ ||
!settings_.wait_for_all_pipeline_stages_before_draw) {
// Wait for the new pending tree to become ready to draw, which may happen
// before or after activation (unless we're in full-pipeline mode and
// need first draw to come through).
active_tree_is_ready_to_draw_ = false;
}
}
// Update state related to forced draws.
......
......@@ -11,6 +11,7 @@
#include "cc/scheduler/scheduler.h"
#include "components/viz/common/frame_sinks/begin_frame_args.h"
#include "components/viz/test/begin_frame_args_test.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
// Macro to compare two enum values and get nice output.
......@@ -21,7 +22,7 @@
// Value of: actual() Actual: "ACTION_DRAW"
// Expected: expected() Which is: "ACTION_NONE"
#define EXPECT_ENUM_EQ(enum_tostring, expected, actual) \
EXPECT_EQ(enum_tostring(expected), enum_tostring(actual))
EXPECT_THAT(enum_tostring(actual), testing::Eq(enum_tostring(expected)))
#define EXPECT_IMPL_FRAME_STATE(expected) \
EXPECT_ENUM_EQ(BeginImplFrameStateToString, expected, \
......@@ -2900,5 +2901,50 @@ TEST(SchedulerStateMachineTest,
EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::ACTIVATE_SYNC_TREE);
}
TEST(SchedulerStateMachineTest, TestFullPipelineModeDoesntBlockAfterCommit) {
SchedulerSettings settings;
settings.wait_for_all_pipeline_stages_before_draw = true;
StateMachine state(settings);
SET_UP_STATE(state);
const bool needs_first_draw_on_activation = true;
state.SetNeedsImplSideInvalidation(needs_first_draw_on_activation);
state.SetNeedsBeginMainFrame();
state.SetNeedsRedraw(true);
viz::BeginFrameId frame_id = viz::BeginFrameId(0, 10);
state.OnBeginImplFrame(frame_id, kAnimateOnly);
EXPECT_ACTION_UPDATE_STATE(
SchedulerStateMachine::Action::SEND_BEGIN_MAIN_FRAME);
state.NotifyReadyToCommit();
EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::COMMIT);
EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::NONE);
state.NotifyReadyToActivate();
EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::ACTIVATE_SYNC_TREE);
state.NotifyReadyToDraw();
EXPECT_TRUE(state.active_tree_needs_first_draw());
EXPECT_IMPL_FRAME_STATE(
SchedulerStateMachine::BeginImplFrameState::INSIDE_BEGIN_FRAME);
// Go all the way until ready to draw, but make sure we're not within
// the frame deadline, so actual draw doesn't happen...
EXPECT_FALSE(state.ShouldDraw());
// ... then have another commit ...
state.SetNeedsBeginMainFrame();
frame_id.sequence_number++;
state.OnBeginImplFrame(frame_id, kAnimateOnly);
EXPECT_ACTION_UPDATE_STATE(
SchedulerStateMachine::Action::SEND_BEGIN_MAIN_FRAME);
state.NotifyReadyToCommit();
EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::COMMIT);
// ... and make sure we're in a state where we can proceed,
// rather than draw being blocked by the pending tree.
state.OnBeginImplFrameDeadline();
EXPECT_TRUE(state.ShouldDraw());
EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::Action::DRAW_IF_POSSIBLE);
}
} // namespace
} // namespace cc
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