Title: [280020] trunk/Source/WebCore
Revision
280020
Author
wei...@apple.com
Date
2021-07-18 12:18:20 -0700 (Sun, 18 Jul 2021)

Log Message

Fix canvas overflow checking to use CheckedArithmatic rather than adhoc floating point mechanism
https://bugs.webkit.org/show_bug.cgi?id=228058

Reviewed by Dean Jackson.

We have CheckedArithmatic just for this case. Use it.

* html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::shouldAccelerate const):
(WebCore::HTMLCanvasElement::createImageBuffer const):
* html/HTMLCanvasElement.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (280019 => 280020)


--- trunk/Source/WebCore/ChangeLog	2021-07-18 15:41:37 UTC (rev 280019)
+++ trunk/Source/WebCore/ChangeLog	2021-07-18 19:18:20 UTC (rev 280020)
@@ -1,3 +1,17 @@
+2021-07-18  Sam Weinig  <wei...@apple.com>
+
+        Fix canvas overflow checking to use CheckedArithmatic rather than adhoc floating point mechanism
+        https://bugs.webkit.org/show_bug.cgi?id=228058
+
+        Reviewed by Dean Jackson.
+
+        We have CheckedArithmatic just for this case. Use it.
+
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::shouldAccelerate const):
+        (WebCore::HTMLCanvasElement::createImageBuffer const):
+        * html/HTMLCanvasElement.h:
+
 2021-07-18  Wenson Hsieh  <wenson_hs...@apple.com>
 
         [iOS] [AX] Keyboard text candidates don't update when changing selection using Switch Control

Modified: trunk/Source/WebCore/html/HTMLCanvasElement.cpp (280019 => 280020)


--- trunk/Source/WebCore/html/HTMLCanvasElement.cpp	2021-07-18 15:41:37 UTC (rev 280019)
+++ trunk/Source/WebCore/html/HTMLCanvasElement.cpp	2021-07-18 19:18:20 UTC (rev 280020)
@@ -862,12 +862,17 @@
 
 bool HTMLCanvasElement::shouldAccelerate(const IntSize& size) const
 {
+    auto checkedArea = size.area<RecordOverflow>();
+    if (checkedArea.hasOverflowed())
+        return false;
+
+    return shouldAccelerate(checkedArea.value());
+}
+
+bool HTMLCanvasElement::shouldAccelerate(unsigned area) const
+{
     auto& settings = document().settings();
 
-    auto area = size.area<RecordOverflow>();
-    if (area.hasOverflowed())
-        return false;
-
     if (area > settings.maximumAccelerated2dCanvasSize())
         return false;
 
@@ -874,7 +879,6 @@
 #if USE(IOSURFACE_CANVAS_BACKING_STORE)
     return settings.canvasUsesAcceleratedDrawing();
 #else
-    UNUSED_PARAM(size);
     return false;
 #endif
 }
@@ -927,27 +931,29 @@
     m_hasCreatedImageBuffer = true;
     m_didClearImageBuffer = true;
 
-    // Perform multiplication as floating point to avoid overflow
-    if (float(width()) * height() > maxCanvasArea) {
+    auto checkedArea = size().area<RecordOverflow>();
+
+    if (checkedArea.hasOverflowed() || checkedArea > maxCanvasArea) {
         auto message = makeString("Canvas area exceeds the maximum limit (width * height > ", maxCanvasArea, ").");
         document().addConsoleMessage(MessageSource::JS, MessageLevel::Warning, message);
         return;
     }
-    
+
     // Make sure we don't use more pixel memory than the system can support.
-    size_t requestedPixelMemory = 4 * width() * height();
-    if (activePixelMemory() + requestedPixelMemory > maxActivePixelMemory()) {
+    auto checkedRequestedPixelMemory = (4 * checkedArea) + activePixelMemory();
+    if (checkedRequestedPixelMemory.hasOverflowed() || checkedRequestedPixelMemory > maxActivePixelMemory()) {
         auto message = makeString("Total canvas memory use exceeds the maximum limit (", maxActivePixelMemory() / 1024 / 1024, " MB).");
         document().addConsoleMessage(MessageSource::JS, MessageLevel::Warning, message);
         return;
     }
 
-    if (!width() || !height())
+    unsigned area = checkedArea.value();
+    if (!area)
         return;
 
     auto hostWindow = (document().view() && document().view()->root()) ? document().view()->root()->hostWindow() : nullptr;
 
-    auto renderingMode = shouldAccelerate(size()) ? RenderingMode::Accelerated : RenderingMode::Unaccelerated;
+    auto renderingMode = shouldAccelerate(area) ? RenderingMode::Accelerated : RenderingMode::Unaccelerated;
     // FIXME: Add a new setting for DisplayList drawing on canvas.
     auto useDisplayList = m_usesDisplayListDrawing.value_or(document().settings().displayListDrawingEnabled()) ? ShouldUseDisplayList::Yes : ShouldUseDisplayList::No;
 

Modified: trunk/Source/WebCore/html/HTMLCanvasElement.h (280019 => 280020)


--- trunk/Source/WebCore/html/HTMLCanvasElement.h	2021-07-18 15:41:37 UTC (rev 280019)
+++ trunk/Source/WebCore/html/HTMLCanvasElement.h	2021-07-18 19:18:20 UTC (rev 280020)
@@ -123,6 +123,7 @@
     SecurityOrigin* securityOrigin() const final;
 
     bool shouldAccelerate(const IntSize&) const;
+    bool shouldAccelerate(unsigned area) const;
 
     WEBCORE_EXPORT void setUsesDisplayListDrawing(bool);
     WEBCORE_EXPORT void setTracksDisplayListReplay(bool);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to