Commit 1329d0ac authored by skyostil's avatar skyostil Committed by Commit bot

scheduler: Reset gesture state at start of a mouse drag

This patch fixes a bug where the scheduler did not reset gesture
tracking state when a new mouse drag gesture was started. This meant
that an unrelated gesture (e.g., a wheel scroll) could be used to
compute the policy for the mouse drag, potentially causing tasks
necessary to the gesture (e.g., timers) to get deferred.

We now consider a left button press as the start of a new gesture and
also keep track of which thread is processing the mouse move events
as we do with touch gestures.

BUG=638212

Review-Url: https://codereview.chromium.org/2266883002
Cr-Commit-Position: refs/heads/master@{#413475}
parent f3d1a581
......@@ -468,7 +468,8 @@ bool RendererSchedulerImpl::ShouldPrioritizeInputEvent(
const blink::WebInputEvent& web_input_event) {
// We regard MouseMove events with the left mouse button down as a signal
// that the user is doing something requiring a smooth frame rate.
if (web_input_event.type == blink::WebInputEvent::MouseMove &&
if ((web_input_event.type == blink::WebInputEvent::MouseDown ||
web_input_event.type == blink::WebInputEvent::MouseMove) &&
(web_input_event.modifiers & blink::WebInputEvent::LeftButtonDown)) {
return true;
}
......@@ -528,58 +529,73 @@ void RendererSchedulerImpl::UpdateForInputEventOnCompositorThread(
if (input_event_state == InputEventState::EVENT_CONSUMED_BY_COMPOSITOR)
AnyThread().user_model.DidFinishProcessingInputEvent(now);
if (type) {
switch (type) {
case blink::WebInputEvent::TouchStart:
AnyThread().awaiting_touch_start_response = true;
// This is just a fail-safe to reset the state of
// |last_gesture_was_compositor_driven| to the default. We don't know
// yet where the gesture will run.
AnyThread().last_gesture_was_compositor_driven = false;
AnyThread().have_seen_touchstart = true;
// Assume the default gesture is prevented until we see evidence
// otherwise.
AnyThread().default_gesture_prevented = true;
break;
case blink::WebInputEvent::TouchMove:
// Observation of consecutive touchmoves is a strong signal that the
// page is consuming the touch sequence, in which case touchstart
// response prioritization is no longer necessary. Otherwise, the
// initial touchmove should preserve the touchstart response pending
// state.
if (AnyThread().awaiting_touch_start_response &&
CompositorThreadOnly().last_input_type ==
blink::WebInputEvent::TouchMove) {
AnyThread().awaiting_touch_start_response = false;
}
break;
case blink::WebInputEvent::GesturePinchUpdate:
case blink::WebInputEvent::GestureScrollUpdate:
// If we see events for an established gesture, we can lock it to the
// appropriate thread as the gesture can no longer be cancelled.
AnyThread().last_gesture_was_compositor_driven =
input_event_state == InputEventState::EVENT_CONSUMED_BY_COMPOSITOR;
switch (type) {
case blink::WebInputEvent::TouchStart:
AnyThread().awaiting_touch_start_response = true;
// This is just a fail-safe to reset the state of
// |last_gesture_was_compositor_driven| to the default. We don't know
// yet where the gesture will run.
AnyThread().last_gesture_was_compositor_driven = false;
AnyThread().have_seen_touchstart = true;
// Assume the default gesture is prevented until we see evidence
// otherwise.
AnyThread().default_gesture_prevented = true;
break;
case blink::WebInputEvent::TouchMove:
// Observation of consecutive touchmoves is a strong signal that the
// page is consuming the touch sequence, in which case touchstart
// response prioritization is no longer necessary. Otherwise, the
// initial touchmove should preserve the touchstart response pending
// state.
if (AnyThread().awaiting_touch_start_response &&
CompositorThreadOnly().last_input_type ==
blink::WebInputEvent::TouchMove) {
AnyThread().awaiting_touch_start_response = false;
AnyThread().default_gesture_prevented = false;
break;
}
break;
case blink::WebInputEvent::GesturePinchUpdate:
case blink::WebInputEvent::GestureScrollUpdate:
// If we see events for an established gesture, we can lock it to the
// appropriate thread as the gesture can no longer be cancelled.
AnyThread().last_gesture_was_compositor_driven =
input_event_state == InputEventState::EVENT_CONSUMED_BY_COMPOSITOR;
AnyThread().awaiting_touch_start_response = false;
AnyThread().default_gesture_prevented = false;
break;
case blink::WebInputEvent::GestureFlingCancel:
AnyThread().fling_compositor_escalation_deadline = base::TimeTicks();
break;
case blink::WebInputEvent::GestureFlingCancel:
AnyThread().fling_compositor_escalation_deadline = base::TimeTicks();
break;
case blink::WebInputEvent::GestureTapDown:
case blink::WebInputEvent::GestureShowPress:
case blink::WebInputEvent::GestureScrollEnd:
// With no observable effect, these meta events do not indicate a
// meaningful touchstart response and should not impact task priority.
break;
case blink::WebInputEvent::GestureTapDown:
case blink::WebInputEvent::GestureShowPress:
case blink::WebInputEvent::GestureScrollEnd:
// With no observable effect, these meta events do not indicate a
// meaningful touchstart response and should not impact task priority.
break;
default:
AnyThread().awaiting_touch_start_response = false;
break;
}
case blink::WebInputEvent::MouseDown:
// Reset tracking state at the start of a new mouse drag gesture.
AnyThread().last_gesture_was_compositor_driven = false;
AnyThread().default_gesture_prevented = true;
break;
case blink::WebInputEvent::MouseMove:
// Consider mouse movement with the left button held down (see
// ShouldPrioritizeInputEvent) similarly to a touch gesture.
AnyThread().last_gesture_was_compositor_driven =
input_event_state == InputEventState::EVENT_CONSUMED_BY_COMPOSITOR;
AnyThread().awaiting_touch_start_response = false;
break;
case blink::WebInputEvent::Undefined:
break;
default:
AnyThread().awaiting_touch_start_response = false;
break;
}
// Avoid unnecessary policy updates if the use case did not change.
......@@ -1019,8 +1035,6 @@ RendererSchedulerImpl::UseCase RendererSchedulerImpl::ComputeCurrentUseCase(
// stream of input events and has prevented a default gesture from being
// started.
// 4. SYNCHRONIZED_GESTURE where the gesture is processed on both threads.
// TODO(skyostil): Consider removing in_idle_period_ and
// HadAnIdlePeriodRecently() unless we need them here.
if (AnyThread().last_gesture_was_compositor_driven) {
if (AnyThread().begin_main_frame_on_critical_path) {
return UseCase::SYNCHRONIZED_GESTURE;
......@@ -1281,12 +1295,6 @@ void RendererSchedulerImpl::OnNavigationStarted() {
ResetForNavigationLocked();
}
bool RendererSchedulerImpl::HadAnIdlePeriodRecently(base::TimeTicks now) const {
return (now - AnyThread().last_idle_period_end_time) <=
base::TimeDelta::FromMilliseconds(
kIdlePeriodStarvationThresholdMillis);
}
void RendererSchedulerImpl::SuspendTimerQueueWhenBackgrounded() {
DCHECK(MainThreadOnly().renderer_backgrounded);
if (MainThreadOnly().timer_queue_suspended_when_backgrounded)
......
......@@ -265,13 +265,6 @@ class BLINK_PLATFORM_EXPORT RendererSchedulerImpl
// defined by RAILS.
static const int kRailsResponseTimeMillis = 50;
// For the purposes of deciding whether or not it's safe to turn timers and
// loading tasks on only in idle periods, we regard the system as being as
// being "idle period" starved if there hasn't been an idle period in the last
// 10 seconds. This was chosen to be long enough to cover most anticipated
// user gestures.
static const int kIdlePeriodStarvationThresholdMillis = 10000;
// The amount of time to wait before suspending shared timers after the
// renderer has been backgrounded. This is used only if background suspension
// of shared timers is enabled.
......@@ -320,10 +313,6 @@ class BLINK_PLATFORM_EXPORT RendererSchedulerImpl
void UpdateForInputEventOnCompositorThread(WebInputEvent::Type type,
InputEventState input_event_state);
// Returns true if there has been at least one idle period in the last
// |kIdlePeriodStarvationThresholdMillis|.
bool HadAnIdlePeriodRecently(base::TimeTicks now) const;
// Helpers for safely suspending/resuming the timer queue after a
// background/foreground signal.
void SuspendTimerQueueWhenBackgrounded();
......
......@@ -589,11 +589,6 @@ class RendererSchedulerImplTest : public testing::Test {
RendererSchedulerImpl::kEndIdleWhenHiddenDelayMillis);
}
static base::TimeDelta idle_period_starvation_threshold() {
return base::TimeDelta::FromMilliseconds(
RendererSchedulerImpl::kIdlePeriodStarvationThresholdMillis);
}
static base::TimeDelta suspend_timers_when_backgrounded_delay() {
return base::TimeDelta::FromMilliseconds(
RendererSchedulerImpl::kSuspendTimersWhenBackgroundedDelayMillis);
......@@ -1258,17 +1253,21 @@ TEST_F(RendererSchedulerImplTest,
std::vector<std::string> run_order;
PostTestTasks(&run_order, "I1 D1 C1 D2 C2");
// Note that currently the compositor will never consume mouse move events,
// but this test reflects what should happen if that was the case.
EnableIdleTasks();
scheduler_->DidHandleInputEventOnCompositorThread(
FakeInputEvent(blink::WebInputEvent::MouseMove,
blink::WebInputEvent::LeftButtonDown),
RendererScheduler::InputEventState::EVENT_CONSUMED_BY_COMPOSITOR);
RunUntilIdle();
// Note compositor tasks are prioritized.
// Note compositor tasks deprioritized.
EXPECT_EQ(RendererSchedulerImpl::UseCase::COMPOSITOR_GESTURE,
CurrentUseCase());
EXPECT_THAT(run_order,
testing::ElementsAre(std::string("C1"), std::string("C2"),
std::string("D1"), std::string("D2"),
std::string("I1")));
testing::ElementsAre(std::string("D1"), std::string("D2"),
std::string("I1"), std::string("C1"),
std::string("C2")));
}
TEST_F(RendererSchedulerImplTest,
......@@ -1291,6 +1290,71 @@ TEST_F(RendererSchedulerImplTest,
blink::WebInputEvent::MouseMove, blink::WebInputEvent::LeftButtonDown));
}
TEST_F(RendererSchedulerImplTest,
EventForwardedToMainThread_MouseMove_WhenMouseDown_AfterMouseWheel) {
// Simulate a main thread driven mouse wheel scroll gesture.
SimulateMainThreadGestureStart(TouchEventPolicy::SEND_TOUCH_START,
blink::WebInputEvent::GestureScrollUpdate);
RunUntilIdle();
EXPECT_FALSE(TouchStartExpectedSoon());
EXPECT_EQ(RendererSchedulerImpl::UseCase::MAIN_THREAD_GESTURE,
CurrentUseCase());
// Now start a main thread mouse touch gesture. It should be detected as main
// thread custom input handling.
std::vector<std::string> run_order;
PostTestTasks(&run_order, "I1 D1 C1 D2 C2");
EnableIdleTasks();
scheduler_->DidHandleInputEventOnCompositorThread(
FakeInputEvent(blink::WebInputEvent::MouseDown,
blink::WebInputEvent::LeftButtonDown),
RendererScheduler::InputEventState::EVENT_FORWARDED_TO_MAIN_THREAD);
scheduler_->DidHandleInputEventOnCompositorThread(
FakeInputEvent(blink::WebInputEvent::MouseMove,
blink::WebInputEvent::LeftButtonDown),
RendererScheduler::InputEventState::EVENT_FORWARDED_TO_MAIN_THREAD);
RunUntilIdle();
EXPECT_EQ(RendererSchedulerImpl::UseCase::MAIN_THREAD_CUSTOM_INPUT_HANDLING,
CurrentUseCase());
// Note compositor tasks are prioritized.
EXPECT_THAT(run_order,
testing::ElementsAre(std::string("C1"), std::string("C2"),
std::string("D1"), std::string("D2"),
std::string("I1")));
}
TEST_F(RendererSchedulerImplTest,
EventForwardedToMainThread_MouseClick) {
// A mouse click should be detected as main thread input handling, which means
// we won't try to defer expensive tasks because of one. We can, however,
// prioritize compositing/input handling.
std::vector<std::string> run_order;
PostTestTasks(&run_order, "I1 D1 C1 D2 C2");
EnableIdleTasks();
scheduler_->DidHandleInputEventOnCompositorThread(
FakeInputEvent(blink::WebInputEvent::MouseDown,
blink::WebInputEvent::LeftButtonDown),
RendererScheduler::InputEventState::EVENT_FORWARDED_TO_MAIN_THREAD);
scheduler_->DidHandleInputEventOnCompositorThread(
FakeInputEvent(blink::WebInputEvent::MouseUp,
blink::WebInputEvent::LeftButtonDown),
RendererScheduler::InputEventState::EVENT_FORWARDED_TO_MAIN_THREAD);
RunUntilIdle();
EXPECT_EQ(RendererSchedulerImpl::UseCase::MAIN_THREAD_CUSTOM_INPUT_HANDLING,
CurrentUseCase());
// Note compositor tasks are prioritized.
EXPECT_THAT(run_order,
testing::ElementsAre(std::string("C1"), std::string("C2"),
std::string("D1"), std::string("D2"),
std::string("I1")));
}
TEST_F(RendererSchedulerImplTest, EventConsumedOnCompositorThread_MouseWheel) {
std::vector<std::string> run_order;
PostTestTasks(&run_order, "I1 D1 C1 D2 C2");
......
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