From 465ce88b6de3c453b1bc5cb81a488ba6ec2f1b1d Mon Sep 17 00:00:00 2001
From: Kaylee Lubick <[email protected]>
Date: Tue, 23 Jun 2026 06:45:00 -0700
Subject: [PATCH] Reland "Add isfinite checks to Skia Mojo boundaries"

This is a reland of commit c110228380f30e0a3588d07ad0b691ecf1c41659

The reason we had to revert was because a few layout tests failed
in the force accessibility build. [1] That code path didn't prevent
infinities or NaNs from getting to the Mojo layer. I think it
should be squelching those, otherwise those values could cause
problems after serialization. Thus, I fixed the place in AXObject
where those problematic values seemed to sneak in. I also added
a unit test to make this easier to find.

While I was taking a second look, I added [[unlikely]] to the
mojo checks to help the compiler write better code for these checks.

Original change's description:
> Add isfinite checks to Skia Mojo boundaries
>
> The linked bug refers to NaNs sneaking in where they don't belong
> so I fixed those in the one spot listed in the bugs and other
> related spots. I added a unit test to verify the original buggy
> behavior was fixed.
>
> This deletes some tests that were added to make sure NaNs don't
> make it into Wayland, but because we are adding them here, they've
> been superceded and can be removed.
>
> The changes to lens/overlay find cases where we were sometimes
> dividing by zero, which put NaNs in the rects. This catches those
> more gracefully, fixing some of the failing tests.
>
> Bug: 500390256
> Fixed: 500390256
> Bug: 520300213
> Change-Id: I181112dc4bf8a77e242e95ffc3a31cfc9917c321
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7897559
> Reviewed-by: Giovanni Ortuno Urquidi <[email protected]>
> Reviewed-by: Thomas Anderson <[email protected]>
> Reviewed-by: Juan Mojica <[email protected]>
> Auto-Submit: Kaylee Lubick <[email protected]>
> Commit-Queue: Thomas Anderson <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1644135}

[1] https://ci.chromium.org/ui/p/chromium/builders/ci/linux-blink-web-tests-force-accessibility-rel/47693/overview

Bug: 500390256
Bug: 520300213
Change-Id: I6c0633c3f867a7f2d21dfcc57199977afe7021ee
Cq-Include-Trybots: luci.chromium.try:linux-blink-web-tests-force-accessibility-rel
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7913477
Reviewed-by: Juan Mojica <[email protected]>
Auto-Submit: Kaylee Lubick <[email protected]>
Commit-Queue: Kaylee Lubick <[email protected]>
Reviewed-by: Giovanni Ortuno Urquidi <[email protected]>
Reviewed-by: Lucas Radaelli <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1650956}
---

