[webkit-reviews] review granted: [Bug 198710] Add support for WeakRef : [Attachment 372046] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 198710: Add support for WeakRef
https://bugs.webkit.org/show_bug.cgi?id=198710

Attachment 372046: Patch

https://bugs.webkit.org/attachment.cgi?id=372046&action=review




--- Comment #21 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 372046
  --> https://bugs.webkit.org/attachment.cgi?id=372046
Patch

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

r=me for JSC part.

> Source/JavaScriptCore/runtime/JSWeakObjectRef.h:32
> +class JSWeakObjectRef final : public JSObject {

The base class needs to be JSNonFinalObject.

> Source/JavaScriptCore/runtime/JSWeakObjectRef.h:34
> +    using Base = JSObject;

Ditto.

> Source/JavaScriptCore/runtime/WeakObjectRefConstructor.cpp:76
> +    JSWeakObjectRef* WeakObjectRef = JSWeakObjectRef::create(vm,
WeakObjectRefStructure, exec->uncheckedArgument(0).getObject());
> +    RETURN_IF_EXCEPTION(scope, encodedJSValue());
> +    return JSValue::encode(WeakObjectRef);

We can make this as `RELEASE_AND_RETURN(scope, JSWeakObjectRef::create(vm,
WeakObjectRefStructure, exec->uncheckedArgument(0).getObject()));`.

> Source/JavaScriptCore/runtime/WeakObjectRefConstructor.h:30
> +class WeakObjectRefConstructor : public InternalFunction {

Annotate it with `final`.

> Source/JavaScriptCore/runtime/WeakObjectRefConstructor.h:32
> +    typedef InternalFunction Base;

Use `using`.

> Source/JavaScriptCore/runtime/WeakObjectRefConstructor.h:51
> +};

Add `static_assert(sizeof(WeakObjectRefConstructor) ==
sizeof(InternalFunction));`, to ensure that WeakObjectRefConstructor is OK to
be allocated from IsoSubspace of InternalFunction.

> Source/JavaScriptCore/runtime/WeakObjectRefPrototype.h:34
> +    typedef JSNonFinalObject Base;

Use `using`.