[webkit-reviews] review granted: [Bug 222896] Be more restrictive about when canvas2d is allowed to update style : [Attachment 423304] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 222896: Be more restrictive about when canvas2d is allowed to update style
https://bugs.webkit.org/show_bug.cgi?id=222896

Attachment 423304: Patch

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




--- Comment #5 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 423304
  --> https://bugs.webkit.org/attachment.cgi?id=423304
Patch

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

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:121
> +    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
> +    ScriptDisallowedScope::InMainThread scriptDisallowedScope;
> +    Style::PostResolutionCallbackDisabler
callbackDisabler(canvas().document());

We don't need WidgetHierarchyUpdatesSuspensionScope or
PostResolutionCallbackDisabler.
Just ScriptDisallowedScope::InMainThread is enough.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:163
> -    auto* style = (computedStyle || direction == Direction::Inherit) ?
canvas().computedStyle() : nullptr;
> +    auto* style = (computedStyle || direction == Direction::Inherit) ?
canvas().existingComputedStyle() : nullptr;

Maybe get rid of the unnecessary parenthesis here?

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:201
> +    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
> +    ScriptDisallowedScope::InMainThread scriptDisallowedScope;
> +    Style::PostResolutionCallbackDisabler
callbackDisabler(canvas().document());

Ditto. We just need ScriptDisallowedScope::InMainThread here.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:-195
> -    auto& canvas = downcast<HTMLCanvasElement>(canvasBase());
> -    canvas.document().updateStyleIfNeeded();

Let's also add ScriptDisallowedScope::InMainThread here?

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:234
> +    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
> +    ScriptDisallowedScope::InMainThread scriptDisallowedScope;
> +    Style::PostResolutionCallbackDisabler
callbackDisabler(canvas().document());

Ditto. We just need ScriptDisallowedScope::InMainThread.