Bug 62237 – ScriptValue should use OwnHandle

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Adam Barth]

ScriptValue should use OwnHandle

[Adam Barth]

Created attachment 96310 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 96310 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96310&action=review

> Source/WebCore/bindings/v8/ScriptValue.h:66
> +        if (value.m_value.get().IsEmpty())

I see.  We can't use -> here because the returned value is not a pointer?  I still wonder if we shouldn't override -> on OwnHandle to make it work like a poitner.

[WebKit Review Bot]

Comment on attachment 96310 [details]
Patch

Attachment 96310 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8776453

[WebKit Review Bot]

Comment on attachment 96310 [details]
Patch

Attachment 96310 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/8786386

[Alexey Proskuryakov]

OwnHandle or OwnPtr?

[Adam Barth]

(In reply to comment #5)
> OwnHandle or OwnPtr?

OwnHandle.  It puts v8::Persistent through it's New/Dispose/Clear lifecycle.

[Eric Seidel (no email)]

Comment on attachment 96310 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96310&action=review

> Source/WebCore/bindings/v8/ScriptValue.h:162
> +    mutable OwnPtr<v8::Value> m_value;

Except you used an OwnPtr...

[Adam Barth]

> Except you used an OwnPtr...

I think I was "on tilt" when I wrote these patches.  You should just R- them.  I'll try again another day.