Bug 222896 - Be more restrictive about when canvas2d is allowed to update style
Summary: Be more restrictive about when canvas2d is allowed to update style
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-07 19:38 PST by Myles C. Maxfield
Modified: 2021-03-16 16:07 PDT (History)
7 users (show)

See Also:

Attachments
Patch (8.01 KB, patch)
2021-03-07 19:42 PST, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (7.88 KB, patch)
2021-03-15 23:12 PDT, Myles C. Maxfield 		rniwa: review+
Details | Formatted Diff | Diff
Patch for committing (7.31 KB, patch)
2021-03-16 12:16 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2021-03-07 19:38:34 PST 
Be more restrictive about when canvas2d is allowed to update style
Comment 1 Myles C. Maxfield 2021-03-07 19:42:49 PST 
Created attachment 422542 [details]
Patch
Comment 2 Myles C. Maxfield 2021-03-08 16:28:52 PST 
Comment on attachment 422542 [details]
Patch

I shouldn't move toTextDirection() before the RAII classes.
Comment 3 Radar WebKit Bug Importer 2021-03-14 20:39:12 PDT 
<rdar://problem/75416260>
Comment 4 Myles C. Maxfield 2021-03-15 23:12:08 PDT 
Created attachment 423304 [details]
Patch
Comment 5 Ryosuke Niwa 2021-03-15 23:19:21 PDT 
Comment on attachment 423304 [details]
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.
Comment 6 Myles C. Maxfield 2021-03-16 12:16:40 PDT 
Created attachment 423378 [details]
Patch for committing
Comment 7 Myles C. Maxfield 2021-03-16 16:07:01 PDT 
Committed r274531 (235382@main): <https://commits.webkit.org/235382@main>