diff --git a/chrome/browser/resources/lens/overlay/post_selection_renderer.ts b/chrome/browser/resources/lens/overlay/post_selection_renderer.ts
index 9d5f8e3..8571e377 100644
--- a/chrome/browser/resources/lens/overlay/post_selection_renderer.ts
+++ b/chrome/browser/resources/lens/overlay/post_selection_renderer.ts
@@ -565,7 +565,8 @@
   }
 
   handleGestureDrag(event: GestureEvent) {
-    if (!this.selectionOverlayRect) {
+    if (!this.selectionOverlayRect || this.selectionOverlayRect.width <= 0 ||
+        this.selectionOverlayRect.height <= 0) {
       return;
     }
 
@@ -687,7 +688,7 @@
     }
 
     const imageBounds = this.selectionOverlayRect;
-    if (!imageBounds) {
+    if (!imageBounds || imageBounds.width <= 0 || imageBounds.height <= 0) {
       return;
     }
     const normalizedMinBoxWidth = MIN_BOX_SIZE_PX / imageBounds.width;
diff --git a/chrome/browser/resources/lens/overlay/region_selection.ts b/chrome/browser/resources/lens/overlay/region_selection.ts
index ee62584f..cd5bd5b 100644
--- a/chrome/browser/resources/lens/overlay/region_selection.ts
+++ b/chrome/browser/resources/lens/overlay/region_selection.ts
@@ -577,6 +577,14 @@
   private getNormalizedCenterRotatedBoxFromDrag(gesture: GestureEvent):
       CenterRotatedBox {
     const parentRect = this.selectionOverlayRect;
+    if (parentRect.width <= 0 || parentRect.height <= 0) {
+      return {
+        box: {x: 0, y: 0, width: 0, height: 0},
+        rotation: 0,
+        coordinateType: CenterRotatedBox_CoordinateType.kNormalized,
+      };
+    }
+
     // Get coordinates relative to the region selection bounds
     const relativeDragStart = getRelativeCoordinate(
         {x: gesture.startX, y: gesture.startY}, parentRect);
@@ -624,6 +632,14 @@
   private getPostSelectionRegionFromDrag(gesture: GestureEvent):
       PostSelectionBoundingBox {
     const parentRect = this.selectionOverlayRect;
+    if (parentRect.width <= 0 || parentRect.height <= 0) {
+      return {
+        top: 0,
+        left: 0,
+        width: 0,
+        height: 0,
+      };
+    }
 
     // Get coordinates relative to the region selection bounds
     const relativeDragStart = getRelativeCoordinate(
@@ -651,6 +667,15 @@
   private getNormalizedRectangleFromTap(gesture: GestureEvent):
       NormalizedRectangle {
     const parentRect = this.selectionOverlayRect;
+    if (parentRect.width <= 0 || parentRect.height <= 0) {
+      return {
+        top: 0,
+        left: 0,
+        center: {x: 0.5, y: 0.5},
+        width: 1,
+        height: 1,
+      };
+    }
     // The size of the canvas relative to the size of the viewport.
     const scaleFactor = Math.min(
         parentRect.height / window.innerHeight,
diff --git a/skia/public/mojom/hdr_metadata_mojom_traits.h b/skia/public/mojom/hdr_metadata_mojom_traits.h
index 2ae9762..eb4d4af 100644
--- a/skia/public/mojom/hdr_metadata_mojom_traits.h
+++ b/skia/public/mojom/hdr_metadata_mojom_traits.h
@@ -2,8 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef SKIA_PUBLIC_MOJOM_HDR_METADATA_MOJOM_TRAITS_H_
-#define SKIA_PUBLIC_MOJOM_HDR_METADATA_MOJOM_TRAITS_H_
+#ifndef SKIA_PUBLIC_MOJOM_HDR_METADATA_MO_TRAITS_H_
+#define SKIA_PUBLIC_MOJOM_HDR_METADATA_MO_TRAITS_H_
+
+#include <cmath>
 
 #include "skia/public/mojom/hdr_metadata.mojom-shared.h"
 #include "skia/public/mojom/skcolorspace_primaries_mojom_traits.h"
@@ -23,6 +25,10 @@
 
   static bool Read(skia::mojom::SkHdrContentLightLevelInformationDataView data,
                    skhdr::ContentLightLevelInformation* out) {
+    if (!std::isfinite(data.max_cll()) || !std::isfinite(data.max_fall()))
+        [[unlikely]] {
+      return false;
+    }
     out->fMaxCLL = data.max_cll();
     out->fMaxFALL = data.max_fall();
     return true;
@@ -45,6 +51,10 @@
 
   static bool Read(skia::mojom::SkHdrMasteringDisplayColorVolumeDataView data,
                    skhdr::MasteringDisplayColorVolume* out) {
+    if (!std::isfinite(data.max_luminance()) ||
+        !std::isfinite(data.min_luminance())) [[unlikely]] {
+      return false;
+    }
     if (!data.ReadPrimaries(&out->fDisplayPrimaries)) {
       return false;
     }
@@ -72,6 +82,10 @@
 
   static bool Read(skia::mojom::SkHdrAgtmGainCurveControlPointDataView data,
                    skhdr::AdaptiveGlobalToneMap::GainCurve::ControlPoint* out) {
+    if (!std::isfinite(data.x()) || !std::isfinite(data.y()) ||
+        !std::isfinite(data.m())) [[unlikely]] {
+      return false;
+    }
     out->fX = data.x();
     out->fY = data.y();
     out->fM = data.m();
@@ -127,6 +141,12 @@
 
   static bool Read(skia::mojom::SkHdrAgtmComponentMixingFunctionDataView data,
                    skhdr::AdaptiveGlobalToneMap::ComponentMixingFunction* out) {
+    if (!std::isfinite(data.red()) || !std::isfinite(data.green()) ||
+        !std::isfinite(data.blue()) || !std::isfinite(data.max()) ||
+        !std::isfinite(data.min()) || !std::isfinite(data.component()))
+        [[unlikely]] {
+      return false;
+    }
     out->fRed = data.red();
     out->fGreen = data.green();
     out->fBlue = data.blue();
@@ -178,6 +198,9 @@
 
   static bool Read(skia::mojom::SkHdrAgtmAlternateImageDataView data,

diff --git a/skia/public/mojom/test/mojom_traits_unittest.cc b/skia/public/mojom/test/mojom_traits_unittest.cc
index a0d3278..dc2af641 100644
--- a/skia/public/mojom/test/mojom_traits_unittest.cc
+++ b/skia/public/mojom/test/mojom_traits_unittest.cc
@@ -15,6 +15,8 @@
 #include "skia/public/mojom/hdr_metadata_mojom_traits.h"
 #include "skia/public/mojom/image_info.mojom-shared.h"
 #include "skia/public/mojom/image_info.mojom.h"
+#include "skia/public/mojom/skcolor4f.mojom.h"
+#include "skia/public/mojom/skcolor4f_mojom_traits.h"
 #include "skia/public/mojom/skcolorspace.mojom.h"
 #include "skia/public/mojom/skcolorspace_mojom_traits.h"
 #include "skia/public/mojom/skcolorspace_primaries.mojom.h"
@@ -37,6 +39,20 @@
 namespace skia {
 namespace {
 
+// A helper to construct a skia.mojom.SkColor4f without using StructTraits
+// to bypass checks on the sending/serialization side.
+skia::mojom::SkColor4fPtr ConstructSkColor4f(float r,
+                                             float g,
+                                             float b,
+                                             float a) {
+  auto mojom_color = skia::mojom::SkColor4f::New();
+  mojom_color->r = r;
+  mojom_color->g = g;
+  mojom_color->b = b;
+  mojom_color->a = a;
+  return mojom_color;
+}
+
 // A helper to construct a skia.mojom.BitmapN32 without using StructTraits
 // to bypass checks on the sending/serialization side.
 mojo::StructPtr<skia::mojom::BitmapN32> ConstructBitmapN32(
@@ -205,6 +221,61 @@
   EXPECT_TRUE(in_p == out_p);
 }
 
+TEST(StructTraitsTest, SkColor4f) {
+  SkColor4f input = {0.1f, 0.2f, 0.3f, 0.4f};
+  SkColor4f output;
+  ASSERT_TRUE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+      input, output));
+  EXPECT_EQ(input, output);
+
+  const float kNaN = std::numeric_limits<float>::quiet_NaN();
+  const float kInf = std::numeric_limits<float>::infinity();
+
+  // Test NaN in each component.
+  {
+    auto input_nan = ConstructSkColor4f(kNaN, 0.2f, 0.3f, 0.4f);
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+        input_nan, output));
+  }
+  {
+    auto input_nan = ConstructSkColor4f(0.1f, kNaN, 0.3f, 0.4f);
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+        input_nan, output));
+  }
+  {
+    auto input_nan = ConstructSkColor4f(0.1f, 0.2f, kNaN, 0.4f);
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+        input_nan, output));
+  }
+  {
+    auto input_nan = ConstructSkColor4f(0.1f, 0.2f, 0.3f, kNaN);
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+        input_nan, output));
+  }
+
+  // Test Infinity in each component.
+  {
+    auto input_inf = ConstructSkColor4f(kInf, 0.2f, 0.3f, 0.4f);
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+        input_inf, output));
+  }
+  {
+    auto input_inf = ConstructSkColor4f(0.1f, kInf, 0.3f, 0.4f);
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+        input_inf, output));
+  }
+  {
+    auto input_inf = ConstructSkColor4f(0.1f, 0.2f, kInf, 0.4f);
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+        input_inf, output));
+  }
+  {
+    auto input_inf = ConstructSkColor4f(0.1f, 0.2f, 0.3f, kInf);
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<skia::mojom::SkColor4f>(
+        input_inf, output));
+  }
+}
+
 TEST(StructTraitsTest, TileMode) {
   SkTileMode input(SkTileMode::kClamp);
   SkTileMode output;
@@ -572,6 +643,29 @@
   EXPECT_EQ(in.fMaxFALL, out.fMaxFALL);
 }
 
