Remove the Animatable interface. [chromium/src : main]

0 views
Skip to first unread message

Mason Freed (Gerrit)

unread,
2:11 PM (10 hours ago) 2:11 PM
to Steinar H Gunderson, Philip Rogers, chrom...@appspot.gserviceaccount.com, Menard, Alexis, chromium...@chromium.org, Olga Gerchikov, blink-revie...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Attention needed from Steinar H Gunderson

Mason Freed added 4 comments

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Mason Freed . resolved

Nice! LGTM, other than moving stuff into `element.cc`.

Commit Message
Line 49, Patchset 2 (Latest): Score [ +0.1%, +0.4%]
Mason Freed . resolved

🎉

File third_party/blink/renderer/core/animation/animatable.cc
Line 5, Patchset 2 (Latest):// Implementation of the Animatable part of Element. (We don't make this
// a separate interface, because it bloats the size of Element.)
Mason Freed . unresolved

I think this is kind of confusing. This file isn't huge - let's just move these methods over into `element.cc` and delete this file. Sound ok?

File third_party/blink/renderer/core/dom/element.h
Line 2584, Patchset 2 (Latest):
// Do not add new members to Element without a good reason; prefer to
// add to ElementRareData unless it is performance-critical. Element
// is 88 bytes on typical 64-bit platforms, and growing it can cause
// both memory and performance regressions if you are not careful.
Mason Freed . resolved

Very nice idea - thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Steinar H Gunderson
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not 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: I6376a8b89146424ddbeb309d89b60f661597443b
Gerrit-Change-Number: 7254563
Gerrit-PatchSet: 2
Gerrit-Owner: Steinar H Gunderson <se...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: Philip Rogers <p...@chromium.org>
Gerrit-Attention: Steinar H Gunderson <se...@chromium.org>
Gerrit-Comment-Date: Mon, 15 Dec 2025 19:10:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Steinar H Gunderson (Gerrit)

unread,
2:14 PM (10 hours ago) 2:14 PM
to Mason Freed, Philip Rogers, chrom...@appspot.gserviceaccount.com, Menard, Alexis, chromium...@chromium.org, Olga Gerchikov, blink-revie...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Attention needed from Mason Freed

Steinar H Gunderson added 1 comment

File third_party/blink/renderer/core/animation/animatable.cc
Line 5, Patchset 2 (Latest):// Implementation of the Animatable part of Element. (We don't make this
// a separate interface, because it bloats the size of Element.)
Mason Freed . unresolved

I think this is kind of confusing. This file isn't huge - let's just move these methods over into `element.cc` and delete this file. Sound ok?

Steinar H Gunderson

Sounds good. To demonstrate the lack of change here in the history so that digging backwards is easier, what if I make the actual move in a followup CL? (I can chain it together with this and submit them together.)

Open in Gerrit

Related details

Attention is currently required from:
  • Mason Freed
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not 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: I6376a8b89146424ddbeb309d89b60f661597443b
Gerrit-Change-Number: 7254563
Gerrit-PatchSet: 2
Gerrit-Owner: Steinar H Gunderson <se...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: Philip Rogers <p...@chromium.org>
Gerrit-Attention: Mason Freed <mas...@chromium.org>
Gerrit-Comment-Date: Mon, 15 Dec 2025 19:13:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Mason Freed (Gerrit)

unread,
2:52 PM (9 hours ago) 2:52 PM
to Steinar H Gunderson, Philip Rogers, chrom...@appspot.gserviceaccount.com, Menard, Alexis, chromium...@chromium.org, Olga Gerchikov, blink-revie...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Attention needed from Steinar H Gunderson

Mason Freed added 1 comment

File third_party/blink/renderer/core/animation/animatable.cc
Line 5, Patchset 2 (Latest):// Implementation of the Animatable part of Element. (We don't make this
// a separate interface, because it bloats the size of Element.)
Mason Freed . unresolved

I think this is kind of confusing. This file isn't huge - let's just move these methods over into `element.cc` and delete this file. Sound ok?

Steinar H Gunderson

Sounds good. To demonstrate the lack of change here in the history so that digging backwards is easier, what if I make the actual move in a followup CL? (I can chain it together with this and submit them together.)

Mason Freed

Great by me! End state is all I care about here.

Open in Gerrit

Related details

Attention is currently required from:
  • Steinar H Gunderson
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not 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: I6376a8b89146424ddbeb309d89b60f661597443b
Gerrit-Change-Number: 7254563
Gerrit-PatchSet: 2
Gerrit-Owner: Steinar H Gunderson <se...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: Philip Rogers <p...@chromium.org>
Gerrit-Attention: Steinar H Gunderson <se...@chromium.org>
Gerrit-Comment-Date: Mon, 15 Dec 2025 19:52:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
Comment-In-Reply-To: Steinar H Gunderson <se...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Mason Freed (Gerrit)

unread,
2:52 PM (9 hours ago) 2:52 PM
to Steinar H Gunderson, Philip Rogers, chrom...@appspot.gserviceaccount.com, Menard, Alexis, chromium...@chromium.org, Olga Gerchikov, blink-revie...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Attention needed from Steinar H Gunderson

Mason Freed voted Code-Review+1

Code-Review+1
Open in Gerrit

Related details

Attention is currently required from:
  • Steinar H Gunderson
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • 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: I6376a8b89146424ddbeb309d89b60f661597443b
Gerrit-Change-Number: 7254563
Gerrit-PatchSet: 2
Gerrit-Owner: Steinar H Gunderson <se...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: Philip Rogers <p...@chromium.org>
Gerrit-Attention: Steinar H Gunderson <se...@chromium.org>
Gerrit-Comment-Date: Mon, 15 Dec 2025 19:52:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy
Reply all
Reply to author
Forward
0 new messages