Rework VR texture font rendering to be testable on both Linux and Android |
||||||||||
Issue descriptionVR UI unit tests now run on Linux. One test works differently (and fails) because our font code falls flat on Linux: TEST(UrlBarTextureTest, WillNotFailOnNonAsciiURLs) We should either fix Linux font rendering, or make the font subsystem mockable, so that the test can run meaningfully on any platform.
,
Aug 9 2017
,
Oct 2 2017
I filed a similar bug for Windows: crbug/770893. I'm keeping them separate because there may need to be a Windows-specific implementation for font fallback. However, if the fix for this also addresses 770893, feel free to resolve that as duplicate.
,
Oct 27 2017
Chris, what would it take to get this running on linux?
,
Oct 27 2017
Not sure other than it'll take an investigation.
,
Jan 3 2018
,
Jan 3 2018
,
Apr 17 2018
Looking at this now - the tests in question work properly on Android, and text rendering works fine on Windows (even if the specific test in question doesn't work the same way on Windows). I'm going to close this off as a WontFix.
,
Jul 4
,
Aug 8
With newer versions of harfbuzz this test now fails on Android too. Can we please fix or disable the test as this is blocking several text rendering improvements and performance optimizations for Chrome and Blink. https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8938748478828460688/+/steps/vr_common_unittests_on_Android_device_Nexus_5X__with_patch_/0/stdout
,
Aug 8
+vollick as cjgrant is OOO.
,
Aug 9
Looks like it's UrlText.WillNotFailOnNonAsciiURLs. I'll need to confirm that VR works with the harfbuzz changes, but if so, I'll disable this test on Android until we can get a proper fix.
,
Aug 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a2ce72e4384ec85ce4730dad11aa59469272060e commit a2ce72e4384ec85ce4730dad11aa59469272060e Author: Ian Vollick <vollick@chromium.org> Date: Fri Aug 10 19:54:17 2018 [vr] Disable WillNotFailOnNonAsciiURLs This will currently not pass with new versions of HarfBuzz. Bug: 731894 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:linux_vr;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: Ib5ae456bf44bfdedd6a0edf36b54dbbf12bae18a Reviewed-on: https://chromium-review.googlesource.com/1169774 Commit-Queue: Ian Vollick <vollick@chromium.org> Reviewed-by: Michael Thiessen <mthiesse@chromium.org> Cr-Commit-Position: refs/heads/master@{#582298} [modify] https://crrev.com/a2ce72e4384ec85ce4730dad11aa59469272060e/chrome/browser/vr/elements/url_text_unittest.cc
,
Aug 16
We should be able to re-enable the test again now.
,
Aug 16
Oh great! I'll create a revert of the disabling patch.
,
Aug 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/86bdbfd2aff5ce74c6c22bfcf701ea81ede2c1a5 commit 86bdbfd2aff5ce74c6c22bfcf701ea81ede2c1a5 Author: Ian Vollick <vollick@chromium.org> Date: Fri Aug 17 10:53:25 2018 Revert "[vr] Disable WillNotFailOnNonAsciiURLs" This reverts commit a2ce72e4384ec85ce4730dad11aa59469272060e. Reason for revert: this test should now pass. Original change's description: > [vr] Disable WillNotFailOnNonAsciiURLs > > This will currently not pass with new versions of HarfBuzz. > > Bug: 731894 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:linux_vr;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel > Change-Id: Ib5ae456bf44bfdedd6a0edf36b54dbbf12bae18a > Reviewed-on: https://chromium-review.googlesource.com/1169774 > Commit-Queue: Ian Vollick <vollick@chromium.org> > Reviewed-by: Michael Thiessen <mthiesse@chromium.org> > Cr-Commit-Position: refs/heads/master@{#582298} TBR=vollick@chromium.org,mthiesse@chromium.org,eae@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 731894 Change-Id: Id02d8c770a763c5c37f993d701d03e518a043ee7 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:linux_vr;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Reviewed-on: https://chromium-review.googlesource.com/1178164 Reviewed-by: Ian Vollick <vollick@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Commit-Queue: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#584018} [modify] https://crrev.com/86bdbfd2aff5ce74c6c22bfcf701ea81ede2c1a5/chrome/browser/vr/elements/url_text_unittest.cc
,
Aug 23
Assigning to you, Ian, since you worked on this. Please add a milestone if appropriate or close this bug if no more work is necessary. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by ddorwin@chromium.org
, Jul 10 2017