+TEST(StructTraitsTest, SkHdrContentLightLevelInformation_InvalidFloats) {
+  const float kNaN = std::numeric_limits<float>::quiet_NaN();
+  const float kInf = std::numeric_limits<float>::infinity();
+
+  skhdr::ContentLightLevelInformation in;
+  in.fMaxCLL = 1.2f;
+  in.fMaxFALL = 3.4f;
+  skhdr::ContentLightLevelInformation out;
+
+  {
+    skhdr::ContentLightLevelInformation bad = in;
+    bad.fMaxCLL = kNaN;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrContentLightLevelInformation>(bad, out));
+  }
+  {
+    skhdr::ContentLightLevelInformation bad = in;
+    bad.fMaxFALL = kInf;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrContentLightLevelInformation>(bad, out));
+  }
+}
+
 TEST(StructTraitsTest, SkHdrMasteringDisplayColorVolume) {
   skhdr::MasteringDisplayColorVolume in;
   in.fDisplayPrimaries = SkNamedPrimaries::kRec2020;
@@ -589,6 +683,30 @@
             out.fMinimumDisplayMasteringLuminance);
 }
 
+TEST(StructTraitsTest, SkHdrMasteringDisplayColorVolume_InvalidFloats) {
+  const float kNaN = std::numeric_limits<float>::quiet_NaN();
+  const float kInf = std::numeric_limits<float>::infinity();
+
+  skhdr::MasteringDisplayColorVolume in;
+  in.fDisplayPrimaries = SkNamedPrimaries::kRec2020;
+  in.fMaximumDisplayMasteringLuminance = 1.2f;
+  in.fMinimumDisplayMasteringLuminance = 3.4f;
+  skhdr::MasteringDisplayColorVolume out;
+
+  {
+    skhdr::MasteringDisplayColorVolume bad = in;
+    bad.fMaximumDisplayMasteringLuminance = kNaN;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrMasteringDisplayColorVolume>(bad, out));
+  }
+  {
+    skhdr::MasteringDisplayColorVolume bad = in;
+    bad.fMinimumDisplayMasteringLuminance = kInf;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrMasteringDisplayColorVolume>(bad, out));
+  }
+}
+
 TEST(StructTraitsTest, SkHdrAdaptiveGlobalToneMap) {
   skhdr::AdaptiveGlobalToneMap::HeadroomAdaptiveToneMap inHatm;
   inHatm.fBaselineHdrHeadroom = 0.1f,
@@ -684,5 +802,77 @@
                 .fM);
 }
 
