Title: [113344] trunk/Source/WebKit/chromium
Revision
113344
Author
commit-qu...@webkit.org
Date
2012-04-05 11:47:43 -0700 (Thu, 05 Apr 2012)

Log Message

[Chromium] Properly align members in WebInputEvent and subclasses to make Valgrind happy.
https://bugs.webkit.org/show_bug.cgi?id=81570

Patch by Lei Zhang <thes...@chromium.org> on 2012-04-05
Reviewed by Darin Fisher.

* public/WebInputEvent.h:
(WebKit):
(WebKit::WebInputEvent::WebInputEvent):
(WebKeyboardEvent):
(WebKit::WebTouchEvent::WebTouchEvent):

Modified Paths

Diff

Modified: trunk/Source/WebKit/chromium/ChangeLog (113343 => 113344)


--- trunk/Source/WebKit/chromium/ChangeLog	2012-04-05 18:32:22 UTC (rev 113343)
+++ trunk/Source/WebKit/chromium/ChangeLog	2012-04-05 18:47:43 UTC (rev 113344)
@@ -1,3 +1,16 @@
+2012-04-05  Lei Zhang  <thes...@chromium.org>
+
+        [Chromium] Properly align members in WebInputEvent and subclasses to make Valgrind happy.
+        https://bugs.webkit.org/show_bug.cgi?id=81570
+
+        Reviewed by Darin Fisher.
+
+        * public/WebInputEvent.h:
+        (WebKit):
+        (WebKit::WebInputEvent::WebInputEvent):
+        (WebKeyboardEvent):
+        (WebKit::WebTouchEvent::WebTouchEvent):
+
 2012-04-05  Shawn Singh  <shawnsi...@chromium.org>
 
         [chromium] Need to clip to homogeneous w=0 plane when applying transforms.

Modified: trunk/Source/WebKit/chromium/public/WebInputEvent.h (113343 => 113344)


--- trunk/Source/WebKit/chromium/public/WebInputEvent.h	2012-04-05 18:32:22 UTC (rev 113343)
+++ trunk/Source/WebKit/chromium/public/WebInputEvent.h	2012-04-05 18:47:43 UTC (rev 113344)
@@ -45,16 +45,23 @@
 // WARNING! These classes must remain PODs (plain old data).  They are
 // intended to be "serializable" by copying their raw bytes, so they must
 // not contain any non-bit-copyable member variables!
+//
+// Furthermore, the class members need to be packed so they are aligned
+// properly and don't have paddings/gaps, otherwise memory check tools
+// like Valgrind will complain about uninitialized memory usage when
+// transferring these classes over the wire.
 
