[SVG] Make `SVGPropertyBase::Add()` return bool to signal failure [chromium/src : main]

0 views
Skip to first unread message

Virali Purbey (Gerrit)

unread,
2:21 AM (12 hours ago) 2:21 AM
to Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Olga Gerchikov, Stephen Chenney, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org
Attention needed from Virali Purbey

Message from Virali Purbey

Set Ready For Review

Open in Gerrit

Related details

Attention is currently required from:
  • Virali Purbey
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I8cf7798b904c7fa64c027e36cf44ad5e0a370436
Gerrit-Change-Number: 7806138
Gerrit-PatchSet: 3
Gerrit-Owner: Virali Purbey <virali...@microsoft.com>
Gerrit-Reviewer: Virali Purbey <virali...@microsoft.com>
Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
Gerrit-CC: Fredrik Söderquist <f...@opera.com>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: Stephen Chenney <sche...@chromium.org>
Gerrit-Attention: Virali Purbey <virali...@microsoft.com>
Gerrit-Comment-Date: Tue, 05 May 2026 06:21:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Fredrik Söderquist (Gerrit)

unread,
8:08 AM (6 hours ago) 8:08 AM
to Virali Purbey, Vinay Singh, Divyansh Mangal, Philip Rogers, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Olga Gerchikov, Stephen Chenney, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org
Attention needed from Divyansh Mangal, Vinay Singh and Virali Purbey

Fredrik Söderquist voted and added 3 comments

Votes added by Fredrik Söderquist

Code-Review+1

3 comments

Patchset-level comments
File-level comment, Patchset 3 (Latest):
Fredrik Söderquist . resolved

LGTM w/ nits

File third_party/blink/renderer/core/svg/svg_path.cc
Line 144, Patchset 3 (Latest): // Empty other (e.g. absent from in by-animation) acts as zero: 0 + by = by.
Fredrik Söderquist . unresolved

...acts as neutral value

