[webkit-reviews] review granted: [Bug 213462] [WebXR] Introducing XRLayer : [Attachment 402484] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 213462: [WebXR] Introducing XRLayer
https://bugs.webkit.org/show_bug.cgi?id=213462

Attachment 402484: Patch

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




--- Comment #2 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 402484
  --> https://bugs.webkit.org/attachment.cgi?id=402484
Patch

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

> Source/WebCore/Modules/webxr/WebXRLayer.h:48
> +    WebXRLayer(ScriptExecutionContext*);

explicit and ScriptExecutionContext& or is there a point in supporting nullptr?

> Source/WebCore/Modules/webxr/WebXRLayer.h:51
> +    ScriptExecutionContext* scriptExecutionContext() const override { return
ContextDestructionObserver::scriptExecutionContext(); }

final?

> Source/WebCore/Modules/webxr/WebXRLayer.h:57
> +    void derefEventTarget() override { deref(); }

final for all of these?

> Source/WebCore/Modules/webxr/WebXRSession.h:80
> +    ScriptExecutionContext* scriptExecutionContext() const override { return
ActiveDOMObject::scriptExecutionContext(); }

final?

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.cpp:107
>  WebXRWebGLLayer::WebXRWebGLLayer(WebXRSession& session,
WebXRRenderingContext&& context, const XRWebGLLayerInit& init)

Would be best to have a Ref<WebXRSession>&&

> Source/WebCore/Modules/webxr/WebXRWebGLLayer.h:62
> +    virtual ~WebXRWebGLLayer();

Not needed I believe