[Android] Out with the Android GR, in with the new unified C++ GR

content/browser/renderer_host/input/content_gesture_provider.h

+// Copyright 2013 The Chromium Authors. All rights reserved.

[tdresser, 2014/02/27 15:28:30]

2014

[jdduke (slow), 2014/02/27 17:40:29]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_BROWSER_RENDERER_HOST_INPUT_CONTENT_GESTURE_PROVIDER_H_
+#define CONTENT_BROWSER_RENDERER_HOST_INPUT_CONTENT_GESTURE_PROVIDER_H_
+
+#include "base/basictypes.h"
+#include "content/browser/renderer_host/input/gesture_event_packet.h"
+#include "content/browser/renderer_host/input/touch_disposition_gesture_filter.h"

[tdresser, 2014/02/27 15:28:30]

Wrapping after the #include would probably be nicer.

[jdduke (slow), 2014/02/27 17:40:29]

Hmm, I haven't noticed line breaking on long header inclusions before, is that
standard?

+#include "ui/events/gesture_detection/gesture_provider.h"
+
+namespace content {
+
+class ContentGestureProviderClient {
+ public:
+  virtual ~ContentGestureProviderClient() {}
+  virtual void OnGestureEvent(const blink::WebGestureEvent& event) = 0;

[tdresser, 2014/02/27 15:28:30]

Couldn't this object be entirely platform independent if we passed a
ui::GestureEventParams here? I thought that was the point of introducing
ui::GestureEventParams.

I don't think the split between platform specific code and common code is quite
right.

[jdduke (slow), 2014/02/27 17:40:29]

I'm not sure I understand what you're saying. The primary purpose of this class
is performing the content-specific task of converting ui::GestureEventParams
into a blink::WebGestureEvent and bundling it into a GesturePacket.  The
TouchDispositionGestureFilter is added as a convenience, but it's not required
here.  

+};
+
+// Provides gesture detection and dispatch given a sequence of touch events
+// and touch event acks.
+class ContentGestureProvider : public TouchDispositionGestureFilterClient,
+                               public ui::GestureProviderClient {
+ public:
+  // TODO(jdduke): Move the scaling constant out of this class, instead hosting
+  // it on the generating MotionEvent.
+  ContentGestureProvider(ContentGestureProviderClient* client,
+                         float touch_to_gesture_scale);
+
+  // Returns true if |event| was both valid and successfully handled by the
+  // gesture detector.  Otherwise, returns false, in which case the caller
+  // should drop |event|, not forwarding it to the renderer.
+  bool OnTouchEvent(const ui::MotionEvent& event);
+
+  // To be called upon ack of an event that was forwarded after a successful
+  // call to |OnTouchEvent()|.
+  void OnTouchEventAck(InputEventAckState ack_state);
+
+  // Methods delegated to |gesture_provider_|.
+  void ResetGestureDetectors();
+  void CancelActiveTouchSequence();
+  void UpdateMultiTouchSupport(bool support_multi_touch_zoom);

[tdresser, 2014/02/27 15:28:30]

This would take some renaming to be consistent, but what would you think of
using
Set{MultiTouchSupport,DoubleTapSupportForPlatform,DoubleTapSupportForPage}Enabled,
to make the meaning of the parameter clearer?

[jdduke (slow), 2014/02/27 17:40:29]

Done.

+  void UpdateDoubleTapSupportForPlatform(bool support_double_tap);
+  void UpdateDoubleTapSupportForPage(bool support_double_tap);
+
+ private:
+  // ui::GestureProviderClient
+  virtual void OnGestureEvent(const ui::GestureEventParams& params) OVERRIDE;
+
+  // TouchDispositionGestureFilterClient
+  virtual void ForwardGestureEvent(const blink::WebGestureEvent& event)
+      OVERRIDE;
+
+  ContentGestureProviderClient* const client_;
+  float touch_to_gesture_scale_;
+
+  ui::GestureProvider gesture_provider_;
+  TouchDispositionGestureFilter gesture_filter_;
+
+  bool handling_event_;
+  GestureEventPacket pending_gesture_packet_;
+
+  DISALLOW_COPY_AND_ASSIGN(ContentGestureProvider);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_RENDERER_HOST_INPUT_CONTENT_GESTURE_PROVIDER_H_