[webkit-reviews] review requested: [Bug 64572] Initial implementation of calling WebScriptObject::callWebScriptMethod() via COM : [Attachment 101367] Update patch v3

[bugzilla-daemon at webkit.org]

Anthony Johnson <anthony.johnson at flexsim.com> has asked  for review:
Bug 64572: Initial implementation of calling
WebScriptObject::callWebScriptMethod() via COM
https://bugs.webkit.org/show_bug.cgi?id=64572

Attachment 101367: Update patch v3
https://bugs.webkit.org/attachment.cgi?id=101367&action=review

------- Additional Comments from Anthony Johnson <anthony.johnson at flexsim.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=101367&action=review


> Source/WebKit/win/DOMCoreClasses.cpp:112
> +    if (!wrapper)

I've noticed that in some cases, m_node's wrapper is NULL. I don't remember the
specific case, but it was somewhat random, i.e. a pretty innocuous change in
the HTML made the node's wrapper NULL, and another change would make it valid
again. I couldn't figure out how to create a wrapper if it is not already
there, so here I simply fail if it is NULL. Any suggestions for future
improvements are welcome.

> Source/WebKit/win/WebScriptObject.cpp:118
> +    if (arg.vt & VT_ARRAY) { // it's an array

This is the main array part. For now I've only filled out short's, int's,
float's, and double's, design feedback pending, but hopefully you get the idea.


> Source/WebKit/win/WebScriptObject.cpp:163
> +	   switch (vt) {

Here's the straight-forward translation piece. Pretty simple, although there
are still a few things I still need to fill out going forward.

> Source/WebKit/win/WebScriptObject.cpp:199
> +{

This is mostly adapted from WebCore/bindings/objc/WebScriptObject.mm