+#pragma pack(push, 4)
+
 // WebInputEvent --------------------------------------------------------------
 
 class WebInputEvent {
 public:
     WebInputEvent(unsigned sizeParam = sizeof(WebInputEvent))
-        : size(sizeParam)
+        : timeStampSeconds(0.0)
+        , size(sizeParam)
         , type(Undefined)
-        , modifiers(0)
-        , timeStampSeconds(0.0) { }
+        , modifiers(0) { }
 
     // When we use an input method (or an input method editor), we receive
     // two events for a keypress. The former event is a keydown, which
@@ -145,10 +152,10 @@
 
     static const int InputModifiers = ShiftKey | ControlKey | AltKey | MetaKey;
 
-    unsigned size;   // The size of this structure, for serialization.
+    double timeStampSeconds; // Seconds since epoch.
+    unsigned size; // The size of this structure, for serialization.
     Type type;
     int modifiers;
-    double timeStampSeconds;   // Seconds since epoch.
 
     // Returns true if the WebInputEvent |type| is a mouse event.
     static bool isMouseEventType(int type)
@@ -226,6 +233,14 @@
     // doesn't hurt to have this one around.
     int nativeKeyCode;
 
+    // This identifies whether this event was tagged by the system as being
+    // a "system key" event (see
+    // http://msdn.microsoft.com/en-us/library/ms646286(VS.85).aspx for
+    // details). Other platforms don't have this concept, but it's just
+    // easier to leave it always false than ifdef.
+    // See comment at the top of the file for why an int is used here.
+    bool isSystemKey;
+
     // |text| is the text generated by this keystroke.  |unmodifiedText| is
     // |text|, but unmodified by an concurrently-held modifiers (except
     // shift).  This is useful for working out shortcut keys.  Linux and
@@ -238,17 +253,6 @@
     // This is a string identifying the key pressed.
     char keyIdentifier[keyIdentifierLengthCap];
 
-    // This identifies whether this event was tagged by the system as being
-    // a "system key" event (see
-    // http://msdn.microsoft.com/en-us/library/ms646286(VS.85).aspx for
-    // details).  Other platforms don't have this concept, but it's just
-    // easier to leave it always false than ifdef.
-    // int is used instead of bool to ensure the size of this structure is
-    // strictly aligned to a factor of 4 bytes, otherwise memory check tools
-    // like valgrind may complain about uninitialized memory usage when
-    // transfering it over the wire.
-    int isSystemKey;
-
     WebKeyboardEvent(unsigned sizeParam = sizeof(WebKeyboardEvent))
         : WebInputEvent(sizeParam)
         , windowsKeyCode(0)
@@ -323,13 +327,11 @@
     float wheelTicksX;
     float wheelTicksY;
 
-    // int is used instead of bool to ensure the size of this structure is
-    // strictly aligned to a factor of 4 bytes, otherwise memory check tools
-    // like valgrind may complain about uninitialized memory usage when
-    // transfering it over the wire.
+    // See comment at the top of the file for why an int is used here.
     int scrollByPage;
 
-    bool hasPreciseScrollingDeltas;
+    // See comment at the top of the file for why an int is used here.
+    int hasPreciseScrollingDeltas;
     Phase phase;
     Phase momentumPhase;
 
@@ -399,6 +401,8 @@
     }
 };
 
+#pragma pack(pop)
+
 } // namespace WebKit
 
 #endif

Modified: trunk/Source/WebKit/chromium/src/WebInputEvent.cpp (113343 => 113344)


--- trunk/Source/WebKit/chromium/src/WebInputEvent.cpp	2012-04-05 18:32:22 UTC (rev 113343)
+++ trunk/Source/WebKit/chromium/src/WebInputEvent.cpp	2012-04-05 18:47:43 UTC (rev 113344)
@@ -43,6 +43,38 @@
 
 namespace WebKit {
 
+class SameSizeAsWebInputEvent {
+    int inputData[5];
+};
+
+class SameSizeAsWebKeyboardEvent : public SameSizeAsWebInputEvent {
+    int keyboardData[12];
+};
+
+class SameSizeAsWebMouseEvent : public SameSizeAsWebInputEvent {
+    int mouseData[10];
+};
+
+class SameSizeAsWebMouseWheelEvent : public SameSizeAsWebMouseEvent {
+    int mousewheelData[8];
+};
+
+class SameSizeAsWebGestureEvent : public SameSizeAsWebInputEvent {
+    int gestureData[6];
+};
+
+class SameSizeAsWebTouchEvent : public SameSizeAsWebInputEvent {
+    WebTouchPoint touchPoints[3 * WebTouchEvent::touchesLengthCap];
+    int touchData[3];
+};
+
+COMPILE_ASSERT(sizeof(WebInputEvent) == sizeof(SameSizeAsWebInputEvent), WebInputEvent_has_gaps);
+COMPILE_ASSERT(sizeof(WebKeyboardEvent) == sizeof(SameSizeAsWebKeyboardEvent), WebKeyboardEvent_has_gaps);
+COMPILE_ASSERT(sizeof(WebMouseEvent) == sizeof(SameSizeAsWebMouseEvent), WebMouseEvent_has_gaps);
+COMPILE_ASSERT(sizeof(WebMouseWheelEvent) == sizeof(SameSizeAsWebMouseWheelEvent), WebMouseWheelEvent_has_gaps);
+COMPILE_ASSERT(sizeof(WebGestureEvent) == sizeof(SameSizeAsWebGestureEvent), WebGestureEvent_has_gaps);
+COMPILE_ASSERT(sizeof(WebTouchEvent) == sizeof(SameSizeAsWebTouchEvent), WebTouchEvent_has_gaps);
+
 static const char* staticKeyIdentifiers(unsigned short keyCode)
 {
     switch (keyCode) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to