[Webkit-unassigned] [Bug 40835] Adds support for throwing behavior of SerializedScriptValue to be specified in the IDL.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40835


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62810|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #43 from Adam Barth <abarth at webkit.org>  2011-01-12 14:19:50 PST ---
(From update of attachment 62810)
View in context: https://bugs.webkit.org/attachment.cgi?id=62810&action=review

Looks reasonable.  Some cleanup mentioned below.  The only serious problem is what looks like a memory leak.

> WebCore/bindings/js/SerializedScriptValue.cpp:977
> +    RefPtr<SerializedScriptValue> serializedValue = new SerializedScriptValue(SerializedScriptValueData::serialize(exec, value));

Isn't this a memory leak?  We need to call adoptRef, right?

> WebCore/bindings/js/SerializedScriptValue.cpp:991
> +    RefPtr<SerializedScriptValue> serializedValue = new SerializedScriptValue(SerializedScriptValueData::serialize(exec, value));

Same here.  adoptRef?

> WebCore/bindings/scripts/CodeGeneratorJS.pm:2491
> +    $ret = $ret . "    PassRefPtr<SerializedScriptValue> serializedScriptValue = $value;\n";

We don't usually store PassRefPtrs on the stack.  We usually us RefPtr and then release.

> WebCore/bindings/scripts/test/V8/V8TestObj.cpp:610
> +    if (UNLIKELY(ec))
> +        goto fail;

That's pretty nasty code.  Also, the indent is wrong here.

> WebCore/bindings/scripts/test/V8/V8TestObj.cpp:637
> +    {
> +    RefPtr<SerializedScriptValue> result = imp->serializedValueReturnRaises(ec);
> +    if (UNLIKELY(ec))
> +        goto fail;
> +    return result.release()->deserialize();
> +    }

Another bad indent.  Why do we need these { }  brackets?

> WebCore/bindings/v8/SerializedScriptValue.cpp:1031
> +        didThrow = true;

Why are we using didThrow instead of the ec, like we usually do?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.