WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
121656
Avoid calling AtomicString::lower() in makeFontGlyphsCacheKey
https://bugs.webkit.org/show_bug.cgi?id=121656
Summary
Avoid calling AtomicString::lower() in makeFontGlyphsCacheKey
Anders Carlsson
Reported
2013-09-19 21:31:49 PDT
Avoid calling AtomicString::lower() in makeFontGlyphsCacheKey
Attachments
Patch
(3.64 KB, patch)
2013-09-19 21:34 PDT
,
Anders Carlsson
darin
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Anders Carlsson
Comment 1
2013-09-19 21:34:17 PDT
Created
attachment 212122
[details]
Patch
Darin Adler
Comment 2
2013-09-19 21:37:24 PDT
Comment on
attachment 212122
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=212122&action=review
> Source/WebCore/platform/graphics/Font.cpp:171 > + if (!equalIgnoringCase(a.families[i].impl(), b.families[i].impl()))
Why is the impl() needed here? Shouldn't we have functions overloaded so that isn’t needed?
> Source/WebCore/platform/graphics/Font.cpp:206 > + Vector<unsigned, 7> hashCodes; > + hashCodes.reserveInitialCapacity(4 + key.families.size());
Need a comment explaining why 7. Why not, say, more like 16 to cover virtually all non-ridiculous cases without allocation?
> Source/WebCore/platform/graphics/Font.cpp:215 > + return StringHasher::hashMemory(hashCodes.data(), hashCodes.size() * sizeof(unsigned));
The proper comment for this line is, "Yo dawg! I heard you like hashes, so I hashed your hash."
Ryosuke Niwa
Comment 3
2013-09-19 21:37:37 PDT
Comment on
attachment 212122
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=212122&action=review
> Source/WebCore/platform/graphics/Font.cpp:170 > + for (unsigned i = 0; i < a.families.size(); ++i) {
Why don't we cache the size in a local variable or traverse backwards?
Anders Carlsson
Comment 4
2013-09-19 21:46:22 PDT
Committed
r156139
: <
http://trac.webkit.org/changeset/156139
>
Csaba Osztrogonác
Comment 5
2013-09-19 22:38:20 PDT
(In reply to
comment #4
)
> Committed
r156139
: <
http://trac.webkit.org/changeset/156139
>
FYI:It made all test crash everywhere.
Csaba Osztrogonác
Comment 6
2013-09-20 00:07:54 PDT
Fix landed in
http://trac.webkit.org/changeset/156142
. Thanks.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug