New issue
Advanced search Search tips

Issue 699820 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocking:
issue 454835
issue 603386



Sign in to add a comment

RenderTextHarfBuzz crashes on Mac if given an invalid font name.

Project Member Reported by karandeepb@chromium.org, Mar 9 2017

Issue description

Chrome Version: 59.0.3033.0
OS: Mac

This is not a problem on other OS. The native font would be nil in this case on Mac which causes the crash.

https://codereview.chromium.org/2734333005/ adds a test testing this behavior. Stack trace of crash:

    frame #0: 0x00007fffc2d92f28 CoreFoundation`CFArrayGetCount + 24
    frame #1: 0x0000000100d4eacd libgfx.dylib`gfx::GetFallbackFonts(font=0x000000010b210bf0) + 413 at font_fallback_mac.mm:42
    frame #2: 0x0000000100de4ccf libgfx.dylib`gfx::RenderTextHarfBuzz::ShapeRun(this=0x0000000106d0d930, text=0x0000000106d0da68, run=0x000000010b2127d0) + 1215 at render_text_harfbuzz.cc:1501
    frame #3: 0x0000000100de0104 libgfx.dylib`gfx::RenderTextHarfBuzz::ShapeRunList(this=0x0000000106d0d930, text=0x0000000106d0da68, run_list=0x0000000106d0db38) + 500 at render_text_harfbuzz.cc:1442
    frame #4: 0x0000000100dd93fa libgfx.dylib`gfx::RenderTextHarfBuzz::EnsureLayoutRunList(this=0x0000000106d0d930) + 730 at render_text_harfbuzz.cc:1647
    frame #5: 0x0000000100ddd6f6 libgfx.dylib`gfx::RenderTextHarfBuzz::EnsureLayout(this=0x0000000106d0d930) + 54 at render_text_harfbuzz.cc:1186
    frame #6: 0x000000010035ae54 gfx_unittests`gfx::test::RenderTextTestApi::DrawVisualText(this=0x000000010b20dcc0, renderer=0x0000000106d0d7a8) + 36 at render_text_unittest.cc:79
    frame #7: 0x000000010030fda7 gfx_unittests`gfx::RenderTextTest::DrawVisualText(this=0x0000000106d0d760) + 71 at render_text_unittest.cc:451
    frame #8: 0x0000000100342420 gfx_unittests`gfx::RenderTextTest_InvalidFont_Test::TestBody(this=0x0000000106d0d760) + 1312 at render_text_unittest.cc:4360

This is also probably the reason for some of the crashes in  issue 640068 .
 
Labels: MacViews-Controls

Comment 2 by tapted@chromium.org, Apr 13 2017

Blocking: 603386
Labels: -Pri-3 Pri-2
See also  Issue 368192  Issue 668058 and r455833 (similar problem, but for RenderTextMac instead of RTHB).

Comment 3 by tapted@chromium.org, Jun 27 2017

Owner: tapted@chromium.org
Status: Started (was: Available)
https://codereview.chromium.org/2959913002
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f5cbd14b845f2795a87e9caa7e048e2ccf95947

commit 0f5cbd14b845f2795a87e9caa7e048e2ccf95947
Author: tapted <tapted@chromium.org>
Date: Thu Jun 29 00:35:53 2017

PlatformFontMac: Use a default NSFont when an invalid font is passed.

Currently RenderTextHarfbuzz crashes trying to find fallbacks for an
null font.

Ensuring PlatformFontMac never has a null NSFont is similar to the
approach in PlatformFontLinux (its anon::CreateSkTypeface(..) never
returns null). Having this invariant allows some logic in
PlatformFontMac to be simplified.

BUG= 699820 

Review-Url: https://codereview.chromium.org/2959913002
Cr-Commit-Position: refs/heads/master@{#483223}

[modify] https://crrev.com/0f5cbd14b845f2795a87e9caa7e048e2ccf95947/ui/gfx/font_fallback_mac.mm
[modify] https://crrev.com/0f5cbd14b845f2795a87e9caa7e048e2ccf95947/ui/gfx/platform_font_mac.h
[modify] https://crrev.com/0f5cbd14b845f2795a87e9caa7e048e2ccf95947/ui/gfx/platform_font_mac.mm
[modify] https://crrev.com/0f5cbd14b845f2795a87e9caa7e048e2ccf95947/ui/gfx/render_text_unittest.cc

Comment 5 by tapted@chromium.org, Jun 29 2017

Status: Fixed (was: Started)
Maybe the changes introduced in https://codereview.chromium.org/2734333005 are no longer necessary with this change?

Comment 7 by tapted@chromium.org, Jun 29 2017

possibly so.. (but we also just want to delete RenderTextMac entirely :)

Sign in to add a comment