Devirtualize and inline various NativeTheme getters. [chromium/src : main]
0 views
Skip to first unread message
Peter Kasting (Gerrit)
Sep 4, 2025, 6:46:49 PM (3 days ago) Sep 4
to Lucas Radaelli, Roman Arora, Alex Moshchuk, Tom Lukaszewicz, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Lucas Radaelli, Peter Kasting, Roman Arora and Tom Lukaszewicz
Peter Kasting voted and added 1 comment![]( "Open in Gerrit")
Votes added by Peter Kasting
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Peter Kasting . resolved
lucasradaelli: c/b/accessibility
romanarora: c/b/thems
alexmos: content/browser/web_contents
tluk: Rest
Related details
Attention is currently required from:
- Alex Moshchuk
- Lucas Radaelli
- Peter Kasting
- Roman Arora
- Tom Lukaszewicz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Lukaszewicz (Gerrit)
Sep 4, 2025, 8:49:59 PM (3 days ago) Sep 4
to Peter Kasting, Lucas Radaelli, Roman Arora, Alex Moshchuk, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Lucas Radaelli, Peter Kasting and Roman Arora
Tom Lukaszewicz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Tom Lukaszewicz
| Code-Review | +1 |
1 comment
Patchset-level comments
Tom Lukaszewicz . resolved
Nice - lgtm!
Related details
Attention is currently required from:
- Alex Moshchuk
- Lucas Radaelli
- Peter Kasting
- Roman Arora
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kasting (Gerrit)
Sep 4, 2025, 8:59:46 PM (3 days ago) Sep 4
to Tom Lukaszewicz, Lucas Radaelli, Roman Arora, Alex Moshchuk, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Lucas Radaelli, Roman Arora and Tom Lukaszewicz
Peter Kasting added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Alex Moshchuk
- Lucas Radaelli
- Roman Arora
- Tom Lukaszewicz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Lukaszewicz (Gerrit)
Sep 4, 2025, 10:20:59 PM (3 days ago) Sep 4
to Peter Kasting, Lucas Radaelli, Roman Arora, Alex Moshchuk, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Lucas Radaelli, Peter Kasting and Roman Arora
Tom Lukaszewicz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Tom Lukaszewicz
| Code-Review | +1 |
1 comment
Patchset-level comments
Tom Lukaszewicz . resolved
still lgtm (though we may still be missing at least third_party/blink/renderer/core/paint/theme_painter_default.cc)
Related details
Attention is currently required from:
- Alex Moshchuk
- Lucas Radaelli
- Peter Kasting
- Roman Arora
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kasting (Gerrit)
Sep 5, 2025, 11:08:14 AM (2 days ago) Sep 5
to Patrick Monette, Chris Harrelson, Tom Lukaszewicz, Lucas Radaelli, Roman Arora, Alex Moshchuk, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Chris Harrelson, Lucas Radaelli, Patrick Monette, Roman Arora and Tom Lukaszewicz
Peter Kasting added 2 comments![]( "Open in Gerrit")
Patchset-level comments
Tom Lukaszewicz . resolved
still lgtm (though we may still be missing at least third_party/blink/renderer/core/paint/theme_painter_default.cc)
Peter Kasting
Oof, we were missing multiple files. I apparently didn't notate that I needed to update GetPreferredContrast().
File-level comment, Patchset 4 (Latest):
Peter Kasting . resolved
pmonette: c/b/win
chrishtr: third_party/blink/renderer/core
Related details
Attention is currently required from:
- Alex Moshchuk
- Chris Harrelson
- Lucas Radaelli
- Patrick Monette
- Roman Arora
- Tom Lukaszewicz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kasting (Gerrit)
Sep 5, 2025, 11:19:23 AM (2 days ago) Sep 5
to Patrick Monette, Chris Harrelson, Tom Lukaszewicz, Roman Arora, Alex Moshchuk, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, mac-r...@chromium.org, jdonnel...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, omnibox-...@chromium.org, blink-...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Chris Harrelson, Patrick Monette, Roman Arora and Tom Lukaszewicz
Peter Kasting added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 5 (Latest):
Peter Kasting . resolved
Actually chrishtr@ can cover c/b/accessibility also
Related details
Attention is currently required from:
- Alex Moshchuk
- Chris Harrelson
- Patrick Monette
- Roman Arora
- Tom Lukaszewicz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Sep 5, 2025, 11:57:42 AM (2 days ago) Sep 5
to Peter Kasting, Chris Harrelson, Tom Lukaszewicz, Roman Arora, Alex Moshchuk, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, mac-r...@chromium.org, jdonnel...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, omnibox-...@chromium.org, blink-...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Chris Harrelson, Peter Kasting, Roman Arora and Tom Lukaszewicz
Patrick Monette voted and added 3 comments![]( "Open in Gerrit")
Votes added by Patrick Monette
| Code-Review | +1 |
3 comments
Patchset-level comments
Patrick Monette . resolved
lgtm with nits
File ui/native_theme/native_theme.h
Line 511, Patchset 5 (Latest):
bool inverted_colors() const { return inverted_colors_; }Patrick Monette . unresolved
Previous comment applies here too.
Line 479, Patchset 5 (Latest):
bool forced_colors() const { return forced_colors_; }Patrick Monette . unresolved
That reads less like a bool now, especially in juxtaposition with `page_colors` below. Why not keep it as `in_forced_colors_mode`?
Related details
Attention is currently required from:
- Alex Moshchuk
- Chris Harrelson
- Peter Kasting
- Roman Arora
- Tom Lukaszewicz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kasting (Gerrit)
Sep 5, 2025, 12:30:37 PM (2 days ago) Sep 5
to Patrick Monette, Chris Harrelson, Tom Lukaszewicz, Roman Arora, Alex Moshchuk, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, mac-r...@chromium.org, jdonnel...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, omnibox-...@chromium.org, blink-...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Chris Harrelson, Patrick Monette, Roman Arora and Tom Lukaszewicz
Peter Kasting added 2 comments![]( "Open in Gerrit")
File ui/native_theme/native_theme.h
Line 511, Patchset 5:
bool inverted_colors() const { return inverted_colors_; }Patrick Monette . resolved
Previous comment applies here too.
Peter Kasting
Acknowledged
Line 479, Patchset 5:
bool forced_colors() const { return forced_colors_; }Patrick Monette . resolved
That reads less like a bool now, especially in juxtaposition with `page_colors` below. Why not keep it as `in_forced_colors_mode`?
Peter Kasting
I agree. But in a followup change I'll be eliminating page_colors() and making this return an enum instead of a bool, so this minimizes the diff to that.
Related details
Attention is currently required from:
- Alex Moshchuk
- Chris Harrelson
- Patrick Monette
- Roman Arora
- Tom Lukaszewicz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chris Harrelson (Gerrit)
Sep 5, 2025, 1:07:26 PM (2 days ago) Sep 5
to Peter Kasting, Patrick Monette, Tom Lukaszewicz, Roman Arora, Alex Moshchuk, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, ntp-dev...@chromium.org, mac-r...@chromium.org, jdonnel...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, omnibox-...@chromium.org, blink-...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Alex Moshchuk, Patrick Monette, Peter Kasting, Roman Arora and Tom Lukaszewicz
Chris Harrelson voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Alex Moshchuk
- Patrick Monette
- Peter Kasting
- Roman Arora
- Tom Lukaszewicz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Moshchuk (Gerrit)
Sep 5, 2025, 1:38:00 PM (2 days ago) Sep 5
to Peter Kasting, Chris Harrelson, Patrick Monette, Tom Lukaszewicz, Roman Arora, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, ntp-dev...@chromium.org, mac-r...@chromium.org, jdonnel...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, omnibox-...@chromium.org, blink-...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Patrick Monette, Peter Kasting, Roman Arora and Tom Lukaszewicz
Alex Moshchuk voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alex Moshchuk
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
Roman Arora (Gerrit)
Sep 5, 2025, 3:05:16 PM (2 days ago) Sep 5
to Peter Kasting, Alex Moshchuk, Chris Harrelson, Patrick Monette, Tom Lukaszewicz, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, ntp-dev...@chromium.org, mac-r...@chromium.org, jdonnel...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, omnibox-...@chromium.org, blink-...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Patrick Monette, Peter Kasting and Tom Lukaszewicz
Roman Arora voted and added 1 comment![]( "Open in Gerrit")
Votes added by Roman Arora
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Patrick Monette
- Peter Kasting
- Tom Lukaszewicz
Peter Kasting (Gerrit)
Sep 5, 2025, 3:07:26 PM (2 days ago) Sep 5
to Roman Arora, Alex Moshchuk, Chris Harrelson, Patrick Monette, Tom Lukaszewicz, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, ntp-dev...@chromium.org, mac-r...@chromium.org, jdonnel...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, omnibox-...@chromium.org, blink-...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Patrick Monette and Tom Lukaszewicz
Peter Kasting voted and added 1 comment![]( "Open in Gerrit")
Votes added by Peter Kasting
| Auto-Submit | +1 |
1 comment
Patchset-level comments
Peter Kasting . resolved
Sorry Tom, back to you one more time for missed files :(
Related details
Attention is currently required from:
- Patrick Monette
- Tom Lukaszewicz
Tom Lukaszewicz (Gerrit)
2:36 PM (3 hours ago) 2:36 PM
to Peter Kasting, Roman Arora, Alex Moshchuk, Chris Harrelson, Patrick Monette, AyeAye, chromium...@chromium.org, (Julie)Jeongeun Kim, Chromium LUCI CQ, ntp-dev...@chromium.org, mac-r...@chromium.org, jdonnel...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, omnibox-...@chromium.org, blink-...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, alexmo...@chromium.org, creis...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kuragin+web-ap...@chromium.org, kyungjunle...@google.com, loyso...@chromium.org, mgiuca...@chromium.org, navigation...@chromium.org, nektar...@chromium.org, philli...@chromium.org, roblia...@chromium.org, sky+...@chromium.org, webap...@microsoft.com, yuzo+...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Patrick Monette and Peter Kasting
Tom Lukaszewicz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Tom Lukaszewicz
| Code-Review | +1 |
1 comment
Patchset-level comments
Peter Kasting . resolved
Sorry Tom, back to you one more time for missed files :(
Attention is currently required from:
- Patrick Monette
- Peter Kasting
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages