Resolves a FIXME I added here on r81676.
Created attachment 86578 [details] Patch
Attachment 86578 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8221791
Created attachment 86579 [details] Patch - remove linux warning
Comment on attachment 86579 [details] Patch - remove linux warning View in context: https://bugs.webkit.org/attachment.cgi?id=86579&action=review omg, the style in this file is so inconsistent. > Source/WebKit/chromium/public/WebBindings.h:52 > + WEBKIT_API static bool construct(NPP, NPObject*, const NPVariant* args, uint32_t count, NPVariant* result); Why change this? argCount seems more clear to me. > Source/WebKit/chromium/public/WebBindings.h:76 > + WEBKIT_API static void getStringIdentifiers(const NPUTF8** names, int32_t count, NPIdentifier*); ditto > Source/WebKit/chromium/src/WebBindings.cpp:207 > + if (!object || (object->_class != npScriptObjectClass)) > + return false; Since this is a code change could we do the style change first and then a code change? > Source/WebKit/chromium/src/WebBindings.cpp:227 > + V8NPObject* v8npobject = reinterpret_cast<V8NPObject*>(object); To match the class name, this maybe should be v8NPObject. > Source/WebKit/chromium/src/WebBindings.cpp:228 > + v8::Handle<v8::Object> v8object(v8npobject->v8Object); Ditto. This should probably be v8Object.
Comment on attachment 86579 [details] Patch - remove linux warning View in context: https://bugs.webkit.org/attachment.cgi?id=86579&action=review >> Source/WebKit/chromium/public/WebBindings.h:52 >> + WEBKIT_API static bool construct(NPP, NPObject*, const NPVariant* args, uint32_t count, NPVariant* result); > > Why change this? argCount seems more clear to me. Done. >> Source/WebKit/chromium/public/WebBindings.h:76 >> + WEBKIT_API static void getStringIdentifiers(const NPUTF8** names, int32_t count, NPIdentifier*); > > ditto Done. And argCount in the remaining places below. >> Source/WebKit/chromium/src/WebBindings.cpp:207 >> + return false; > > Since this is a code change could we do the style change first and then a code change? Understood. Removed the code change. >> Source/WebKit/chromium/src/WebBindings.cpp:227 >> + V8NPObject* v8npobject = reinterpret_cast<V8NPObject*>(object); > > To match the class name, this maybe should be v8NPObject. Done. >> Source/WebKit/chromium/src/WebBindings.cpp:228 >> + v8::Handle<v8::Object> v8object(v8npobject->v8Object); > > Ditto. This should probably be v8Object. Done.
Created attachment 86741 [details] Patch (part 1) style change
Comment on attachment 86741 [details] Patch (part 1) style change View in context: https://bugs.webkit.org/attachment.cgi?id=86741&action=review > Source/WebKit/chromium/ChangeLog:10 > + [chromium] WebBindings::getRangeImpl() should NULL check its NPObject argument > + https://bugs.webkit.org/show_bug.cgi?id=56898 > + > + Address FIXME I noted/added in r81676. Wanted to just copy/paste the checking > + code from elsewhere in the file, but inconsistent argument naming made it more > + difficult than it need be so, to clean up the commons, add consistent naming. This description no longer matches the patch. It's a bit lame, but since you'll be doing the NULL check in a separate patch, this cleanup should be done on a different bug.
Ok changing the bug title.
Created attachment 86742 [details] Patch (mod 2) style change
Created attachment 86743 [details] Patch (mod 2) style change changelog
Opened bug 56996 re the NULL check.
Comment on attachment 86743 [details] Patch (mod 2) style change changelog Clearing flags on attachment: 86743 Committed r81854: <http://trac.webkit.org/changeset/81854>
All reviewed patches have been landed. Closing bug.