geometry: Propagate NaN in min/max operations

Currently in some geometry classes we are using std::min() and
std::max() over numbers specified in IDL as "unrestricted double",
meaning they could take the special value NaN. These STL helpers
internally use the < operator, as in

    std::min(a, b) = a < b ? a : b.

However, the IEEE 794 < operator always returns false when _either_
operand is NaN, so the result of min(0, NaN) and min(NaN, 0) could,
confusingly, be different.

This is difference is in fact visible through JavaScript. For instance,

    new DOMQuad({ x: NaN }, { x: 0 }, { x: 0 }, { x: 0 }).getBounds().x

gives NaN, but

    new DOMQuad({ x: 0 }, { x: 0 }, { x: 0 }, { x: NaN }).getBounds().x

gives 0.

A similar problem is present for DOMRect/DOMRectReadOnly as well.

This CL implements [1], which is to adopt semantics similar to
JavaScript's Math.min() and Math.max(), which propagates NaN from either
operand. This also aligns our behavior with WebKit.

[1]: https://github.com/w3c/fxtf-drafts/pull/395

Fixed: 1066499
Change-Id: Id9a4282fa00dafcfe9c5616643efbe2eaace411e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129889
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Jinho Bang <jinho.bang@samsung.com>
Cr-Commit-Position: refs/heads/master@{#755373}

third_party/blink/renderer/core/geometry/BUILD.gn

@@ -22,5 +22,6 @@ blink_core_sources("geometry") {
     "dom_rect_list.h",
     "dom_rect_read_only.cc",
     "dom_rect_read_only.h",
+    "geometry_util.h",
   ]
 }

third_party/blink/renderer/core/geometry/dom_quad.cc

@@ -10,6 +10,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/v8_object_builder.h"
 #include "third_party/blink/renderer/core/geometry/dom_point.h"
 #include "third_party/blink/renderer/core/geometry/dom_rect.h"
