Title: [274531] trunk/Source/WebCore
Revision
274531
Author
[email protected]
Date
2021-03-16 16:05:00 -0700 (Tue, 16 Mar 2021)

Log Message

Be more restrictive about when canvas2d is allowed to update style
https://bugs.webkit.org/show_bug.cgi?id=222896

Reviewed by Ryosuke Niwa.

We can use the RAII class ScriptDisallowedScope::InMainThread to enforce places where
script is allowed to be run.

This is just a cleanup/clarity patch.

No new tests because there is no behavior change.

* html/canvas/CanvasRenderingContext2D.cpp:
(WebCore::CanvasRenderingContext2D::setFont):
(WebCore::CanvasRenderingContext2D::setFontWithoutUpdatingStyle):
(WebCore::CanvasRenderingContext2D::toTextDirection const):
(WebCore::CanvasRenderingContext2D::measureText):
(WebCore::CanvasRenderingContext2D::fontProxy const):
(WebCore::CanvasRenderingContext2D::drawTextInternal):
* html/canvas/CanvasRenderingContext2D.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (274530 => 274531)


--- trunk/Source/WebCore/ChangeLog	2021-03-16 23:02:47 UTC (rev 274530)
+++ trunk/Source/WebCore/ChangeLog	2021-03-16 23:05:00 UTC (rev 274531)
@@ -1,3 +1,26 @@
+2021-03-15  Myles C. Maxfield  <[email protected]>
+
+        Be more restrictive about when canvas2d is allowed to update style
+        https://bugs.webkit.org/show_bug.cgi?id=222896
+
+        Reviewed by Ryosuke Niwa.
+
+        We can use the RAII class ScriptDisallowedScope::InMainThread to enforce places where
+        script is allowed to be run.
+
+        This is just a cleanup/clarity patch.
+
+        No new tests because there is no behavior change.
+
+        * html/canvas/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::setFont):
+        (WebCore::CanvasRenderingContext2D::setFontWithoutUpdatingStyle):
+        (WebCore::CanvasRenderingContext2D::toTextDirection const):
+        (WebCore::CanvasRenderingContext2D::measureText):
+        (WebCore::CanvasRenderingContext2D::fontProxy const):
+        (WebCore::CanvasRenderingContext2D::drawTextInternal):
+        * html/canvas/CanvasRenderingContext2D.h:
+
 2021-03-16  Commit Queue  <[email protected]>
 
         Unreviewed, reverting r274473.

Modified: trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp (274530 => 274531)


--- trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp	2021-03-16 23:02:47 UTC (rev 274530)
+++ trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp	2021-03-16 23:05:00 UTC (rev 274531)
@@ -44,13 +44,16 @@
 #include "NodeRenderStyle.h"
 #include "Path2D.h"
 #include "RenderTheme.h"
+#include "RenderWidget.h"
 #include "ResourceLoadObserver.h"
 #include "RuntimeEnabledFeatures.h"
+#include "ScriptDisallowedScope.h"
 #include "Settings.h"
 #include "StyleBuilder.h"
 #include "StyleFontSizeFunctions.h"
 #include "StyleProperties.h"
 #include "StyleResolveForFontRaw.h"
+#include "StyleTreeResolver.h"
 #include "TextMetrics.h"
 #include "TextRun.h"
 #include <wtf/CheckedArithmetic.h>
