[webkit-reviews] review canceled: [Bug 93218] [V8] Replace v8::Undefined() in bindings/v8/* with v8Undefined() : [Attachment 156619] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has canceled Kentaro Hara
<haraken at chromium.org>'s request for review:
Bug 93218: [V8] Replace v8::Undefined() in bindings/v8/* with v8Undefined()
https://bugs.webkit.org/show_bug.cgi?id=93218

Attachment 156619: Patch
https://bugs.webkit.org/attachment.cgi?id=156619&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
I discussed with the V8 team and found the following facts:

- v8::Handle<v8::Value>() and v8::Undefined() are not equivalent. A bunch of V8
APIs do not support v8::Handle<v8::Value>(). If we pass v8::Handle<v8::Value>()
to such V8 APIs, they crash. We have to pass v8::Undefined(). In other words,
we cannot simply replace v8::Undefined() with v8::Handle<v8::Value>().

- For return values of DOM attribute/method callbacks, v8::Handle<v8::Value>()
and v8::Undefined() are equivalent.


Possible solutions:

[A] Implement v8FastUndefined() as follows:

  v8FastUndefined() { return v8::Handle<v8::Value>(); }

Use v8FastUndefined() at the places where v8::Handle<v8::Value>() and
v8::Undefined() are equivalent. Use v8::Undefined() otherwise.


[B] Implement v8Undefined(Isolate*) as follows:

  v8Undefined(Isolate* isolate = 0) { return isolate ? v8::Undefined(isolate) :
v8::Undefined(); }

Use v8Undefined(Isolate*) everywhere.

In terms of performance, if an Isolate exists, [A] and [B] are almost the same.
If an Isolate does not exist, [A] is faster than [B]. (See
https://bugs.webkit.org/show_bug.cgi?id=93093#c0)

Which would be better?