Line 154, Patchset 3 (Latest): if (result) {
Fredrik Söderquist . unresolved
Could you reverse this branch instead?
```
auto result = ...
if (!result) {
  return false;
}
path_value_ = ...;
return true;
```
Open in Gerrit

Related details

Attention is currently required from:
  • Divyansh Mangal
  • Vinay Singh
  • Virali Purbey
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I8cf7798b904c7fa64c027e36cf44ad5e0a370436
Gerrit-Change-Number: 7806138
Gerrit-PatchSet: 3
Gerrit-Owner: Virali Purbey <virali...@microsoft.com>
Gerrit-Reviewer: Divyansh Mangal <dma...@microsoft.com>
Gerrit-Reviewer: Fredrik Söderquist <f...@opera.com>
Gerrit-Reviewer: Vinay Singh <vinay...@microsoft.com>
Gerrit-Reviewer: Virali Purbey <virali...@microsoft.com>
Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: Philip Rogers <p...@chromium.org>
Gerrit-CC: Stephen Chenney <sche...@chromium.org>
Gerrit-Attention: Vinay Singh <vinay...@microsoft.com>
Gerrit-Attention: Virali Purbey <virali...@microsoft.com>
Gerrit-Attention: Divyansh Mangal <dma...@microsoft.com>
Gerrit-Comment-Date: Tue, 05 May 2026 12:07:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Virali Purbey (Gerrit)

unread,
11:38 AM (3 hours ago) 11:38 AM
to Fredrik Söderquist, Vinay Singh, Divyansh Mangal, Philip Rogers, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Olga Gerchikov, Stephen Chenney, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org
Attention needed from Divyansh Mangal, Fredrik Söderquist and Vinay Singh

Virali Purbey added 2 comments

File third_party/blink/renderer/core/svg/svg_path.cc
Line 144, Patchset 3: // Empty other (e.g. absent from in by-animation) acts as zero: 0 + by = by.
Fredrik Söderquist . resolved

...acts as neutral value

Virali Purbey

Done

Line 154, Patchset 3: if (result) {
Fredrik Söderquist . resolved
Could you reverse this branch instead?
```
auto result = ...
if (!result) {
  return false;
}
path_value_ = ...;
return true;
```
Virali Purbey

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Divyansh Mangal
  • Fredrik Söderquist
  • Vinay Singh
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I8cf7798b904c7fa64c027e36cf44ad5e0a370436
    Gerrit-Change-Number: 7806138
    Gerrit-PatchSet: 4
    Gerrit-Owner: Virali Purbey <virali...@microsoft.com>
    Gerrit-Reviewer: Divyansh Mangal <dma...@microsoft.com>
    Gerrit-Reviewer: Fredrik Söderquist <f...@opera.com>
    Gerrit-Reviewer: Vinay Singh <vinay...@microsoft.com>
    Gerrit-Reviewer: Virali Purbey <virali...@microsoft.com>
    Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
    Gerrit-CC: Menard, Alexis <alexis...@intel.com>
    Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
    Gerrit-CC: Philip Rogers <p...@chromium.org>
    Gerrit-CC: Stephen Chenney <sche...@chromium.org>
    Gerrit-Attention: Vinay Singh <vinay...@microsoft.com>
    Gerrit-Attention: Fredrik Söderquist <f...@opera.com>
    Gerrit-Attention: Divyansh Mangal <dma...@microsoft.com>
    Gerrit-Comment-Date: Tue, 05 May 2026 15:38:14 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Fredrik Söderquist <f...@opera.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Fredrik Söderquist (Gerrit)

    unread,
    12:05 PM (2 hours ago) 12:05 PM
    to Virali Purbey, Vinay Singh, Divyansh Mangal, Philip Rogers, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Olga Gerchikov, Stephen Chenney, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org
    Attention needed from Divyansh Mangal, Vinay Singh and Virali Purbey

    Fredrik Söderquist voted Code-Review+1

    Code-Review+1
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Divyansh Mangal
    • Vinay Singh
    • Virali Purbey
    Gerrit-Attention: Virali Purbey <virali...@microsoft.com>
    Gerrit-Attention: Divyansh Mangal <dma...@microsoft.com>
    Gerrit-Comment-Date: Tue, 05 May 2026 16:05:12 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Divyansh Mangal (Gerrit)

    unread,
    12:41 PM (2 hours ago) 12:41 PM
    to Virali Purbey, Fredrik Söderquist, Vinay Singh, Philip Rogers, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Olga Gerchikov, Stephen Chenney, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org
    Attention needed from Vinay Singh and Virali Purbey

    Divyansh Mangal voted and added 1 comment

    Votes added by Divyansh Mangal

    Code-Review+1

    1 comment

    Patchset-level comments
    File-level comment, Patchset 5 (Latest):
    Divyansh Mangal . resolved

    LGTM

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Vinay Singh
    • Virali Purbey
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I8cf7798b904c7fa64c027e36cf44ad5e0a370436
      Gerrit-Change-Number: 7806138
      Gerrit-PatchSet: 5
      Gerrit-Owner: Virali Purbey <virali...@microsoft.com>
      Gerrit-Reviewer: Divyansh Mangal <dma...@microsoft.com>
      Gerrit-Reviewer: Fredrik Söderquist <f...@opera.com>
      Gerrit-Reviewer: Vinay Singh <vinay...@microsoft.com>
      Gerrit-Reviewer: Virali Purbey <virali...@microsoft.com>
      Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
      Gerrit-CC: Menard, Alexis <alexis...@intel.com>
      Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
      Gerrit-CC: Philip Rogers <p...@chromium.org>
      Gerrit-CC: Stephen Chenney <sche...@chromium.org>
      Gerrit-Attention: Vinay Singh <vinay...@microsoft.com>
      Gerrit-Attention: Virali Purbey <virali...@microsoft.com>
      Gerrit-Comment-Date: Tue, 05 May 2026 16:41:18 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      open
      diffy

      Virali Purbey (Gerrit)

      unread,
      12:47 PM (1 hour ago) 12:47 PM
      to Divyansh Mangal, Fredrik Söderquist, Vinay Singh, Philip Rogers, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Olga Gerchikov, Stephen Chenney, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org
      Attention needed from Vinay Singh

      Virali Purbey voted Commit-Queue+2

      Commit-Queue+2
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Vinay Singh
      Gerrit-Comment-Date: Tue, 05 May 2026 16:47:00 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      open
      diffy

      Chromium LUCI CQ (Gerrit)

      unread,
      1:13 PM (1 hour ago) 1:13 PM
      to Virali Purbey, Divyansh Mangal, Fredrik Söderquist, Vinay Singh, Philip Rogers, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Olga Gerchikov, Stephen Chenney, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org

      Chromium LUCI CQ submitted the change

      Change information

      Commit message:
      [SVG] Make `SVGPropertyBase::Add()` return bool to signal failure

      Change `Add()` from void to bool so that
      `SVGAnimateElement::CalculateFromAndByValues()` can detect and
      propagate addition failures (e.g. structurally incompatible path
      data), marking the animation invalid.

      `SVGPath::Add()` now returns false on mismatched structure without
      mutation. An empty addend (absent `from` in by-animation) is treated
      as identity. All other property types trivially return true.
      Bug: 483220224
      Change-Id: I8cf7798b904c7fa64c027e36cf44ad5e0a370436
      Reviewed-by: Divyansh Mangal <dma...@microsoft.com>
      Reviewed-by: Fredrik Söderquist <f...@opera.com>
      Commit-Queue: Virali Purbey <virali...@microsoft.com>
      Cr-Commit-Position: refs/heads/main@{#1625537}
      Files:
      • M third_party/blink/renderer/core/svg/properties/svg_property.h
      • M third_party/blink/renderer/core/svg/svg_angle.cc
      • M third_party/blink/renderer/core/svg/svg_angle.h
      • M third_party/blink/renderer/core/svg/svg_animate_element.cc
      • M third_party/blink/renderer/core/svg/svg_animated_color.cc
      • M third_party/blink/renderer/core/svg/svg_animated_color.h
      • M third_party/blink/renderer/core/svg/svg_boolean.cc
      • M third_party/blink/renderer/core/svg/svg_boolean.h
      • M third_party/blink/renderer/core/svg/svg_enumeration.cc
      • M third_party/blink/renderer/core/svg/svg_enumeration.h
      • M third_party/blink/renderer/core/svg/svg_integer.cc
      • M third_party/blink/renderer/core/svg/svg_integer.h
      • M third_party/blink/renderer/core/svg/svg_integer_optional_integer.cc
      • M third_party/blink/renderer/core/svg/svg_integer_optional_integer.h
      • M third_party/blink/renderer/core/svg/svg_length.cc
      • M third_party/blink/renderer/core/svg/svg_length.h
      • M third_party/blink/renderer/core/svg/svg_length_list.cc
      • M third_party/blink/renderer/core/svg/svg_length_list.h
      • M third_party/blink/renderer/core/svg/svg_number.cc
      • M third_party/blink/renderer/core/svg/svg_number.h
      • M third_party/blink/renderer/core/svg/svg_number_list.cc
      • M third_party/blink/renderer/core/svg/svg_number_list.h
      • M third_party/blink/renderer/core/svg/svg_number_optional_number.cc
      • M third_party/blink/renderer/core/svg/svg_number_optional_number.h
      • M third_party/blink/renderer/core/svg/svg_path.cc
      • M third_party/blink/renderer/core/svg/svg_path.h
      • M third_party/blink/renderer/core/svg/svg_point.cc
      • M third_party/blink/renderer/core/svg/svg_point.h
      • M third_party/blink/renderer/core/svg/svg_point_list.cc
      • M third_party/blink/renderer/core/svg/svg_point_list.h
      • M third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.cc
      • M third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.h
      • M third_party/blink/renderer/core/svg/svg_rect.cc
      • M third_party/blink/renderer/core/svg/svg_rect.h
      • M third_party/blink/renderer/core/svg/svg_string.cc
      • M third_party/blink/renderer/core/svg/svg_string.h
      • M third_party/blink/renderer/core/svg/svg_string_list.cc
      • M third_party/blink/renderer/core/svg/svg_string_list.h
      • M third_party/blink/renderer/core/svg/svg_transform.cc
      • M third_party/blink/renderer/core/svg/svg_transform.h
      • M third_party/blink/renderer/core/svg/svg_transform_list.cc
      • M third_party/blink/renderer/core/svg/svg_transform_list.h
      Change size: M
      Delta: 42 files changed, 76 insertions(+), 59 deletions(-)
      Branch: refs/heads/main
      Submit Requirements:
      • requirement satisfiedCode-Review: +1 by Fredrik Söderquist, +1 by Divyansh Mangal
      Open in Gerrit
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: merged
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I8cf7798b904c7fa64c027e36cf44ad5e0a370436
      Gerrit-Change-Number: 7806138
      Gerrit-PatchSet: 6
      Gerrit-Owner: Virali Purbey <virali...@microsoft.com>
      Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Reviewer: Divyansh Mangal <dma...@microsoft.com>
      Gerrit-Reviewer: Fredrik Söderquist <f...@opera.com>
      Gerrit-Reviewer: Vinay Singh <vinay...@microsoft.com>
      Gerrit-Reviewer: Virali Purbey <virali...@microsoft.com>
      open
      diffy
      satisfied_requirement
      Reply all
      Reply to author
      Forward
      0 new messages