@@ -100,6 +103,21 @@
 
 void CanvasRenderingContext2D::setFont(const String& newFont)
 {
+    Document& document = canvas().document();
+    document.updateStyleIfNeeded();
+
+    setFontWithoutUpdatingStyle(newFont);
+}
+
+void CanvasRenderingContext2D::setFontWithoutUpdatingStyle(const String& newFont)
+{
+    // Intentionally don't update style here, because updating style can cause JS to run synchronously.
+    // This function is called in the middle of processing, and running arbitrary JS in the middle of processing can cause unexpected behavior.
+    // Instead, the relevant canvas entry points update style once when they begin running, and we won't touch the style after that.
+    // This means that the style may end up being stale here, but that's at least better than running arbitrary JS in the middle of processing.
+
+    ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+
     if (newFont.isEmpty())
         return;
 
@@ -112,11 +130,6 @@
     if (!fontRaw)
         return;
 
-    // Map the <canvas> font into the text style. If the font uses keywords like larger/smaller, these will work
-    // relative to the canvas.
-    Document& document = canvas().document();
-    document.updateStyleIfNeeded();
-
     FontCascadeDescription fontDescription;
     if (auto* computedStyle = canvas().computedStyle())
         fontDescription = FontCascadeDescription { computedStyle->fontDescription() };
@@ -127,6 +140,9 @@
         fontDescription.setComputedSize(DefaultFontSize);
     }
 
+    // Map the <canvas> font into the text style. If the font uses keywords like larger/smaller, these will work
+    // relative to the canvas.
+    Document& document = canvas().document();
     auto fontCascade = Style::resolveForFontRaw(*fontRaw, WTFMove(fontDescription), document);
     if (!fontCascade)
         return;
@@ -142,7 +158,7 @@
 
 inline TextDirection CanvasRenderingContext2D::toTextDirection(Direction direction, const RenderStyle** computedStyle) const
 {
-    auto* style = (computedStyle || direction == Direction::Inherit) ? canvas().computedStyle() : nullptr;
+    auto* style = computedStyle || direction == Direction::Inherit ? canvas().existingComputedStyle() : nullptr;
     if (computedStyle)
         *computedStyle = style;
     switch (direction) {
@@ -176,6 +192,10 @@
 
 Ref<TextMetrics> CanvasRenderingContext2D::measureText(const String& text)
 {
+    downcast<HTMLCanvasElement>(canvasBase()).document().updateStyleIfNeeded();
+
+    ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+
     if (RuntimeEnabledFeatures::sharedFeatures().webAPIStatisticsEnabled()) {
         auto& canvas = this->canvas();
         ResourceLoadObserver::shared().logCanvasWriteOrMeasure(canvas.document(), text);
@@ -191,15 +211,23 @@
 }
 
 auto CanvasRenderingContext2D::fontProxy() -> const FontProxy* {
-    auto& canvas = downcast<HTMLCanvasElement>(canvasBase());
-    canvas.document().updateStyleIfNeeded();
+    // Intentionally don't update style here, because updating style can cause JS to run synchronously.
+    // This function is called in the middle of processing, and running arbitrary JS in the middle of processing can cause unexpected behavior.
+    // Instead, the relevant canvas entry points update style once when they begin running, and we won't touch the style after that.
+    // This means that the style may end up being stale here, but that's at least better than running arbitrary JS in the middle of processing.
+    ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+
     if (!state().font.realized())
-        setFont(state().unparsedFont);
+        setFontWithoutUpdatingStyle(state().unparsedFont);
     return &state().font;
 }
 
 void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, float y, bool fill, Optional<float> maxWidth)
 {
+    downcast<HTMLCanvasElement>(canvasBase()).document().updateStyleIfNeeded();
+
+    ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+
     if (RuntimeEnabledFeatures::sharedFeatures().webAPIStatisticsEnabled())
         ResourceLoadObserver::shared().logCanvasWriteOrMeasure(this->canvas().document(), text);
 

Modified: trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.h (274530 => 274531)


--- trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.h	2021-03-16 23:02:47 UTC (rev 274530)
+++ trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.h	2021-03-16 23:05:00 UTC (rev 274531)
@@ -58,6 +58,7 @@
 
     bool is2d() const final { return true; }
     const FontProxy* fontProxy() final;
+    void setFontWithoutUpdatingStyle(const String&);
 
     void drawTextInternal(const String& text, float x, float y, bool fill, Optional<float> maxWidth = WTF::nullopt);
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to