Bug 72780 - Move HarfBuzz files into their own directory
Summary: Move HarfBuzz files into their own directory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-18 17:48 PST by Daniel Bates
Modified: 2011-12-27 16:49 PST (History)
15 users (show)

See Also:


Attachments
Patch (185.09 KB, patch)
2011-12-20 16:43 PST, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (185.73 KB, patch)
2011-12-27 10:35 PST, Tony Chang
no flags Details | Formatted Diff | Diff
Patch for landing (185.83 KB, patch)
2011-12-27 13:09 PST, Tony Chang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.