+TEST(StructTraitsTest, SkHdrAdaptiveGlobalToneMap_InvalidFloats) {
+  const float kNaN = std::numeric_limits<float>::quiet_NaN();
+  const float kInf = std::numeric_limits<float>::infinity();
+
+  skhdr::AdaptiveGlobalToneMap::HeadroomAdaptiveToneMap inHatm;
+  inHatm.fBaselineHdrHeadroom = 0.1f,
+  inHatm.fGainApplicationSpacePrimaries = SkNamedPrimaries::kRec2020;
+  inHatm.fAlternateImages = {
+      {
+          .fHdrHeadroom = 0.2,
+          .fColorGainFunction =
+              {
+                  .fComponentMixing =
+                      {
+                          .fRed = 0.1f,
+                          .fGreen = 0.2f,
+                          .fBlue = 0.3f,
+                          .fMax = 0.4f,
+                          .fMin = 0.5f,
+                          .fComponent = 0.6f,
+                      },
+                  .fGainCurve =
+                      {
+                          .fControlPoints =
+                              {
+                                  {.fX = 0.7f, .fY = 0.8f, .fM = 0.9f},
+                              },
+                      },
+              },
+      },
+  };
+  skhdr::AdaptiveGlobalToneMap in = {
+      .fHdrReferenceWhite = 100.0f,
+      .fHeadroomAdaptiveToneMap = {inHatm},
+  };
+  skhdr::AdaptiveGlobalToneMap out;
+
+  {
+    skhdr::AdaptiveGlobalToneMap bad = in;
+    bad.fHdrReferenceWhite = kNaN;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrAdaptiveGlobalToneMap>(bad, out));
+  }
+  {
+    skhdr::AdaptiveGlobalToneMap bad = in;
+    bad.fHeadroomAdaptiveToneMap->fBaselineHdrHeadroom = kInf;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrAdaptiveGlobalToneMap>(bad, out));
+  }
+  {
+    skhdr::AdaptiveGlobalToneMap bad = in;
+    bad.fHeadroomAdaptiveToneMap->fAlternateImages[0].fHdrHeadroom = kNaN;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrAdaptiveGlobalToneMap>(bad, out));
+  }
+  {
+    skhdr::AdaptiveGlobalToneMap bad = in;
+    bad.fHeadroomAdaptiveToneMap->fAlternateImages[0]
+        .fColorGainFunction.fComponentMixing.fRed = kInf;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrAdaptiveGlobalToneMap>(bad, out));
+  }
+  {
+    skhdr::AdaptiveGlobalToneMap bad = in;
+    bad.fHeadroomAdaptiveToneMap->fAlternateImages[0]
+        .fColorGainFunction.fGainCurve.fControlPoints[0]
+        .fX = kNaN;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<
+                 skia::mojom::SkHdrAdaptiveGlobalToneMap>(bad, out));
+  }
+}
+
 }  // namespace
 }  // namespace skia
