The new |boundingBox| member will be used to store the touch region for a WebGestureEvent. This will be useful for gesture target fuzzing across multiple different gesture types (tap, long press, scroll, etc.)
Created attachment 154111 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 154111 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154111&action=review Can you also update the computation of PlatformGestureEvent::m_area at the same time? (looks like should be trivial?) > Source/WebKit/chromium/public/WebInputEvent.h:42 > +struct WebRect; This probably isn't necessary since you include the header? > Source/WebKit/chromium/public/WebInputEvent.h:388 > + , boundingBox(0, 0, 0, 0) WebRect initializes itself correctly. This should not be necessary.
Comment on attachment 154111 [details] Patch Please address sadrul's concerns before landing.
Created attachment 154117 [details] Patch for landing
Comment on attachment 154117 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=154117&action=review > Source/WebKit/chromium/src/WebInputEventConversion.cpp:155 > m_type = PlatformEvent::GestureTap; > - m_area = IntSize(e.deltaX * 2, e.deltaY * 2); > + IntRect bounds(e.boundingBox); > + m_area = IntSize(bounds.width(), bounds.height()); Don't you need to do this in a subsequent patch once your change to WebInputEvent has landed and you've updated Chromium to use this API?
Comment on attachment 154117 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=154117&action=review > Source/WebKit/chromium/public/WebInputEvent.h:373 > // NOTE: |deltaX| and |deltaY| represents the amount to scroll for Scroll gesture events. For Pinch gesture events, |deltaX| represents the scaling/magnification factor. For a GestureTap event, |deltaX| and |deltaY| represent the horizontal and vertical radii of the touch region. Do we need to update this comment?
(In reply to comment #6) > (From update of attachment 154117 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154117&action=review > > > Source/WebKit/chromium/src/WebInputEventConversion.cpp:155 > > m_type = PlatformEvent::GestureTap; > > - m_area = IntSize(e.deltaX * 2, e.deltaY * 2); > > + IntRect bounds(e.boundingBox); > > + m_area = IntSize(bounds.width(), bounds.height()); > > Don't you need to do this in a subsequent patch once your change to WebInputEvent has landed and you've updated Chromium to use this API? Yes, you're right... I will make this change in another patch once the Chromium side has landed.
(In reply to comment #7) > (From update of attachment 154117 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154117&action=review > > > Source/WebKit/chromium/public/WebInputEvent.h:373 > > // NOTE: |deltaX| and |deltaY| represents the amount to scroll for Scroll gesture events. For Pinch gesture events, |deltaX| represents the scaling/magnification factor. For a GestureTap event, |deltaX| and |deltaY| represent the horizontal and vertical radii of the touch region. > > Do we need to update this comment? Yes, we do. I'll update it in the next patch after this and the Chromium patch both land.
Created attachment 154121 [details] Patch
Comment on attachment 154121 [details] Patch Clearing flags on attachment: 154121 Committed r123520: <http://trac.webkit.org/changeset/123520>
All reviewed patches have been landed. Closing bug.