Bug 73452

Summary:

[Chromium] Add the FontCache implementation for Android

Product:

WebKit

Reporter:

Peter Beverloo <peter>

Component:

Platform

Assignee:

Peter Beverloo <peter>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, cc-bugs, jamesr, jrg, steveblock, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

66687

Attachments:

Description	Flags
Patch	abarth: review+, abarth: commit-queue-
Patch for landing	none

Peter Beverloo

Reported 2011-11-30 08:36:26 PST

Add the FontCache implementation specific for the Chromium WebKit port on Android, and include various font-related files intended for Linux which can be re-used by us.

Attachments
Patch (11.84 KB, patch) 2011-11-30 08:37 PST, Peter Beverloo	abarth: review+ abarth: commit-queue-	Details Formatted Diff Diff
Patch for landing (11.64 KB, patch) 2011-12-01 03:10 PST, Peter Beverloo	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Peter Beverloo

Comment 1 2011-11-30 08:37:37 PST

Created attachment 117200 [details] Patch

Adam Barth

Comment 2 2011-11-30 10:24:06 PST

Comment on attachment 117200 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117200&action=review > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:103 > +const SimpleFontData* FontCache::getFontDataForCharacters(const Font& font, > + const UChar* characters, > + int length) One line, please. > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:124 > + DEFINE_STATIC_LOCAL(const AtomicString, sansStr, ("Sans")); > + DEFINE_STATIC_LOCAL(const AtomicString, serifStr, ("Serif")); > + DEFINE_STATIC_LOCAL(const AtomicString, monospaceStr, ("Monospace")); Please use complete words in variable names. > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:145 > +void FontCache::getTraitsInFamily(const AtomicString& familyName, > + Vector<unsigned>& traitsMasks) One line. > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:151 > +FontPlatformData* FontCache::createFontPlatformData(const FontDescription& fontDescription, > + const AtomicString& family) One line. > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:183 > + typeface->unref(); Can typeface be a RefPtr? > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:191 > + (style & SkTypeface::kBold) && !typeface->isBold(), > + (style & SkTypeface::kItalic) && !typeface->isItalic(), > + fontDescription.orientation(), > + fontDescription.textOrientation()); I would indent this to match the (

Peter Beverloo

Comment 3 2011-12-01 03:10:27 PST

Created attachment 117389 [details] Patch for landing Thank you for the review! (In reply to comment #2) > (From update of attachment 117200 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117200&action=review > > > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:103 > > +const SimpleFontData* FontCache::getFontDataForCharacters(const Font& font, > > + const UChar* characters, > > + int length) > > One line, please. Done > > > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:124 > > + DEFINE_STATIC_LOCAL(const AtomicString, sansStr, ("Sans")); > > + DEFINE_STATIC_LOCAL(const AtomicString, serifStr, ("Serif")); > > + DEFINE_STATIC_LOCAL(const AtomicString, monospaceStr, ("Monospace")); > > Please use complete words in variable names. Done, also sorted them by their time of usage. > > > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:145 > > +void FontCache::getTraitsInFamily(const AtomicString& familyName, > > + Vector<unsigned>& traitsMasks) > > One line. Done > > > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:151 > > +FontPlatformData* FontCache::createFontPlatformData(const FontDescription& fontDescription, > > + const AtomicString& family) > > One line. Done > > > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:183 > > + typeface->unref(); > > Can typeface be a RefPtr? I don't think so, as the reference counting is already being managed by Skia in SkRefCnt (which SkTypeface extends from). > > > Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp:191 > > + (style & SkTypeface::kBold) && !typeface->isBold(), > > + (style & SkTypeface::kItalic) && !typeface->isItalic(), > > + fontDescription.orientation(), > > + fontDescription.textOrientation()); > > I would indent this to match the ( Done.

WebKit Review Bot

Comment 4 2011-12-01 14:08:30 PST

Comment on attachment 117389 [details] Patch for landing Clearing flags on attachment: 117389 Committed r101704: <http://trac.webkit.org/changeset/101704>

WebKit Review Bot

Comment 5 2011-12-01 14:08:35 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.