New issue
Advanced search Search tips

Issue 792785 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug


Show other hotlists

Hotlists containing this issue:
Launcher-Tech-Debt


Sign in to add a comment

app_list::SearchResultView should use views::StyledLabel for title/details

Project Member Reported by tapted@chromium.org, Dec 7 2017

Issue description

Chrome Version: m65

currently it manipulates gfx::RenderText instances directly, which takes some decision-making and optimisation opportunities away from the toolkit.

E.g. it uses Font::DeriveWithSizeDelta(2) which is quite slow, and has custom color constants rather than using ash_typography.h for styling

Context: https://chromium-review.googlesource.com/c/chromium/src/+/805534#157 and Issue 791391
 

Comment 1 by vadimt@chromium.org, Dec 13 2017

Labels: Touch-Friendly-Launcher-Triaged Touch-Friendly-Launcher
Cc: newcomer@chromium.org
Owner: tapted@chromium.org
Should we not be using DeriveWithSizeDelta? It's used in other places as well.
Components: UI>Shell>Launcher
Owner: ----
Yeah all the places using gfx::Font[List]::DeriveWithSizeDelta are slow and they should all be replaced. That method typically requires synchronous IPC with the font server, and may even access font files on disk. These are not things we do on the UI thread in Chrome.

Note there is also ui::ResourceBundle::GetFontListWithDelta(), which has a cache. It's better.

But more generally, we shouldn't be scattering constants throughout the codebase in order to call GetFontListWithDelta(). Typically there's another set of constants for text color, and another for line spacing.

ash_typography.h provides a means to bundle these up together, in one place.
Cc: -newcomer@chromium.org
Owner: newcomer@chromium.org
Status: Assigned (was: Available)
Ok, thanks for explaining! Very interesting.
Labels: -Pri-3 M-74 Pri-2
Owner: ----
Unassigning so others from the launcher team can take it.
Status: Available (was: Assigned)

Sign in to add a comment