+#include "third_party/blink/renderer/core/geometry/geometry_util.h"
 
 namespace blink {
 namespace {
@@ -53,12 +54,14 @@ class DOMQuadPoint final : public DOMPoint {
   WeakMember<DOMQuad> quad_;
 };
 
-double min4(double a, double b, double c, double d) {
-  return std::min(std::min(a, b), std::min(c, d));
+double NanSafeMin4(double a, double b, double c, double d) {
+  using geometry_util::NanSafeMin;
+  return NanSafeMin(NanSafeMin(a, b), NanSafeMin(c, d));
 }
 
-double max4(double a, double b, double c, double d) {
-  return std::max(std::max(a, b), std::max(c, d));
+double NanSafeMax4(double a, double b, double c, double d) {
+  using geometry_util::NanSafeMax;
+  return NanSafeMax(NanSafeMax(a, b), NanSafeMax(c, d));
 }
 
 }  // namespace
@@ -90,10 +93,10 @@ DOMRect* DOMQuad::getBounds() {
 }
 
 void DOMQuad::CalculateBounds() {
-  x_ = min4(p1()->x(), p2()->x(), p3()->x(), p4()->x());
-  y_ = min4(p1()->y(), p2()->y(), p3()->y(), p4()->y());
-  width_ = max4(p1()->x(), p2()->x(), p3()->x(), p4()->x()) - x_;
-  height_ = max4(p1()->y(), p2()->y(), p3()->y(), p4()->y()) - y_;
+  x_ = NanSafeMin4(p1()->x(), p2()->x(), p3()->x(), p4()->x());
+  y_ = NanSafeMin4(p1()->y(), p2()->y(), p3()->y(), p4()->y());
+  width_ = NanSafeMax4(p1()->x(), p2()->x(), p3()->x(), p4()->x()) - x_;
+  height_ = NanSafeMax4(p1()->y(), p2()->y(), p3()->y(), p4()->y()) - y_;
   needs_bounds_calculation_ = false;
 }
 

third_party/blink/renderer/core/geometry/dom_rect_read_only.h

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_GEOMETRY_DOM_RECT_READ_ONLY_H_
 
 #include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/blink/renderer/core/geometry/geometry_util.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
 #include "third_party/blink/renderer/platform/geometry/float_rect.h"
 #include "third_party/blink/renderer/platform/geometry/int_rect.h"
@@ -35,10 +36,10 @@ class CORE_EXPORT DOMRectReadOnly : public ScriptWrappable {
   double width() const { return width_; }
   double height() const { return height_; }
 
-  double top() const { return std::min(y_, y_ + height_); }
-  double right() const { return std::max(x_, x_ + width_); }
-  double bottom() const { return std::max(y_, y_ + height_); }
-  double left() const { return std::min(x_, x_ + width_); }
+  double top() const { return geometry_util::NanSafeMin(y_, y_ + height_); }
+  double right() const { return geometry_util::NanSafeMax(x_, x_ + width_); }
+  double bottom() const { return geometry_util::NanSafeMax(y_, y_ + height_); }
+  double left() const { return geometry_util::NanSafeMin(x_, x_ + width_); }
 
   ScriptValue toJSONForBinding(ScriptState*) const;
 

third_party/blink/renderer/core/geometry/geometry_util.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_GEOMETRY_GEOMETRY_UTIL_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_GEOMETRY_GEOMETRY_UTIL_H_
+
+#include <algorithm>
+#include <cmath>
+#include <limits>
+
+namespace blink {
+
+namespace geometry_util {
+
+// Returns the minimum of |a| and |b|. If either operand is NaN, then NaN is
+// returned, consistent with Math.min() in JavaScript.
+inline double NanSafeMin(double a, double b) {
+  if (std::isnan(a) || std::isnan(b)) {
+    return std::numeric_limits<double>::quiet_NaN();
+  }
+  return std::min(a, b);
+}
+
+// Returns the maximum of |a| and |b|. If either operand is NaN, then NaN is
+// returned, consistent with Math.max() in JavaScript.
+inline double NanSafeMax(double a, double b) {
+  if (std::isnan(a) || std::isnan(b)) {
+    return std::numeric_limits<double>::quiet_NaN();
+  }
+  return std::max(a, b);
+}
+
+}  // namespace geometry_util
+
+}  // namespace blink
+
+#endif

third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py

@@ -345,6 +345,7 @@ _CONFIG = [
             'event_handling_util::.+',
             'event_util::.+',
             'file_error::.+',
+            'geometry_util::.+',
             'inspector_\\w+_event::.+',
             'inspector_async_task::.+',
             'inspector_set_layer_tree_id::.+',

third_party/blink/web_tests/external/wpt/css/geometry/DOMQuad-001.html

@@ -67,7 +67,7 @@
                     p2: { x: NaN, y: -Infinity, z: 0, w: 1 },
                     p3: { x: NaN, y: NaN, z: 0, w: 1 },
                     p4: { x: -Infinity, y: NaN, z: 0, w: 1 },
-                    bounds: { x: -Infinity, y: -Infinity, width: NaN, height: NaN } },
+                    bounds: { x: NaN, y: NaN, width: NaN, height: NaN } },
                 'fromRect() method on DOMQuad with Infinity');
 
         checkDOMQuad(function() { return new DOMQuad(new DOMRect()); }, initial, 'testConstructor8');

third_party/blink/web_tests/external/wpt/css/geometry/DOMQuad-nan.html

+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>DOMRect's handling of NaN in top/bottom/left/right</title>
+<link rel=help href="https://drafts.fxtf.org/geometry/#dom-domrectreadonly-domrect-top">
+<link rel=help href="https://drafts.fxtf.org/geometry/#dom-domrectreadonly-domrect-bottom">
+<link rel=help href="https://drafts.fxtf.org/geometry/#dom-domrectreadonly-domrect-left">
+<link rel=help href="https://drafts.fxtf.org/geometry/#dom-domrectreadonly-domrect-right">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+"use strict";
+
+for (const i of [1, 2, 3, 4]) {
+  for (const comp of ["x", "y"]) {
+    test(() => {
+      const args = [{ x: 0, y: 0 }, { x: 0, y: 0 }, { x: 0, y: 0 }, { x: 0, y: 0 }];
+      args[i - 1][comp] = NaN;
+      const quad = new DOMQuad(...args);
+      const bounds = quad.getBounds();
+      if (comp === "x") {
+        assert_equals(bounds.x, NaN, "x coordinate");
+        assert_equals(bounds.y, 0, "y coordinate");
+        assert_equals(bounds.width, NaN, "width");
+        assert_equals(bounds.height, 0, "height");
+      } else {
+        assert_equals(bounds.x, 0, "x coordinate");
+        assert_equals(bounds.y, NaN, "y coordinate");
+        assert_equals(bounds.width, 0, "width");
+        assert_equals(bounds.height, NaN, "height");
+      }
+    }, `Setting DOMQuad's p${i}.${comp} to NaN`);
+  }
+}
+</script>

third_party/blink/web_tests/external/wpt/css/geometry/DOMRect-nan.html

+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>DOMQuad's handling of NaN in getBounds()</title>
+<link rel=help href="https://drafts.fxtf.org/geometry/#dom-domquad-getbounds">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+"use strict";
+
+const testCases = [
+  {
+    name: "x coordinate is NaN",
+    idx: 0,
+    exp: {
+      x: NaN,
+      y: 0,
+      width: 0,
+      height: 0,
+      top: 0,
+      bottom: 0,
+      left: NaN,
+      right: NaN
+    }
+  },
+  {
+    name: "y coordinate is NaN",
+    idx: 1,
+    exp: {
+      x: 0,
+      y: NaN,
+      width: 0,
+      height: 0,
+      top: NaN,
+      bottom: NaN,
+      left: 0,
+      right: 0
+    }
+  },
+  {
+    name: "width is NaN",
+    idx: 2,
+    exp: {
+      x: 0,
+      y: 0,
+      width: NaN,
+      height: 0,
+      top: 0,
+      bottom: 0,
+      left: NaN,
+      right: NaN
+    }
+  },
+  {
+    name: "height is NaN",
+    idx: 3,
+    exp: {
+      x: 0,
+      y: 0,
+      width: 0,
+      height: NaN,
+      top: NaN,
+      bottom: NaN,
+      left: 0,
+      right: 0
+    }
+  },
+];
+
+for (const Rect of [DOMRect, DOMRectReadOnly]) {
+  for (const testCase of testCases) {
+    test(() => {
+      const args = [0, 0, 0, 0];
+      args[testCase.idx] = NaN;
+      const rect = new Rect(...args);
+      assert_object_equals(rect.toJSON(), testCase.exp);
+    }, `${Rect.name}'s ${testCase.name}`);
+  }
+}
+</script>