Explicitly suppress scrolling for wheel events that will trigger zooming

content/browser/renderer_host/web_input_event_aura.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/web_input_event_aura.h"
 
 #include "content/browser/renderer_host/input/web_input_event_util.h"
 #include "content/browser/renderer_host/ui_events_helper.h"
 #include "ui/aura/window.h"
 #include "ui/events/event.h"
@@ skipping 179 matching lines @@
 
   // Replace the event's coordinate fields with translated position data from
   // |event|.
   webkit_event.windowX = webkit_event.x = event->x();
   webkit_event.windowY = webkit_event.y = event->y();
 
   const gfx::Point root_point = event->root_location();
   webkit_event.globalX = root_point.x();
   webkit_event.globalY = root_point.y();
 
+#if !defined(OS_MACOSX)

[tdresser, 2014/11/27 15:06:08]

Are _aura files compiled on Mac?
Don't we want different behavior on ChromeOS and Windows?

[lanwei, 2014/12/02 06:28:54]

Done.

+  if ((webkit_event.modifiers & blink::WebInputEvent::ControlKey) &&
+      !webkit_event.hasPreciseScrollingDeltas) {

[Rick Byers, 2014/11/27 17:31:32]

Actually, now that I review the rest of your CL you're not duplicating the logic
afterall - you're relying on this bit downstream.  That's MUCH simpler than what
I'm suggesting and I think nearly as good.  It's a little weird to assume
canScroll==false means must zoom, but this is all within content so given the
simplicity I think it's fine.

I'll leave the above comment since it took me awhile to figure out exactly what
the alternatives would look like and it might still be worth revisiting at some
point.

[Rick Byers, 2014/11/27 17:31:32]

I don't think we should be duplicating the logic for when wheel events trigger
zooming and when they don't.  This feels like a policy that's likely to change
over time, and so should be localized to one function somewhere. In some sense
that's the underlying problem in the bugs we're trying to fix - I effectively
duplicated the logic between chromium and blink, but even within chromium it
shouldn't be duplicated.

Today the logic is in WebContentsImpl::HandleWheelEvent.  So perhaps we should
extract it out to a new private function like 'bool
WebContentsImpl::ShouldWheelEventZoom'.  Then We could add a public API
WebContents::ShouldWheelEventScroll which RenderWidgetHostImpl::ForwardWheel
event calls in order to set the 'canScroll' bit on the WebMouseWheelEvent it's
sending.

Alternately, it might be cleaner (would let us remove an #ifdef!) to move the
logic for deciding when wheel should zoom from WebContentsImpl to
RenderWidgetHostView.  Then RenderWidgetHostViewAura could make it's own
platform-specific decision for when wheel events should trigger zooming (and Mac
wouldn't get that behavior at all).  It would use this logic both when calling
MakeWebMouseWheelEvent and in RenderWidgetHostViewAura::WheelEventAck in order
to trigger zooming.  This would probably require adding a method like
RenderViewHostDelegate::RequestZoom to get the zoom request back to the
WebContentsImpl.

+    webkit_event.suppressScroll = true;

[Rick Byers, 2014/11/27 17:31:32]

You just need a comment here saying that by suppressing scrolling we're enabling
WebContentsImpl::HandleWheelEvent to use the event to zoom the page.

[Rick Byers, 2014/11/27 17:31:32]

You need to set this bit in InputRouterImpl::SendSyntheticWheelEventForPinch
also, otherwise touchpad pinch on Mac will cause scrolling!

[lanwei, 2014/12/02 06:28:54]

Done.

[lanwei, 2014/12/02 06:28:54]

Done.

[lanwei, 2014/12/02 06:28:54]

Done.

+  }
+#endif
   return webkit_event;
 }
 
 blink::WebMouseWheelEvent MakeWebMouseWheelEvent(ui::ScrollEvent* event) {
 #if defined(OS_WIN)
   // Construct an untranslated event from the platform event data.
   blink::WebMouseWheelEvent webkit_event =
       MakeUntranslatedWebMouseWheelEventFromNativeEvent(event->native_event());
 #else
   blink::WebMouseWheelEvent webkit_event =
@@ skipping 142 matching lines @@
     webkit_event.deltaY = event->y_offset();
   }
 
   webkit_event.wheelTicksX = webkit_event.deltaX / kPixelsPerTick;
   webkit_event.wheelTicksY = webkit_event.deltaY / kPixelsPerTick;
 
   return webkit_event;
 }
 
 }  // namespace content