Reland "Add isfinite checks to Skia Mojo boundaries" [chromium/src : main]

[Juan Mojica (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi, Kaylee Lubick, Nick Yamane and Thomas Anderson

Juan Mojica voted and added 1 comment

Votes added by Juan Mojica

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Juan Mojica . resolved

c/b/r/lens/* LGTM!

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Kaylee Lubick
• Nick Yamane
• Thomas Anderson

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6c0633c3f867a7f2d21dfcc57199977afe7021ee

Gerrit-Change-Number: 7913477

Gerrit-PatchSet: 1

Gerrit-Owner: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Reviewer: Juan Mojica <juanm...@google.com>

Gerrit-Reviewer: Nick Yamane <nick...@igalia.com>

Gerrit-Reviewer: Thomas Anderson <thomasa...@chromium.org>

Gerrit-CC: Chromium IPC Reviews <chrome-ip...@google.com>

Gerrit-CC: Florin Malita <fma...@google.com>

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: gwsq

Gerrit-Attention: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Attention: Thomas Anderson <thomasa...@chromium.org>

Gerrit-Attention: Nick Yamane <nick...@igalia.com>

Gerrit-Attention: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Comment-Date: Wed, 10 Jun 2026 18:13:28 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Thomas Anderson (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi, Kaylee Lubick and Nick Yamane

Thomas Anderson voted and added 1 comment

Votes added by Thomas Anderson

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Thomas Anderson . resolved

ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc lgtm

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Kaylee Lubick
• Nick Yamane

Gerrit-Attention: Nick Yamane <nick...@igalia.com>

Gerrit-Attention: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Comment-Date: Wed, 10 Jun 2026 20:22:58 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Kaylee Lubick (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi, Juan Mojica, Nick Yamane, Stefan Zager and Thomas Anderson

Kaylee Lubick voted and added 1 comment

Votes added by Kaylee Lubick

Auto-Submit	+1
Commit-Queue	+1

1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Kaylee Lubick . resolved

Ok, ready for a reland after fixing AXObject

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Juan Mojica
• Nick Yamane
• Stefan Zager
• Thomas Anderson

Submit Requirements:

• Code-Coverage
• Code-Owners

• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6c0633c3f867a7f2d21dfcc57199977afe7021ee

Gerrit-Change-Number: 7913477

Gerrit-PatchSet: 6

Gerrit-Owner: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Reviewer: Juan Mojica <juanm...@google.com>

Gerrit-Reviewer: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Reviewer: Nick Yamane <nick...@igalia.com>

Gerrit-Reviewer: Stefan Zager <sza...@chromium.org>

Gerrit-Reviewer: Thomas Anderson <thomasa...@chromium.org>

Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>

Gerrit-CC: Akihiro Ota <akihi...@chromium.org>

Gerrit-CC: Chromium IPC Reviews <chrome-ip...@google.com>

Gerrit-CC: Florin Malita <fma...@google.com>

Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>

Gerrit-CC: Nektarios Paisios <nek...@chromium.org>

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: gwsq

Gerrit-Attention: Stefan Zager <sza...@chromium.org>

Gerrit-Attention: Thomas Anderson <thomasa...@chromium.org>

Gerrit-Attention: Juan Mojica <juanm...@google.com>

Gerrit-Attention: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Attention: Nick Yamane <nick...@igalia.com>

Gerrit-Comment-Date: Fri, 12 Jun 2026 18:48:52 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Juan Mojica (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi, Kaylee Lubick and Stefan Zager

Juan Mojica voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Kaylee Lubick
• Stefan Zager

Submit Requirements:

• Code-Coverage
• Code-Owners

• Code-Review
• Review-Enforcement

Gerrit-Attention: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Attention: Stefan Zager <sza...@chromium.org>

Gerrit-Attention: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Comment-Date: Fri, 12 Jun 2026 19:04:23 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[Stefan Zager (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi and Kaylee Lubick

Stefan Zager added 1 comment

File third_party/blink/renderer/modules/accessibility/ax_object.cc

Line 1347, Patchset 6 (Latest): if (!std::isfinite(bounds_in_container.x())) {

Stefan Zager . unresolved

Can/should this code be moved into `GetRelativeBounds()`?

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Kaylee Lubick

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• No-Unresolved-Comments
• Review-Enforcement

Gerrit-Attention: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Comment-Date: Fri, 12 Jun 2026 19:21:21 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Kaylee Lubick (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi, Lucas Radaelli and Stefan Zager

Kaylee Lubick added 1 comment

File third_party/blink/renderer/modules/accessibility/ax_object.cc

Line 1347, Patchset 6 (Latest): if (!std::isfinite(bounds_in_container.x())) {

Stefan Zager . unresolved

Can/should this code be moved into `GetRelativeBounds()`?

Kaylee Lubick

I'm not sure about that. GetRelativeBounds is a virtual and has (currently) 3 implementations. It felt like a smaller change to just put the sanitization here (once). PopulateAXRelativeBounds is also called in only 2 places the Action and the serialization path, GetRelativeBounds is called in more places. In short, this seems like a smaller blast radius.

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Lucas Radaelli
• Stefan Zager

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6c0633c3f867a7f2d21dfcc57199977afe7021ee

Gerrit-Change-Number: 7913477

Gerrit-PatchSet: 6

Gerrit-Owner: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Reviewer: Juan Mojica <juanm...@google.com>

Gerrit-Reviewer: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Reviewer: Lucas Radaelli <lucasr...@google.com>

Gerrit-Reviewer: Nick Yamane <nick...@igalia.com>

Gerrit-Reviewer: Thomas Anderson <thomasa...@chromium.org>

Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>

Gerrit-CC: Akihiro Ota <akihi...@chromium.org>

Gerrit-CC: Chromium IPC Reviews <chrome-ip...@google.com>

Gerrit-CC: Florin Malita <fma...@google.com>

Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>

Gerrit-CC: Nektarios Paisios <nek...@chromium.org>

Gerrit-CC: Stefan Zager <sza...@chromium.org>

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: gwsq

Gerrit-Attention: Stefan Zager <sza...@chromium.org>

Gerrit-Attention: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Attention: Lucas Radaelli <lucasr...@google.com>

Gerrit-Comment-Date: Mon, 15 Jun 2026 12:41:34 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Stefan Zager <sza...@chromium.org>

[Giovanni Ortuno Urquidi (Gerrit)]

Attention needed from Kaylee Lubick, Lucas Radaelli and Stefan Zager

Giovanni Ortuno Urquidi added 1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Giovanni Ortuno Urquidi . resolved

mojo slgtm

Open in Gerrit

Related details

Attention is currently required from:

• Kaylee Lubick
• Lucas Radaelli
• Stefan Zager

Gerrit-Attention: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Attention: Stefan Zager <sza...@chromium.org>

Gerrit-Attention: Lucas Radaelli <lucasr...@google.com>

Gerrit-Comment-Date: Mon, 15 Jun 2026 13:50:35 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Giovanni Ortuno Urquidi (Gerrit)]

Attention needed from Kaylee Lubick, Lucas Radaelli and Stefan Zager

Giovanni Ortuno Urquidi voted Code-Review+1

Code-Review

+1

Gerrit-Comment-Date: Mon, 15 Jun 2026 13:50:46 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[Lucas Radaelli (Gerrit)]

Attention needed from Kaylee Lubick and Stefan Zager

Lucas Radaelli added 2 comments

File third_party/blink/renderer/modules/accessibility/ax_object.cc

Line 1365, Patchset 6 (Latest): goto done;

Lucas Radaelli . unresolved

why goto is necessary here? can we break?

File third_party/blink/renderer/modules/accessibility/ax_object_test.cc

Line 2547, Patchset 6 (Latest): if (bounds.transform) {

Lucas Radaelli . unresolved

is this test actually running? Can we make it more explicit / detemrinistic? this if should not be necessary here. if the test is testing what it is supposed to test, the values would be infinite and we would be checking the identity matrix.

Open in Gerrit

Related details

Attention is currently required from:

• Kaylee Lubick
• Stefan Zager

Gerrit-Comment-Date: Mon, 15 Jun 2026 22:13:42 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Kaylee Lubick (Gerrit)]

Attention needed from Lucas Radaelli and Stefan Zager

Kaylee Lubick voted and added 2 comments

Votes added by Kaylee Lubick

Auto-Submit	+1
Commit-Queue	+1

2 comments

File third_party/blink/renderer/modules/accessibility/ax_object.cc

Line 1365, Patchset 6: goto done;

Lucas Radaelli . resolved

why goto is necessary here? can we break?

Kaylee Lubick

There's a nested loop, so AFAIK, a goto is the cleanest way to do that.

File third_party/blink/renderer/modules/accessibility/ax_object_test.cc

Line 2547, Patchset 6: if (bounds.transform) {

Lucas Radaelli . resolved

is this test actually running? Can we make it more explicit / detemrinistic? this if should not be necessary here. if the test is testing what it is supposed to test, the values would be infinite and we would be checking the identity matrix.

Kaylee Lubick

Good catch. This was left over from earlier. Also fixed below.

Open in Gerrit

Related details

Attention is currently required from:

• Lucas Radaelli
• Stefan Zager

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6c0633c3f867a7f2d21dfcc57199977afe7021ee

Gerrit-Change-Number: 7913477

Gerrit-PatchSet: 7

Gerrit-Owner: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Reviewer: Juan Mojica <juanm...@google.com>

Gerrit-Reviewer: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Reviewer: Lucas Radaelli <lucasr...@google.com>

Gerrit-Reviewer: Nick Yamane <nick...@igalia.com>

Gerrit-Reviewer: Thomas Anderson <thomasa...@chromium.org>

Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>

Gerrit-CC: Akihiro Ota <akihi...@chromium.org>

Gerrit-CC: Chromium IPC Reviews <chrome-ip...@google.com>

Gerrit-CC: Florin Malita <fma...@google.com>

Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>

Gerrit-CC: Nektarios Paisios <nek...@chromium.org>

Gerrit-CC: Stefan Zager <sza...@chromium.org>

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: gwsq

Gerrit-Attention: Stefan Zager <sza...@chromium.org>

Gerrit-Attention: Lucas Radaelli <lucasr...@google.com>

Gerrit-Comment-Date: Tue, 16 Jun 2026 13:16:33 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

Comment-In-Reply-To: Lucas Radaelli <lucasr...@google.com>

[Lucas Radaelli (Gerrit)]

Attention needed from Kaylee Lubick and Stefan Zager

Lucas Radaelli voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

• Kaylee Lubick
• Stefan Zager

Submit Requirements:

• Code-Coverage
• Code-Owners

Gerrit-Attention: Kaylee Lubick <kjlu...@chromium.org>

Gerrit-Attention: Stefan Zager <sza...@chromium.org>

Gerrit-Comment-Date: Tue, 16 Jun 2026 22:42:57 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[Kaylee Lubick (Gerrit)]

Attention needed from Stefan Zager

Kaylee Lubick voted and added 1 comment

Votes added by Kaylee Lubick

Commit-Queue

+2

1 comment

File third_party/blink/renderer/modules/accessibility/ax_object.cc

Line 1347, Patchset 6: if (!std::isfinite(bounds_in_container.x())) {

Stefan Zager . resolved

Can/should this code be moved into `GetRelativeBounds()`?

Kaylee Lubick

I'm not sure about that. GetRelativeBounds is a virtual and has (currently) 3 implementations. It felt like a smaller change to just put the sanitization here (once). PopulateAXRelativeBounds is also called in only 2 places the Action and the serialization path, GetRelativeBounds is called in more places. In short, this seems like a smaller blast radius.

Kaylee Lubick

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:

• Stefan Zager

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• Review-Enforcement

Gerrit-Attention: Stefan Zager <sza...@chromium.org>

Gerrit-Comment-Date: Tue, 23 Jun 2026 12:00:42 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

Comment-In-Reply-To: Kaylee Lubick <kjlu...@chromium.org>

Comment-In-Reply-To: Stefan Zager <sza...@chromium.org>

[Chromium LUCI CQ (Gerrit)]

Chromium LUCI CQ submitted the change

Change information

Commit message:

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 <ort...@chromium.org>
> Reviewed-by: Thomas Anderson <thomasa...@chromium.org>
> Reviewed-by: Juan Mojica <juanm...@google.com>
> Auto-Submit: Kaylee Lubick <kjlu...@chromium.org>
> Commit-Queue: Thomas Anderson <thomasa...@chromium.org>
> 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 <juanm...@google.com>

Auto-Submit: Kaylee Lubick <kjlu...@chromium.org>

Commit-Queue: Kaylee Lubick <kjlu...@chromium.org>

Reviewed-by: Giovanni Ortuno Urquidi <ort...@chromium.org>

Reviewed-by: Lucas Radaelli <lucasr...@google.com>

Cr-Commit-Position: refs/heads/main@{#1650956}

Files:

• M chrome/browser/resources/lens/overlay/post_selection_renderer.ts
• M chrome/browser/resources/lens/overlay/region_selection.ts
• M skia/public/mojom/hdr_metadata_mojom_traits.h
• M skia/public/mojom/skcolor4f_mojom_traits.h
• M skia/public/mojom/skcolorspace_mojom_traits.h
• M skia/public/mojom/skcolorspace_primaries_mojom_traits.h
• M skia/public/mojom/test/mojom_traits_unittest.cc
• M third_party/blink/renderer/modules/accessibility/ax_object.cc
• M third_party/blink/renderer/modules/accessibility/ax_object_test.cc
• M ui/gfx/geometry/mojom/geometry_mojom_traits.h
• M ui/gfx/geometry/mojom/geometry_mojom_traits_unittest.cc
• M ui/gfx/mojom/color_space_mojom_traits.cc
• M ui/gfx/mojom/mojom_traits_unittest.cc
• M ui/gfx/mojom/transform_mojom_traits.h
• M ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc

Change size: L

Delta: 15 files changed, 572 insertions(+), 153 deletions(-)

Branch: refs/heads/main

Submit Requirements:

• Code-Review: +1 by Giovanni Ortuno Urquidi, +1 by Lucas Radaelli, +1 by Juan Mojica

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: merged

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6c0633c3f867a7f2d21dfcc57199977afe7021ee

Gerrit-Change-Number: 7913477

Gerrit-PatchSet: 8