almost there

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/LocalObjectTable.h
File plugins/npapi/LocalObjectTable.h (right):

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/LocalObjectTable.h#newcode166
plugins/npapi/LocalObjectTable.h:166: set( id, jsObject );
nit: extra whitespace here

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/NPVariantUtil.h
File plugins/npapi/NPVariantUtil.h (right):

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/NPVariantUtil.h#newcode8
plugins/npapi/NPVariantUtil.h:8: // Sun's cstring doesn't define
strlen/etc
given that we don't actually build solaris, we should remove any such
ifdefs from our code as they are sure to grow stale.

please remove commented out includes as well.

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/ScriptableInstance.cpp
File plugins/npapi/ScriptableInstance.cpp (right):

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/ScriptableInstance.cpp#newcode356
plugins/npapi/ScriptableInstance.cpp:356: void
ScriptableInstance::testJsIdentity(const NPVariant* args, unsigned
argCount, NPVariant* result) {
i was thinking more along the lines of checking the UA during the
connect. there's no need to add a separate API hook to the JS for this.

don't we risk a false positive with this approach?

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/ScriptableInstance.h
File plugins/npapi/ScriptableInstance.h (right):

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/ScriptableInstance.h#newcode91
plugins/npapi/ScriptableInstance.h:91: static bool jsIdentitySafe;
does this really need to be static? it should be fine to test/set this
per connect

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/prebuilt/gwt-dev-plugin/background.html
File plugins/npapi/prebuilt/gwt-dev-plugin/background.html (right):

http://gwt-code-reviews.appspot.com/1469803/diff/6009/plugins/npapi/prebuilt/gwt-dev-plugin/background.html#newcode69
plugins/npapi/prebuilt/gwt-dev-plugin/background.html:69:
plugin.testJsIdentity( idObject, idObject );
if you go the route I suggested of simply testing (or sniffing the UA)
during connect, then you can remove this chunk.

http://gwt-code-reviews.appspot.com/1469803/

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

Reply via email to