RESOLVED FIXED222896
Be more restrictive about when canvas2d is allowed to update style
https://bugs.webkit.org/show_bug.cgi?id=222896
Summary Be more restrictive about when canvas2d is allowed to update style
Myles C. Maxfield
Reported 2021-03-07 19:38:34 PST
Be more restrictive about when canvas2d is allowed to update style
Attachments
Patch (8.01 KB, patch)
2021-03-07 19:42 PST, Myles C. Maxfield
no flags
Patch (7.88 KB, patch)
2021-03-15 23:12 PDT, Myles C. Maxfield
rniwa: review+
Patch for committing (7.31 KB, patch)
2021-03-16 12:16 PDT, Myles C. Maxfield
no flags
Myles C. Maxfield
Comment 1 2021-03-07 19:42:49 PST
Myles C. Maxfield
Comment 2 2021-03-08 16:28:52 PST
Comment on attachment 422542 [details] Patch I shouldn't move toTextDirection() before the RAII classes.
Radar WebKit Bug Importer
Comment 3 2021-03-14 20:39:12 PDT
Myles C. Maxfield
Comment 4 2021-03-15 23:12:08 PDT
Ryosuke Niwa
Comment 5 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.
Myles C. Maxfield
Comment 6 2021-03-16 12:16:40 PDT
Created attachment 423378 [details] Patch for committing
Myles C. Maxfield
Comment 7 2021-03-16 16:07:01 PDT
Note You need to log in before you can comment on or make changes to this bug.