LGTM

This should solve Fabio's issue with the Android browser as well, right?


http://gwt-code-reviews.appspot.com/1277801/diff/1/3
File plugins/npapi/NPVariantWrapper.h (right):

http://gwt-code-reviews.appspot.com/1277801/diff/1/3#newcode105
plugins/npapi/NPVariantWrapper.h:105: // numerical variants out of V8.
Have we talked to them about this change?  That seems like a pretty big
change that could impact performance and break a lot of plugins, so I
would wonder if it is worth making this change if they are going to
revert it.

http://gwt-code-reviews.appspot.com/1277801/diff/1/3#newcode106
plugins/npapi/NPVariantWrapper.h:106: return
static_cast<int>(NPVARIANT_TO_DOUBLE(variant));
Should there be a check that the value actually is an integer?  I don't
know if it is a good idea to silently truncate a value here.

It might be better to have isInt accept doubles that have integral
values instead, and would require no changes to callers that expect
integers.

http://gwt-code-reviews.appspot.com/1277801/diff/1/5
File plugins/npapi/main.cpp (right):

http://gwt-code-reviews.appspot.com/1277801/diff/1/5#newcode241
plugins/npapi/main.cpp:241: //Debug::log(Debug::Spam) <<
"NPP_HandleEvent(instance=" << instance << ")" << Debug::flush;
Why commented out?  Spam seems low enough level to leave it on.

http://gwt-code-reviews.appspot.com/1277801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to