Currently we have: - NativeWebKeyboardEvent: Uses GOwnPtr for the native GdkEvent and ::nativeEvent() returns a const GdkEvent. - NativeWebMouseEvent: Doesn't use GOwnPtr, and ::nativeEvent() returns a GdkEvent. - NativeWebWheelEvent: Doesn't use GOwnPtr, and ::nativeEvent() returns a GdkEventScroll. and for WebEventFactory: - createWebKeyboardEvent() receives a const GdkEventKey - createWebMouseEvent() receives a GdkEvent - createWebWheelEvent() receives a GdkEventScroll I think the we could unify the way events are created following the current keyboard events code. We should use a GOwnPtr for the native event (it could be destroyed before the class is deleted) and use always const GdkEvent in public API.
Created attachment 92388 [details] Patch
Comment on attachment 92388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92388&action=review Very nice cleanup, but I do not understand the addition of the copy constructors. > Source/WebKit2/Shared/NativeWebMouseEvent.h:52 > + NativeWebMouseEvent(const NativeWebMouseEvent&); Why is a copy constructor necessary now? > Source/WebKit2/Shared/NativeWebWheelEvent.h:52 > + NativeWebWheelEvent(const NativeWebWheelEvent&); Ditto. > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:99 > +WebMouseEvent WebEventFactory::createWebMouseEvent(const GdkEvent *event, int currentClickCount) The asterisk should move to the left here. > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:141 > + switch (reinterpret_cast<const GdkEventScroll*>(event)->direction) { It's probably better to write event->scroll.direction. > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:175 > + guint keyval = reinterpret_cast<const GdkEventKey*>(event)->keyval; Ditto. event->key.keyval.
(In reply to comment #2) > (From update of attachment 92388 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=92388&action=review > > Very nice cleanup, but I do not understand the addition of the copy constructors. > > > Source/WebKit2/Shared/NativeWebMouseEvent.h:52 > > + NativeWebMouseEvent(const NativeWebMouseEvent&); > > Why is a copy constructor necessary now? Because the default copy constructor doesn't know how to copy a GdkEvent. > > Source/WebKit2/Shared/NativeWebWheelEvent.h:52 > > + NativeWebWheelEvent(const NativeWebWheelEvent&); > > Ditto. > > > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:99 > > +WebMouseEvent WebEventFactory::createWebMouseEvent(const GdkEvent *event, int currentClickCount) > > The asterisk should move to the left here. Ok. > > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:141 > > + switch (reinterpret_cast<const GdkEventScroll*>(event)->direction) { > > It's probably better to write event->scroll.direction. Ok. > > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:175 > > + guint keyval = reinterpret_cast<const GdkEventKey*>(event)->keyval; > > Ditto. event->key.keyval. Ok.
Comment on attachment 92388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92388&action=review >>> Source/WebKit2/Shared/NativeWebMouseEvent.h:52 >>> + NativeWebMouseEvent(const NativeWebMouseEvent&); >> >> Why is a copy constructor necessary now? > > Because the default copy constructor doesn't know how to copy a GdkEvent. Make sense! Might want to put a small note in the ChangeLog.
Committed r85856: <http://trac.webkit.org/changeset/85856>