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

Issue 924903002: Don't allocate Strings in FontDescription(). (Closed)

Created:
5 years, 10 months ago by andersr
Modified:
5 years, 10 months ago
Reviewers:
eae
CC:
blink-reviews, krit, pdr+graphicswatchlist_chromium.org, Dominik Röttsches, blink-reviews-css, dshwang, ed+blinkwatch_opera.com, Justin Novosad, jbroman, danakj, dglazkov+blink, Rik, apavlov+blink_chromium.org, darktears, f(malita), Stephen Chennney, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Don't allocate Strings in FontDescription(). Creation of the 'locale' String in the FontDescription constructor accounts for 14% of the time spent in StyleResolver::styleForElement in the test- case html5-full-render.html. Changes in this patch: * Let locale default-initialize, and instead produce "en" on demand. * Use AtomicString instead of String. The locale is already stored on LayoutStyle as an AtomicString, so we might as well use that. R=eae@chromium.org BUG=438069 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190246

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -8 lines) Patch
M Source/core/css/resolver/FontBuilder.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/resolver/FontBuilder.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/fonts/FontDescription.h View 4 chunks +4 lines, -5 lines 0 comments Download
M Source/platform/fonts/FontDescription.cpp View 2 chunks +16 lines, -1 line 0 comments Download

Messages

Total messages: 5 (1 generated)
andersr
5 years, 10 months ago (2015-02-14 00:04:32 UTC) #1
eae
LGTM
5 years, 10 months ago (2015-02-14 00:37:15 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/924903002/1
5 years, 10 months ago (2015-02-16 10:29:25 UTC) #4
commit-bot: I haz the power
5 years, 10 months ago (2015-02-16 11:47:32 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190246

Powered by Google App Engine
This is Rietveld 408576698