Bug 47732

Summary:

[chromium] FontLinux.cpp cleanup (move most methods in TextRunWorker from inlined)

Product:

WebKit

Reporter:

Xiaomei Ji <xji>

Component:

Platform

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, eric, levin, skyul, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

47019

Attachments:

Description	Flags
patch	levin: review+

Description Xiaomei Ji 2010-10-15 12:28:11 PDT

All function members in TextRunWorker are defined as inlined.
Most of them should not.

Comment 1 Xiaomei Ji 2010-10-15 12:35:45 PDT

Created attachment 70888 [details]
patch

Comment 2 David Levin 2010-10-16 12:07:18 PDT

Comment on attachment 70888 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=70888&action=review

Please consider addressing my comments on landing.

> WebCore/platform/graphics/chromium/FontLinux.cpp:164
> +    TextRunWalker(const TextRun& run, unsigned startingX, const Font* font);

Remove param names that add no information: "run", "font".

> WebCore/platform/graphics/chromium/FontLinux.cpp:167
> +    bool isWordBreak(unsigned i, bool isRTL);

What is "i? In its current form, I'd remove it, but I have no idea what that param is. Should it be called index?

> WebCore/platform/graphics/chromium/FontLinux.cpp:170
> +    void setPadding(int padding);

No need for "padding". (Since the method name is setPadding and it only takes one parameter.)

> WebCore/platform/graphics/chromium/FontLinux.cpp:172
> +    void setBackwardsIteration(bool isBackwards);

Remove param name.

Comment 3 Xiaomei Ji 2010-10-18 10:41:41 PDT

Committed r69972: <http://trac.webkit.org/changeset/69972>

Comment 4 WebKit Review Bot 2010-10-18 11:06:43 PDT

http://trac.webkit.org/changeset/69972 might have broken Qt Windows 32-bit Release