Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(254)

Issue 999173008: Don't add Segoe UI to the fallback font list if it is the system font. (Closed)

Created:
5 years, 9 months ago by ananta
Modified:
5 years, 9 months ago
Reviewers:
msw
CC:
chromium-reviews, derat+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't add Segoe UI to the fallback font list if it is the system font. In this case Segoe UI and its linked fonts are already in the fallback list. This is an attempt to fix the perf regression caused on the page_cycler.intl_hi_ru bot caused by the change to remove duplicate fonts from the fallback list. BUG=469700 Committed: https://crrev.com/2cf99389793d41131db47185a28cc153d96a2bab Cr-Commit-Position: refs/heads/master@{#322084}

Patch Set 1 #

Patch Set 2 : Fix build error #

Total comments: 2

Patch Set 3 : Code review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -4 lines) Patch
M ui/gfx/render_text_harfbuzz.cc View 1 2 2 chunks +8 lines, -4 lines 0 comments Download

Messages

Total messages: 9 (3 generated)
ananta
5 years, 9 months ago (2015-03-24 15:45:27 UTC) #2
msw
lgtm https://codereview.chromium.org/999173008/diff/20001/ui/gfx/render_text_harfbuzz.cc File ui/gfx/render_text_harfbuzz.cc (right): https://codereview.chromium.org/999173008/diff/20001/ui/gfx/render_text_harfbuzz.cc#newcode1300 ui/gfx/render_text_harfbuzz.cc:1300: if (!LowerCaseEqualsASCII(primary_family, "segoe ui")) { nit: maybe check ...
5 years, 9 months ago (2015-03-24 17:38:52 UTC) #3
ananta
https://codereview.chromium.org/999173008/diff/20001/ui/gfx/render_text_harfbuzz.cc File ui/gfx/render_text_harfbuzz.cc (right): https://codereview.chromium.org/999173008/diff/20001/ui/gfx/render_text_harfbuzz.cc#newcode1300 ui/gfx/render_text_harfbuzz.cc:1300: if (!LowerCaseEqualsASCII(primary_family, "segoe ui")) { On 2015/03/24 17:38:52, msw ...
5 years, 9 months ago (2015-03-24 20:10:12 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/999173008/40001
5 years, 9 months ago (2015-03-24 21:51:49 UTC) #7
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 9 months ago (2015-03-24 22:47:28 UTC) #8
commit-bot: I haz the power
5 years, 9 months ago (2015-03-24 22:48:20 UTC) #9
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/2cf99389793d41131db47185a28cc153d96a2bab
Cr-Commit-Position: refs/heads/master@{#322084}

Powered by Google App Engine
This is Rietveld 408576698