Skip to content

Commit

Permalink
Revert of Remove enable/disable wheel gestures setting. (patchset chr…
Browse files Browse the repository at this point in the history
…omium#3 id:40001 of https://codereview.chromium.org/2047093002/ )

Reason for revert:
Caused RenderWidgetHostViewMacTest.IgnoreEmptyUnhandledWheelEventWithWheelGestures failure on build:

https://build.chromium.org/p/chromium.mac/builders/Mac10.9%20Tests%20(dbg)/builds/25325

See
https://findit-for-me.appspot.com/build-failure?url=https://build.chromium.org/p/chromium.mac/builders/Mac10.9%20Tests%20(dbg)/builds/25325

Test output:
"""
[ RUN      ] RenderWidgetHostViewMacTest.IgnoreEmptyUnhandledWheelEventWithWheelGestures
../../content/browser/renderer_host/render_widget_host_view_mac_unittest.mm:966: Failure
Value of: view_delegate.get().unhandledWheelEventReceived
  Actual: ''
Expected: __objc_yes
Which is: '?' (1)
[3372:1287:0608/164121:1128775946716:FATAL:surface_manager.cc(43)] Check failed: namespace_client_map_.size() == 0u (1 vs. 0)
"""

Original issue's description:
> Remove enable/disable wheel gestures setting.
>
> Since wheel gesture based scrolling has shipped and the code has been
> removed from blink we can remove the runtime setting.
>
> BUG=598798
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/3f5cea527647ebc8c505b1932d680854e162d82f
> Cr-Commit-Position: refs/heads/master@{#398716}

TBR=aelias@chromium.org,rbyers@chromium.org,creis@chromium.org,dtapuska@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=598798

Review-Url: https://codereview.chromium.org/2050033002
Cr-Commit-Position: refs/heads/master@{#398746}
  • Loading branch information
scheib authored and Commit bot committed Jun 9, 2016
1 parent 51f7596 commit 463485d
Show file tree
Hide file tree
Showing 38 changed files with 908 additions and 45 deletions.
3 changes: 2 additions & 1 deletion cc/debug/debug_rect_history.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ void DebugRectHistory::SaveWheelEventHandlerRects(LayerImpl* root_layer) {
root_layer->layer_tree_impl()->event_listener_properties(
EventListenerClass::kMouseWheel);
if (event_properties == EventListenerProperties::kNone ||
event_properties == EventListenerProperties::kPassive) {
(root_layer->layer_tree_impl()->settings().use_mouse_wheel_gestures &&
event_properties == EventListenerProperties::kPassive)) {
return;
}

Expand Down
1 change: 1 addition & 0 deletions cc/proto/layer_tree_settings.proto
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ message LayerTreeSettings {
optional int32 max_staging_buffer_usage_in_bytes = 47;
optional ManagedMemoryPolicy memory_policy = 48;
optional LayerTreeDebugState initial_debug_state = 49;
optional bool use_mouse_wheel_gestures = 50;
optional bool use_cached_picture_raster = 51;
optional bool async_worker_context_enabled = 52;
}
14 changes: 14 additions & 0 deletions cc/trees/layer_tree_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2523,6 +2523,20 @@ InputHandler::ScrollStatus LayerTreeHostImpl::TryScroll(
}
}

if (IsWheelBasedScroll(type) &&
!active_tree()->settings().use_mouse_wheel_gestures) {
EventListenerProperties event_properties =
active_tree()->event_listener_properties(
EventListenerClass::kMouseWheel);
if (event_properties != EventListenerProperties::kNone) {
TRACE_EVENT0("cc", "LayerImpl::tryScroll: Failed WheelEventHandlers");
scroll_status.thread = InputHandler::SCROLL_ON_MAIN_THREAD;
scroll_status.main_thread_scrolling_reasons =
MainThreadScrollingReason::kEventHandlers;
return scroll_status;
}
}

if (!scroll_node->data.scrollable) {
TRACE_EVENT0("cc", "LayerImpl::tryScroll: Ignored not scrollable");
scroll_status.thread = InputHandler::SCROLL_IGNORED;
Expand Down
35 changes: 29 additions & 6 deletions cc/trees/layer_tree_host_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -763,17 +763,40 @@ TEST_F(LayerTreeHostImplTest, ScrollBlocksOnWheelEventHandlers) {
host_impl_->SetViewportSize(gfx::Size(50, 50));
DrawFrame();

// Wheel handlers determine whether mouse events block scroll.
// With registered event handlers, wheel scrolls don't necessarily
// have to go to the main thread.
host_impl_->active_tree()->set_event_listener_properties(
EventListenerClass::kMouseWheel, EventListenerProperties::kBlocking);
EXPECT_EQ(
EventListenerProperties::kBlocking,
host_impl_->GetEventListenerProperties(EventListenerClass::kMouseWheel));

// But they don't influence the actual handling of the scroll gestures.
InputHandler::ScrollStatus status =
host_impl_->ScrollBegin(BeginState(gfx::Point()).get(),
InputHandler::WHEEL);
EXPECT_EQ(InputHandler::SCROLL_ON_MAIN_THREAD, status.thread);
EXPECT_EQ(MainThreadScrollingReason::kEventHandlers,
status.main_thread_scrolling_reasons);

host_impl_->active_tree()->set_event_listener_properties(
EventListenerClass::kMouseWheel,
EventListenerProperties::kBlockingAndPassive);
status = host_impl_->ScrollBegin(BeginState(gfx::Point()).get(),
InputHandler::WHEEL);
EXPECT_EQ(InputHandler::SCROLL_ON_MAIN_THREAD, status.thread);
EXPECT_EQ(MainThreadScrollingReason::kEventHandlers,
status.main_thread_scrolling_reasons);

// But gesture scrolls can still be handled.
status = host_impl_->ScrollBegin(BeginState(gfx::Point()).get(),
InputHandler::TOUCHSCREEN);
EXPECT_EQ(InputHandler::SCROLL_ON_IMPL_THREAD, status.thread);
EXPECT_EQ(MainThreadScrollingReason::kNotScrollingOnMain,
status.main_thread_scrolling_reasons);
host_impl_->ScrollEnd(EndState().get());

// And if the handlers go away, wheel scrolls can again be processed
// on impl.
host_impl_->active_tree()->set_event_listener_properties(
EventListenerClass::kMouseWheel, EventListenerProperties::kNone);
status = host_impl_->ScrollBegin(BeginState(gfx::Point()).get(),
InputHandler::WHEEL);
EXPECT_EQ(InputHandler::SCROLL_ON_IMPL_THREAD, status.thread);
EXPECT_EQ(MainThreadScrollingReason::kNotScrollingOnMain,
status.main_thread_scrolling_reasons);
Expand Down
3 changes: 3 additions & 0 deletions cc/trees/layer_tree_settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ bool LayerTreeSettings::operator==(const LayerTreeSettings& other) const {
verify_clip_tree_calculations == other.verify_clip_tree_calculations &&
image_decode_tasks_enabled == other.image_decode_tasks_enabled &&
wait_for_beginframe_interval == other.wait_for_beginframe_interval &&
use_mouse_wheel_gestures == other.use_mouse_wheel_gestures &&
max_staging_buffer_usage_in_bytes ==
other.max_staging_buffer_usage_in_bytes &&
memory_policy_ == other.memory_policy_ &&
Expand Down Expand Up @@ -175,6 +176,7 @@ void LayerTreeSettings::ToProtobuf(proto::LayerTreeSettings* proto) const {
use_occlusion_for_tile_prioritization);
proto->set_image_decode_tasks_enabled(image_decode_tasks_enabled);
proto->set_wait_for_beginframe_interval(wait_for_beginframe_interval);
proto->set_use_mouse_wheel_gestures(use_mouse_wheel_gestures);
proto->set_max_staging_buffer_usage_in_bytes(
max_staging_buffer_usage_in_bytes);
memory_policy_.ToProtobuf(proto->mutable_memory_policy());
Expand Down Expand Up @@ -238,6 +240,7 @@ void LayerTreeSettings::FromProtobuf(const proto::LayerTreeSettings& proto) {
proto.use_occlusion_for_tile_prioritization();
image_decode_tasks_enabled = proto.image_decode_tasks_enabled();
wait_for_beginframe_interval = proto.wait_for_beginframe_interval();
use_mouse_wheel_gestures = proto.use_mouse_wheel_gestures();
max_staging_buffer_usage_in_bytes = proto.max_staging_buffer_usage_in_bytes();
memory_policy_.FromProtobuf(proto.memory_policy());
initial_debug_state.FromProtobuf(proto.initial_debug_state());
Expand Down
1 change: 1 addition & 0 deletions cc/trees/layer_tree_settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class CC_EXPORT LayerTreeSettings {
bool image_decode_tasks_enabled = false;
bool wait_for_beginframe_interval = true;
bool abort_commit_before_output_surface_creation = true;
bool use_mouse_wheel_gestures = false;
bool use_layer_lists = false;
int max_staging_buffer_usage_in_bytes = 32 * 1024 * 1024;
ManagedMemoryPolicy memory_policy_;
Expand Down
3 changes: 2 additions & 1 deletion components/test_runner/event_sender.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,7 @@ EventSender::SavedEvent::SavedEvent()

EventSender::EventSender(WebTestProxyBase* web_test_proxy_base)
: web_test_proxy_base_(web_test_proxy_base),
send_wheel_gestures_(false),
replaying_saved_events_(false),
weak_factory_(this) {
Reset();
Expand Down Expand Up @@ -2546,7 +2547,7 @@ void EventSender::InitMouseWheelEvent(gin::Arguments* args,
}
}
}
if (can_scroll) {
if (can_scroll && send_wheel_gestures_) {
can_scroll = false;
*send_gestures = true;
}
Expand Down
5 changes: 5 additions & 0 deletions components/test_runner/event_sender.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ class EventSender {

void DoDragDrop(const blink::WebDragData&, blink::WebDragOperationsMask);

void set_send_wheel_gestures(bool send_wheel_gestures) {
send_wheel_gestures_ = send_wheel_gestures;
}

private:
friend class EventSenderBindings;

Expand Down Expand Up @@ -251,6 +255,7 @@ class EventSender {
const blink::WebView* view() const;
blink::WebView* view();

bool send_wheel_gestures_;
bool force_layout_on_events_;

// When set to true (the default value), we batch mouse move and mouse up
Expand Down
4 changes: 4 additions & 0 deletions components/test_runner/web_test_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ void WebTestProxyBase::SetInterfaces(WebTestInterfaces* interfaces) {
test_interfaces_->WindowOpened(this);
}

void WebTestProxyBase::SetSendWheelGestures(bool send_gestures) {
event_sender_->set_send_wheel_gestures(send_gestures);
}

void WebTestProxyBase::Reset() {
accessibility_controller_->Reset();
event_sender_->Reset();
Expand Down
1 change: 1 addition & 0 deletions components/test_runner/web_test_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class TEST_RUNNER_EXPORT WebTestProxyBase {
void SetInterfaces(WebTestInterfaces* web_test_interfaces);

EventSender* event_sender() { return event_sender_.get(); }
void SetSendWheelGestures(bool send_gestures);

AccessibilityController* accessibility_controller() {
return accessibility_controller_.get();
Expand Down
5 changes: 4 additions & 1 deletion content/browser/renderer_host/input/input_router_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "content/common/content_constants_internal.h"
#include "content/common/edit_command.h"
#include "content/common/input/input_event_ack_state.h"
#include "content/common/input/input_event_utils.h"
#include "content/common/input/touch_action.h"
#include "content/common/input/web_touch_event_traits.h"
#include "content/common/input_messages.h"
Expand Down Expand Up @@ -83,7 +84,9 @@ InputRouterImpl::InputRouterImpl(IPC::Sender* sender,
flush_requested_(false),
active_renderer_fling_count_(0),
touch_scroll_started_sent_(false),
wheel_event_queue_(this, kDefaultWheelScrollTransactionMs),
wheel_event_queue_(this,
UseGestureBasedWheelScrolling(),
kDefaultWheelScrollTransactionMs),
touch_event_queue_(this, config.touch_config),
gesture_event_queue_(this, this, config.gesture_config),
device_scale_factor_(1.f) {
Expand Down
50 changes: 50 additions & 0 deletions content/browser/renderer_host/input/input_router_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,18 @@ class InputRouterImplTest : public testing::Test {
browser_context_.reset();
}

void SetUpForGestureBasedWheelScrolling(bool enabled) {
CHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableWheelGestures) &&
!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableWheelGestures));
base::CommandLine::ForCurrentProcess()->AppendSwitch(
enabled ? switches::kEnableWheelGestures
: switches::kDisableWheelGestures);
TearDown();
SetUp();
}

void SetUpForTouchAckTimeoutTest(int desktop_timeout_ms,
int mobile_timeout_ms) {
config_.touch_config.desktop_touch_ack_timeout_delay =
Expand Down Expand Up @@ -935,6 +947,44 @@ TEST_F(InputRouterImplTest, AckedTouchEventState) {
#endif // defined(USE_AURA)

TEST_F(InputRouterImplTest, UnhandledWheelEvent) {
SetUpForGestureBasedWheelScrolling(false);

// Simulate wheel events.
SimulateWheelEvent(0, 0, 0, -5, 0, false); // sent directly
SimulateWheelEvent(0, 0, 0, -10, 0, false); // enqueued

// Check that only the first event was sent.
EXPECT_TRUE(process_->sink().GetUniqueMessageMatching(
InputMsg_HandleInputEvent::ID));
EXPECT_EQ(1U, GetSentMessageCountAndResetSink());

// Indicate that the wheel event was unhandled.
SendInputEventACK(WebInputEvent::MouseWheel,
INPUT_EVENT_ACK_STATE_NOT_CONSUMED);

// Check that the correct unhandled wheel event was received.
EXPECT_EQ(1U, ack_handler_->GetAndResetAckCount());
EXPECT_EQ(INPUT_EVENT_ACK_STATE_NOT_CONSUMED, ack_handler_->ack_state());
EXPECT_EQ(ack_handler_->acked_wheel_event().deltaY, -5);

// Check that the second event was sent.
EXPECT_TRUE(process_->sink().GetUniqueMessageMatching(
InputMsg_HandleInputEvent::ID));
EXPECT_EQ(1U, GetSentMessageCountAndResetSink());

// Indicate that the wheel event was unhandled.
SendInputEventACK(WebInputEvent::MouseWheel,
INPUT_EVENT_ACK_STATE_NOT_CONSUMED);

// Check that the correct unhandled wheel event was received.
EXPECT_EQ(1U, ack_handler_->GetAndResetAckCount());
EXPECT_EQ(INPUT_EVENT_ACK_STATE_NOT_CONSUMED, ack_handler_->ack_state());
EXPECT_EQ(ack_handler_->acked_wheel_event().deltaY, -10);
}

TEST_F(InputRouterImplTest, UnhandledWheelEventWithGestureScrolling) {
SetUpForGestureBasedWheelScrolling(true);

// Simulate wheel events.
SimulateWheelEvent(0, 0, 0, -5, 0, false); // sent directly
SimulateWheelEvent(0, 0, 0, -10, 0, false); // enqueued
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ class QueuedWebMouseWheelEvent : public MouseWheelEventWithLatencyInfo {
};

MouseWheelEventQueue::MouseWheelEventQueue(MouseWheelEventQueueClient* client,
bool send_gestures,
int64_t scroll_transaction_ms)
: client_(client),
needs_scroll_begin_(true),
needs_scroll_end_(false),
send_gestures_(send_gestures),
scroll_transaction_ms_(scroll_transaction_ms),
scrolling_device_(blink::WebGestureDeviceUninitialized) {
DCHECK(client);
Expand Down Expand Up @@ -79,7 +81,7 @@ void MouseWheelEventQueue::ProcessMouseWheelAck(
client_->OnMouseWheelEventAck(*event_sent_for_gesture_ack_, ack_result);

// If event wasn't consumed then generate a gesture scroll for it.
if (ack_result != INPUT_EVENT_ACK_STATE_CONSUMED &&
if (send_gestures_ && ack_result != INPUT_EVENT_ACK_STATE_CONSUMED &&
event_sent_for_gesture_ack_->event.canScroll &&
event_sent_for_gesture_ack_->event.resendingPluginId == -1 &&
(scrolling_device_ == blink::WebGestureDeviceUninitialized ||
Expand Down
2 changes: 2 additions & 0 deletions content/browser/renderer_host/input/mouse_wheel_event_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class CONTENT_EXPORT MouseWheelEventQueue {
// |scroll_transaction_ms| is the duration in which the
// ScrollEnd should be sent after a ScrollUpdate.
MouseWheelEventQueue(MouseWheelEventQueueClient* client,
bool send_gestures,
int64_t scroll_transaction_ms);

~MouseWheelEventQueue();
Expand Down Expand Up @@ -97,6 +98,7 @@ class CONTENT_EXPORT MouseWheelEventQueue {
// GSB has been sent in the past.
bool needs_scroll_end_;

bool send_gestures_;
int64_t scroll_transaction_ms_;
blink::WebGestureDevice scrolling_device_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class MouseWheelEventQueueTest : public testing::Test,
MouseWheelEventQueueTest()
: acked_event_count_(0),
last_acked_event_state_(INPUT_EVENT_ACK_STATE_UNKNOWN) {
queue_.reset(new MouseWheelEventQueue(this, kScrollEndTimeoutMs));
SetUpForGestureTesting(false);
}

~MouseWheelEventQueueTest() override {}
Expand Down Expand Up @@ -176,6 +176,11 @@ class MouseWheelEventQueueTest : public testing::Test,
}

protected:
void SetUpForGestureTesting(bool send_gestures) {
queue_.reset(
new MouseWheelEventQueue(this, send_gestures, kScrollEndTimeoutMs));
}

size_t queued_event_count() const { return queue_->queued_size(); }

bool event_in_flight() const { return queue_->event_in_flight(); }
Expand Down Expand Up @@ -430,23 +435,28 @@ TEST_F(MouseWheelEventQueueTest, Basic) {
}

TEST_F(MouseWheelEventQueueTest, GestureSending) {
SetUpForGestureTesting(true);
GestureSendingTest(false);
}

TEST_F(MouseWheelEventQueueTest, GestureSendingPrecisePixels) {
SetUpForGestureTesting(true);
GestureSendingTest(false);
}

TEST_F(MouseWheelEventQueueTest, GestureSendingWithPhaseInformation) {
SetUpForGestureTesting(true);
PhaseGestureSendingTest(false);
}

TEST_F(MouseWheelEventQueueTest,
GestureSendingWithPhaseInformationPrecisePixels) {
SetUpForGestureTesting(true);
PhaseGestureSendingTest(true);
}

TEST_F(MouseWheelEventQueueTest, GestureSendingInterrupted) {
SetUpForGestureTesting(true);
const WebGestureEvent::ScrollUnits scroll_units = WebGestureEvent::Pixels;

SendMouseWheel(kWheelScrollX, kWheelScrollY, kWheelScrollGlobalX,
Expand Down Expand Up @@ -509,6 +519,7 @@ TEST_F(MouseWheelEventQueueTest, GestureSendingInterrupted) {
}

TEST_F(MouseWheelEventQueueTest, GestureRailScrolling) {
SetUpForGestureTesting(true);
const WebGestureEvent::ScrollUnits scroll_units = WebGestureEvent::Pixels;

SendMouseWheel(kWheelScrollX, kWheelScrollY, kWheelScrollGlobalX,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ class NonBlockingEventBrowserTest : public ContentBrowserTest {
main_thread_sync.Wait();
}

// ContentBrowserTest:
void SetUpCommandLine(base::CommandLine* cmd) override {
// TODO(dtapuska): Remove this switch once wheel-gestures ships.
cmd->AppendSwitch(switches::kEnableExperimentalWebPlatformFeatures);
cmd->AppendSwitch(switches::kEnableWheelGestures);
}

int ExecuteScriptAndExtractInt(const std::string& script) {
int value = 0;
EXPECT_TRUE(content::ExecuteScriptAndExtractInt(
Expand Down
6 changes: 5 additions & 1 deletion content/browser/renderer_host/overscroll_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "base/command_line.h"
#include "base/logging.h"
#include "content/browser/renderer_host/overscroll_controller_delegate.h"
#include "content/common/input/input_event_utils.h"
#include "content/public/browser/overscroll_configuration.h"
#include "content/public/common/content_switches.h"

Expand Down Expand Up @@ -35,13 +36,15 @@ OverscrollController::OverscrollController()
scroll_state_(STATE_UNKNOWN),
overscroll_delta_x_(0.f),
overscroll_delta_y_(0.f),
delegate_(NULL) {}
delegate_(NULL),
use_gesture_wheel_scrolling_(UseGestureBasedWheelScrolling()) {}

OverscrollController::~OverscrollController() {
}

bool OverscrollController::ShouldProcessEvent(
const blink::WebInputEvent& event) {
if (use_gesture_wheel_scrolling_) {
switch (event.type) {
case blink::WebInputEvent::MouseWheel:
return false;
Expand Down Expand Up @@ -73,6 +76,7 @@ bool OverscrollController::ShouldProcessEvent(
default:
break;
}
}
return true;
}

Expand Down
Loading

0 comments on commit 463485d

Please sign in to comment.