Bug 72780

Summary: Move HarfBuzz files into their own directory
Product: WebKit Reporter: Daniel Bates <dbates>
Component: PlatformAssignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, agl, bashi, cc-bugs, dglazkov, efidler, eric, evan, jamesr, jkjiang, jpetsovits, rakuco, tonikitoo, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Daniel Bates 2011-11-18 17:48:52 PST 
Following up from Antonio Gomes's comment in bug #72768 (https://bugs.webkit.org/show_bug.cgi?id=72768#c5), we should move the HarfBuzz files from WebCore/platform/graphics/chromium
to a HarfBuzz directory since both the Chromium Linux and BlackBerry-port make use of these files. These files include:

platform/graphics/chromium/ComplexTextControllerLinux.cpp
platform/graphics/chromium/FontCacheLinux.cpp
platform/graphics/chromium/FontLinux.cpp
platform/graphics/chromium/FontPlatformDataLinux.cpp
platform/graphics/chromium/HarfbuzzSkia.cpp
platform/graphics/chromium/SimpleFontDataLinux.cpp
Comment 1 Eric Seidel (no email) 2011-11-18 17:53:59 PST 
SGTM.
Comment 2 Adam Barth 2011-11-18 17:57:19 PST 
Should we change the "Linux" suffix to "HarfBuzz" at the same time?
Comment 3 Daniel Bates 2011-11-18 17:58:38 PST 
(In reply to comment #2)
> Should we change the "Linux" suffix to "HarfBuzz" at the same time?

Sounds reasonable.
Comment 4 Tony Chang 2011-12-20 16:43:36 PST 
Created attachment 120114 [details]
Patch
Comment 5 WebKit Review Bot 2011-12-20 16:46:50 PST 
Attachment 120114 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/graphics/harfbuzz/HarfBuzzSkia.cpp:47:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Kenichi Ishibashi 2011-12-20 17:00:38 PST 
Comment on attachment 120114 [details]
Patch

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

LGTM.

FYI, I'm working on Harfbuzz-ng transition on Chromium port (https://bugs.webkit.org/show_bug.cgi?id=67933). It will take time because Harfbuzz-ng have performance and rendering regression as far as I tested.

> Source/WebCore/WebCore.gyp/WebCore.gyp:1481
> +            ['include', 'platform/graphics/harfbuzz/SimpleFontDataSkia\\.cpp$'],

Shouldn't we also add 'harfbuzz' in 'exclude'  pattern at line 1396 and 1451?
Comment 7 Tony Chang 2011-12-27 10:35:37 PST 
Created attachment 120605 [details]
Patch
Comment 8 WebKit Review Bot 2011-12-27 10:38:23 PST 
Attachment 120605 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/graphics/harfbuzz/HarfBuzzSkia.cpp:47:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Daniel Bates 2011-12-27 10:49:31 PST 
Comment on attachment 120605 [details]
Patch

Looks sane to me.
Comment 10 WebKit Review Bot 2011-12-27 12:16:13 PST 
Comment on attachment 120605 [details]
Patch

Attachment 120605 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11040333
Comment 11 Tony Chang 2011-12-27 13:09:07 PST 
Created attachment 120613 [details]
Patch for landing
Comment 12 WebKit Review Bot 2011-12-27 16:49:45 PST 
Comment on attachment 120613 [details]
Patch for landing

Clearing flags on attachment: 120613

Committed r103742: <http://trac.webkit.org/changeset/103742>
Comment 13 WebKit Review Bot 2011-12-27 16:49:52 PST 
All reviewed patches have been landed.  Closing bug.