Use #pragma pack to avoid difference across compiler settings in different modules/platforms.
Created attachment 125905 [details] Patch
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment on attachment 125905 [details] Patch Attachment 125905 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11453063
The EWS failure is what the patch is trying to fix. The other side (which temporarily disables a static assert until this lands+rolls) is here: http://chromiumcodereview.appspot.com/9348029/
Comment on attachment 125905 [details] Patch OK. Seems if we're going to do this we should have a COMPILE_ASSERT to validate the size of these?
(In reply to comment #5) > (From update of attachment 125905 [details]) > OK. Seems if we're going to do this we should have a COMPILE_ASSERT to validate the size of these? There's one in Chromium code that the size matches there (which is the failure above). You mean here that the size is a specific number? Sure, that seems reasonable.
Created attachment 125948 [details] add compile_asserts
Comment on attachment 125948 [details] add compile_asserts View in context: https://bugs.webkit.org/attachment.cgi?id=125948&action=review Oh, at first I didn't realize these were Chromium headers. Looks fine. As the style-bot says, fishd is supposed to approve this sort of thing. Given that he's out this week, I think it's OK to move foward with this r=me, TBR=fishd. > Source/WebKit/chromium/public/platform/WebGamepads.h:50 > +COMPILE_ASSERT(sizeof(WebGamepads) == 1864, WebGamepads_has_wrong_size); WebKit has used this type of thing in the past to find compiler configuration bugs between platforms. :)
Comment on attachment 125948 [details] add compile_asserts Attachment 125948 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11480001
Comment on attachment 125948 [details] add compile_asserts View in context: https://bugs.webkit.org/attachment.cgi?id=125948&action=review > Source/WebKit/chromium/public/platform/WebGamepad.h:74 > +COMPILE_ASSERT(sizeof(WebGamepad) == 465, WebGamepad_has_wrong_size); I don't think you can use this macro in WebKit API header files. Where is it defined?
(In reply to comment #10) > (From update of attachment 125948 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125948&action=review > > > Source/WebKit/chromium/public/platform/WebGamepad.h:74 > > +COMPILE_ASSERT(sizeof(WebGamepad) == 465, WebGamepad_has_wrong_size); > > I don't think you can use this macro in WebKit API header files. Where is it defined? wtf/Assertions.h. Is that OK, or would it be better to have something in WebCommon.h analogous to WEBKIT_ASSERT there?
People using the WebKit API do not (and should not) have wtf/ on their include path, so that won't work.
Created attachment 126125 [details] add/use WEBKIT_COMPILE_ASSERT
Comment on attachment 126125 [details] add/use WEBKIT_COMPILE_ASSERT Attachment 126125 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11463504
You're breaking this COMPILE_ASSERT in pepper code: // Ensure conversion from WebKit::WebGamepads to PP_GamepadsData_Dev is safe. // See also DCHECKs in SampleGamepads below. COMPILE_ASSERT(sizeof(WebKit::WebGamepads) == sizeof(PP_GamepadsData_Dev), size_difference); COMPILE_ASSERT(sizeof(WebKit::WebGamepad) == sizeof(PP_GamepadData_Dev), size_difference);
(In reply to comment #15) > You're breaking this COMPILE_ASSERT in pepper code: > > // Ensure conversion from WebKit::WebGamepads to PP_GamepadsData_Dev is safe. > // See also DCHECKs in SampleGamepads below. > COMPILE_ASSERT(sizeof(WebKit::WebGamepads) == sizeof(PP_GamepadsData_Dev), > size_difference); > COMPILE_ASSERT(sizeof(WebKit::WebGamepad) == sizeof(PP_GamepadData_Dev), > size_difference); That's what this is fixing. It's landed in ppapi already, but lkgr hasn't updated yet.
LKGR is irrelevant. The revision pointed to by Source/WebKit/chromium/DEPS is what this bot is running, so you need to update that to make this pass.
(In reply to comment #17) > LKGR is irrelevant. The revision pointed to by Source/WebKit/chromium/DEPS is what this bot is running, so you need to update that to make this pass. Ah, oops, thanks. I guess that should be a separate patch? https://bugs.webkit.org/show_bug.cgi?id=78152
> Ah, oops, thanks. I guess that should be a separate patch? https://bugs.webkit.org/show_bug.cgi?id=78152 Yep. Thanks.
Comment on attachment 126125 [details] add/use WEBKIT_COMPILE_ASSERT View in context: https://bugs.webkit.org/attachment.cgi?id=126125&action=review > Source/Platform/chromium/public/WebCommon.h:124 > +#define WEBKIT_COMPILE_ASSERT(exp, name) typedef int dummy##name[(exp) ? 1 : -1] an alternative to introducing this macro would to be only perform the compile assert in a .cpp file, or within a "#if WEBKIT_IMPLEMENTATION" section of a header file. maybe that would be better than replicating the compile assert macro?
(In reply to comment #20) > (From update of attachment 126125 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126125&action=review > > > Source/Platform/chromium/public/WebCommon.h:124 > > +#define WEBKIT_COMPILE_ASSERT(exp, name) typedef int dummy##name[(exp) ? 1 : -1] > > an alternative to introducing this macro would to be only perform the compile assert in a .cpp file, or within a "#if WEBKIT_IMPLEMENTATION" section of a header file. maybe that would be better than replicating the compile assert macro? It looked like it belonged there since there was a WEBKIT_ASSERT, but I didn't realize how little that's used. It seems best to keep the assert next to the definition of the structure, so I guess in WEBKIT_IMPLEMENTATION is probably best.
Created attachment 126174 [details] webkit_implementation for compile_assert
Comment on attachment 126174 [details] webkit_implementation for compile_assert Attachment 126174 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11459716
Created attachment 126315 [details] reup for ews
Ping, is this OK to commit now?
Comment on attachment 126315 [details] reup for ews Clearing flags on attachment: 126315 Committed r108099: <http://trac.webkit.org/changeset/108099>
All reviewed patches have been landed. Closing bug.
The commit-queue encountered the following flaky tests while processing attachment 126315 [details]: http/tests/workers/text-encoding.html bug 78917 (author: dimich@chromium.org) The commit-queue is continuing to process your patch.