[webkit-reviews] review granted: [Bug 190979] Add new global object and preliminary Worklets support for CSS painting api : [Attachment 353318] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Justin Michaud
<justin_michaud at apple.com>'s request for review:
Bug 190979: Add new global object and preliminary Worklets support for CSS
painting api
https://bugs.webkit.org/show_bug.cgi?id=190979

Attachment 353318: Patch

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




--- Comment #15 from Dean Jackson <dino at apple.com> ---
Comment on attachment 353318
  --> https://bugs.webkit.org/attachment.cgi?id=353318
Patch

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

> Source/WebCore/bindings/js/JSWorkletGlobalScopeBase.cpp:42
> +namespace WebCore {
> +using namespace JSC;

Nit: We usually put a blank line between these.

> Source/WebCore/css/CSSPaintImageValue.cpp:53
> +    auto* selectedGlobalScope =
renderElement.document().getCSSPaintWorkletGlobalScope();

Shouldn't this be getPaintWorkletGlobalScope() now?

> Source/WebCore/css/DOMCSSPaintWorklet.idl:30
> +    [ImplementedAs=ensurePaintWorklet,CallWith=Document] static readonly
attribute Worklet paintWorklet;

Should DOMCSSPaintWorklet.* be DOMPaintWorklet.* now?

> Source/WebCore/dom/Document.cpp:8381
> +Worklet& Document::ensureCSSPaintWorklet()

ensurePaintWorklet?

> Source/WebCore/worklets/PaintWorkletGlobalScope.cpp:45
> +    return 1.0;

Why not fetch this from the document?

> Source/WebCore/worklets/WorkletGlobalScope.h:55
> +    String origin() const final { ASSERT_NOT_REACHED(); return ""; }

emptyString()

> Source/WebCore/worklets/WorkletScriptController.cpp:170
> +	   // This FIXME is from in WorkerScriptController.

Typo: in