diff --git a/third_party/blink/renderer/modules/accessibility/ax_object_test.cc b/third_party/blink/renderer/modules/accessibility/ax_object_test.cc
index bff1989a..0caabc64 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_object_test.cc
+++ b/third_party/blink/renderer/modules/accessibility/ax_object_test.cc
@@ -2527,5 +2527,47 @@
   EXPECT_EQ(ax::mojom::blink::Role::kListBoxOption, owned->RoleValue());
 }
 
+TEST_F(AccessibilityTest, PopulateAXRelativeBoundsSanitizesNonFiniteValues) {
+  // Set up an element with extreme CSS that produces Infinity in transforms.
+  SetBodyInnerHTML(R"HTML(
+    <div id="target" style="transform: scale(calc(1/0))">
+      Extreme Transform
+    </div>
+  )HTML");
+
+  AXObject* target = GetAXObjectByElementId("target");
+  ASSERT_NE(nullptr, target);
+
+  ui::AXRelativeBounds bounds;
+  bool clips_children = false;
+  target->PopulateAXRelativeBounds(bounds, &clips_children);
+
+  // Verify that the transform is sanitized to identity (represented as null).
+  EXPECT_FALSE(bounds.transform);
+
+  // Set up an element with a negative infinity scale, which could produce
+  // negative width/height if not sanitized.
+  SetBodyInnerHTML(R"HTML(
+    <div id="target2" style="transform: scale(calc(log(0)))">
+      Negative Infinity Transform
+    </div>
+  )HTML");
+
+  AXObject* target2 = GetAXObjectByElementId("target2");
+  ASSERT_NE(nullptr, target2);
+
+  target2->PopulateAXRelativeBounds(bounds, &clips_children);
+
+  EXPECT_TRUE(std::isfinite(bounds.bounds.x()));
+  EXPECT_TRUE(std::isfinite(bounds.bounds.y()));
+  EXPECT_TRUE(std::isfinite(bounds.bounds.width()));
+  EXPECT_TRUE(std::isfinite(bounds.bounds.height()));
+  EXPECT_GE(bounds.bounds.width(), 0.0f);
+  EXPECT_GE(bounds.bounds.height(), 0.0f);
+
+  // null means identity
+  EXPECT_FALSE(bounds.transform);
+}
+
 }  // namespace test
 }  // namespace blink
diff --git a/ui/gfx/geometry/mojom/geometry_mojom_traits_unittest.cc b/ui/gfx/geometry/mojom/geometry_mojom_traits_unittest.cc
index 93603d25..2f31b26 100644
--- a/ui/gfx/geometry/mojom/geometry_mojom_traits_unittest.cc
+++ b/ui/gfx/geometry/mojom/geometry_mojom_traits_unittest.cc
@@ -198,4 +198,111 @@
   EXPECT_EQ(input, output);
 }
 
+TEST(GeometryStructTraitsTest, InvalidFloats) {
+  const float nan = std::numeric_limits<float>::quiet_NaN();
+  const float inf = std::numeric_limits<float>::infinity();
+  const double dnan = std::numeric_limits<double>::quiet_NaN();
+  const double dinf = std::numeric_limits<double>::infinity();
... (truncated)