[chromium] Make sure the touch-points in the touch-events have the correct state.
Created attachment 166600 [details] Patch
Comment on attachment 166600 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166600&action=review > Source/WebKit/chromium/src/WebInputEventConversion.cpp:339 > +static inline WebTouchPoint::State toWebTouchPointState(const AtomicString& type) static and inline are redundant. > Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:94 > + switch (state) { > + case WebKit::WebTouchPoint::StateReleased: return stateReleased.data(); > + case WebKit::WebTouchPoint::StatePressed: return statePressed.data(); > + case WebKit::WebTouchPoint::StateMoved: return stateMoved.data(); > + case WebKit::WebTouchPoint::StateCancelled: return stateCancelled.data(); > + default: break; > + } This switch statement is not in the proper style. Why are you creating these CStrings? You can just return pointers to the string literals. > Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:108 > + const WebKit::WebTouchEvent& touch = reinterpret_cast<const WebKit::WebTouchEvent&>(event); reinterpret_cast -> static_cast Please read up on the differences between these two kinds of C++ casts, especially as they related to multi-inheritance. Using the wrong cast can result is subtle, dangerous bugs!
Comment on attachment 166600 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166600&action=review >> Source/WebKit/chromium/src/WebInputEventConversion.cpp:339 >> +static inline WebTouchPoint::State toWebTouchPointState(const AtomicString& type) > > static and inline are redundant. Fixed (also fixed the other cases of the same issue in this file). >> Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:94 >> + } > > This switch statement is not in the proper style. > > Why are you creating these CStrings? You can just return pointers to the string literals. Fixed. >> Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:108 >> + const WebKit::WebTouchEvent& touch = reinterpret_cast<const WebKit::WebTouchEvent&>(event); > > reinterpret_cast -> static_cast > > Please read up on the differences between these two kinds of C++ casts, especially as they related to multi-inheritance. Using the wrong cast can result is subtle, dangerous bugs! Whoops. My bad. Fixed. Thanks!
Created attachment 166622 [details] Patch
Comment on attachment 166622 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166622&action=review > Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:80 > +const char* pointState(WebKit::WebTouchPoint::State state) This function should probably be static > Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:99 > +void printTouchList(const WebKit::WebTouchPoint* points, int length) ditto > Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:105 > +void printEventDetails(const WebKit::WebInputEvent& event) ditto
Created attachment 166703 [details] Patch
Comment on attachment 166622 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166622&action=review >> Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:80 >> +const char* pointState(WebKit::WebTouchPoint::State state) > > This function should probably be static My bad. Fixed all cases.
Comment on attachment 166703 [details] Patch Thanks!
Comment on attachment 166703 [details] Patch Clearing flags on attachment: 166703 Committed r130197: <http://trac.webkit.org/changeset/130197>
All reviewed patches have been landed. Closing bug.