RESOLVED FIXED 104410
[Chromium-Android] Use harfbuzz-ng instead of harfbuzz-old on Android 
https://bugs.webkit.org/show_bug.cgi?id=104410
Summary [Chromium-Android] Use harfbuzz-ng instead of harfbuzz-old on Android
Xianzhu Wang
Reported 2012-12-07 16:12:18 PST
It's really critical to switch to harfbuzz-ng because so many bugs are resolved with the switch.
Attachments
Patch (5.23 KB, patch)
2012-12-10 13:39 PST, Xianzhu Wang 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Xianzhu Wang
Comment 1 2012-12-10 13:39:59 PST
Created attachment 178624 [details] Patch
Xianzhu Wang
Comment 2 2012-12-10 13:43:41 PST
Just enabled harfbuzz-ng in gyp without any C++ code change. The original Android-specific code in ComplexTextControllerHarfBuzz.cpp (depending on harfbuzz-old) is not needed in the new code path because we already have proper font fallback mechanism on in Skia for Android and used it in WebKit. Perhaps we should cleanup the obsolete code if no one uses harfbuzz-old.
Peter Beverloo
Comment 3 2012-12-12 04:01:36 PST
Do we want this before the M25 branch point? It seems like a risky change to do on such short notice..
Xianzhu Wang
Comment 4 2012-12-12 09:05:51 PST
(In reply to comment #3) > Do we want this before the M25 branch point? It seems like a risky change to do on such short notice.. This is not urgent. I think we can land it after M25 branch point.
Eric Seidel (no email)
Comment 5 2012-12-12 10:30:20 PST
I doubt its particularly risky, given that linux has been on ng for several months iirc. But there is also no hurry in my view.
Eric Seidel (no email)
Comment 6 2012-12-12 10:32:10 PST
Comment on attachment 178624 [details] Patch Lgtm, but be sure to coordinate with peter wrt when this should be landed.
Tony Chang
Comment 7 2012-12-12 11:24:29 PST
Is anyone still on old harfbuzz after this change? Can we delete the old harfbuzz code (there was some extra abstraction to handle both).
Peter Beverloo
Comment 8 2012-12-13 06:28:55 PST
(In reply to comment #5) > I doubt its particularly risky, given that linux has been on ng for several months iirc. But there is also no hurry in my view. We used to have a few modifications in harfbuzz in regards to memory usage, switching Android to harfbuzz-ng just before the branch point can have unexpected side-effects unless it's really thoroughly tested. While it's not a particularly risky change, it's safer to do it in the M26 period. The amount of issues we have with M25 is rather high as it is. Landing this on Tuesday morning (assuming branch occurs on Monday) is perfectly fine with me. Thanks for doing this, Xianzhu!
WebKit Review Bot
Comment 9 2012-12-21 15:28:57 PST
Comment on attachment 178624 [details] Patch Clearing flags on attachment: 178624 Committed r138401: <http://trac.webkit.org/changeset/138401>
WebKit Review Bot
Comment 10 2012-12-21 15:29:01 PST
All reviewed patches have been landed. Closing bug.
Behdad Esfahbod
Comment 11 2012-12-21 16:22:43 PST
Awesome! Thanks everyone.
Note You need to log in before you can comment on or make changes to this bug.