Summary: | [Android] bindings/v8/NPV8Object.cpp does not compile on Android | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andrei Popescu <andreip> | ||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | android-webkit-unforking, eric, ojan | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Android | ||||||||
OS: | Android | ||||||||
Bug Depends on: | 33917 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Andrei Popescu
2010-01-13 10:50:12 PST
Created attachment 46476 [details]
Fix bindings/v8/NPV8Object.cpp to compile on Android
Seems the ARRAYSIZE_UNSAFE macro should move into WTF at some point. Ojan filed a bug about that a while back. Not specifically about that macro, but about the idea of having a macro or template function to do C-style array size calculations. Comment on attachment 46476 [details]
Fix bindings/v8/NPV8Object.cpp to compile on Android
Pls get rid of ifdef's as discussed.
I filed a separate bug (33917) for adding the PlatformBridge.h header. Will upload a new patch here, as well. Created attachment 47109 [details]
Fix bindings/v8/NPV8Object.cpp to compile on Android
Comment on attachment 47109 [details] Fix bindings/v8/NPV8Object.cpp to compile on Android Just a few nits to address below and submit away. > Index: WebCore/bindings/v8/NPV8Object.h > +#if PLATFORM(CHROMIUM) > +// Chromium uses a different npruntime.h, which is in > +// the Chromium source repository under third_party/npapi/bindings. > +// The Google-specific changes in that file should probably be Should this say "FIXME: " right before it. > +// moved into bridge/npruntime.h, guarded by an #if PlATFORM(CHROMIUM). > #include "bindings/npruntime.h" > +#else > +#include "bridge/npruntime.h" // use WebCore version for Android and other ports. Please capitalize the u (s/use/Use/) and only have a single space before end of line comments. > Index: WebCore/platform/android/PlatformBridge.h Add 2010 to the copyright line? > +// ARRAYSIZE_UNSAFE performs essentially the same calculation as arraysize, > +// but can be used on anonymous types or types defined inside > +// functions. It's less safe than arraysize as it accepts some Only use single spaces after periods in comments for WebKit code. > +// (although not all) pointers. Therefore, you should use arraysize Single space after period. > +// ARRAYSIZE_UNSAFE catches a few type errors. If you see a compiler error Single space after period. > +// the array) and sizeof(*(arr)) (the # of bytes in one array > +// element). If the former is divisible by the latter, perhaps arr is Single space after period. > +// indeed an array, in which case the division result is the # of > +// elements in the array. Otherwise, arr cannot possibly be an array, Single space after period. > +// and we generate a compiler error to prevent the code from > +// compiling. > +// pointers, namely where the pointer size is divisible by the pointee > +// size. Since all our code has to go through a 32-bit compiler, Single space after period. Thanks for the review David. I addressed all your comments and Steve Block landed the patch in http://trac.webkit.org/changeset/53634 |