1511131 - WR on Android crashes at fivethirtyeight.com (GetScaledFont)

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Bobby Holley (:bholley)]

STR:

(1) install https://queue.taskcluster.net/v1/task/ZbOcpf3VShmdkeESusiGMg/runs/0/artifacts/public/build/geckoview_example.apk
(2) enable WR in about:config
(3) visit fivethirtyeight.com and scroll around

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Crash stack (from a debug build) indicates GetScaledFont is failing

mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::WriteLog(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) Logging.h:729
mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::Flush() Logging.h:285
mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::~Log() Logging.h:277
mozilla::wr::GetScaledFont(mozilla::gfx::Translator*, mozilla::wr::FontInstanceKey) Moz2DImageRenderer.cpp:315
mozilla::wr::Moz2DRenderCallback(mozilla::Range<unsigned char const>, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::SurfaceFormat, unsigned short const*, mozilla::wr::TypedPoint2D<int, mozilla::wr::TileCoordinate> const*, mozilla::wr::TypedRect<int, mozilla::wr::LayoutPixel> const*, mozilla::Range<unsigned char>) Moz2DImageRenderer.cpp:444
::wr_moz2d_render_cb(const mozilla::wr::ByteSlice, int32_t, int32_t, mozilla::wr::ImageFormat, const uint16_t *, const mozilla::wr::TileOffset *, const mozilla::wr::LayoutIntRect *, mozilla::wr::MutByteSlice) Moz2DImageRenderer.cpp:492
webrender_bindings::moz2d_renderer::rasterize_blob::h54ec110feb7eacfd moz2d_renderer.rs:524
core::ops::function::Fn::call::h9e8549b76fb51131 0x00000000cd9b263c
_$LT$rayon..iter..map..MapFolder$LT$$u27$f$C$$u20$C$C$$u20$F$GT$$u20$as$u20$rayon..iter..plumbing..Folder$LT$T$GT$$GT$::consume::h28a76e6450a634b1 map.rs:225
rayon::iter::plumbing::Folder::consume_iter::h7fd3b261eeac17d2 mod.rs:176
rayon::iter::plumbing::Producer::fold_with::h6e9357f3175fef83 mod.rs:108
rayon::iter::plumbing::bridge_producer_consumer::helper::hcd65b4a7df5847da mod.rs:418
rayon::iter::plumbing::bridge_producer_consumer::helper::_$u7b$$u7b$closure$u7d$$u7d$::h84bcb5495f816a9b mod.rs:413
rayon_core::join::join_context::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h45c2983b02c503ac mod.rs:122
_$LT$rayon_core..job..StackJob$LT$L$C$$u20$F$C$$u20$R$GT$$u20$as$u20$rayon_core..job..Job$GT$::execute::_$u7b$$u7b$closure$u7d$$u7d$::h41c8a13eaa330a08 job.rs:113
_$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::had8f91c179a19c5b 0x00000000cdaeed6a
std::panicking::try::do_call::hf63a757f3396b05b 0x00000000cdaf06a4
__rust_maybe_catch_panic 0x00000000cd931848
std::panicking::try::h80441830b3ade85f 0x00000000cdaf021c
std::panic::catch_unwind::h55150b62a74ba1d9 0x00000000cdaef7be
rayon_core::unwind::halt_unwinding::had9d5669c49c30af unwind.rs:18
_$LT$rayon_core..job..StackJob$LT$L$C$$u20$F$C$$u20$R$GT$$u20$as$u20$rayon_core..job..Job$GT$::execute::hb28617915177a865 job.rs:113
rayon_core::registry::WorkerThread::execute::h23042e8e03e0d8e3 registry.rs:583
rayon_core::registry::WorkerThread::wait_until_cold::h7f8d811cab851a27 registry.rs:567
rayon_core::registry::WorkerThread::wait_until::h1e6b5c9b30af02a9 registry.rs:543
rayon_core::registry::main_loop::h2f81b64a878fcefb registry.rs:674
std::sys_common::backtrace::__rust_begin_short_backtrace::h924b74c5c0f66386 0x00000000cd8d7c8c
std::thread::Builder::spawn::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h6d97af3329bfa2f1 0x00000000cd8d8c8c
_$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::hdc6b9547884cdac3 0x00000000cd8d8a8e
std::panicking::try::do_call::haddd4265a65441d6 0x00000000cdab2148
__rust_maybe_catch_panic 0x00000000cd931848
std::panicking::try::hf8e06d6d6e15052a 0x00000000cdab20ae
std::panic::catch_unwind::h1418cc43a1c667ce 0x00000000cd8d8c6e
std::thread::Builder::spawn::_$u7b$$u7b$closure$u7d$$u7d$::hdf2fe538e1c38a95 0x00000000cd8dc036
_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h1135cafb8dfe26ad 0x00000000cd8dc43a
_$LT$alloc..boxed..Box$LT$$LP$dyn$u20$alloc..boxed..FnBox$LT$A$C$$u20$Output$u3d$R$GT$$u20$$u2b$$u20$$u27$a$RP$$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h0361671c8243d617 0x00000000cd9114a6
std::sys_common::thread::start_thread::hd1b180dd2fa4010c 0x00000000cd9114a2
std::sys::unix::thread::Thread::new::thread_start::h63a610b507ecb711 0x00000000cd91149c
__pthread_start(void*) 0x00000000eb2898d8
__start_thread 0x00000000eb25d342

Comment 2

[Kartikaya Gupta (email:[e-mail])]

This is the relevant entry in sBlobFontTable:

     second = {
        mFontKey = {
          mNamespace = (mHandle = 3)
          mHandle = 65
        }
        mSize = 73.050003
        mOptions = (mStorage = unsigned char [16] @ 0x00007fbc5c3ff2c4, mIsSome = 1 '\x01')
        mPlatformOptions = (mStorage = unsigned char [2] @ 0x00007fbc5c3ff2d8, mIsSome = 1 '\x01')
        mVariations = {
          mTuple = {
            mozilla::detail::PairHelper<mozilla::gfx::FontVariation *, mozilla::DefaultDelete<mozilla::gfx::FontVariation []>, mozilla::detail::AsMember, mozilla::detail::AsBase> = {
              mFirstA = 0x00000000
            }
          }
        }
        mNumVariations = 0
        mScaledFont = {
          mRawPtr = 0x00000000
        }
      }
    }

And the thing in data.mOptions:

(mozilla::wr::FontInstanceOptions) $5 = {
  render_mode = Alpha
  flags = (bits = 0)
  bg_color = (r = 0 '\0', g = 0 '\0', b = 0 '\0', a = 0 '\0')
  synthetic_italics = (angle = 0)
}

and the thing in data.mPlatformOptions:

(mozilla::wr::FontInstancePlatformOptions) $6 = (lcd_filter = None, hinting = None)

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Just realized this is the same problem that Jamie was seeing, bug 1509870.

Comment 4

[Jamie Nicol [:jnicol]]

Here is my understanding of the lead up to the crash:

When we are preparing a blob image we call process_fonts(), and process a FontTemplate::Native. So we call AddNativeFontHandle() which constructs an UnscaledFontFreeType using the (const char* aHandle, uint32_t aIndex) constructor, which means mFace is nullptr. And it adds an item to sFontDataTable with mUnscaledFont set rather than mData etc.

Then when rasterizing the blob we call GetScaledFont(). This calls GetUnscaledFont(), which finds the matching item in sFontDataTable. Because mUnscaledFont has been set it simpply returns that (rather than creating a font resource from the data). Then it calls unscaled->CreateScaledFontFromWRFont, which leads to UnscaledFontFreeType::CreateScaledFont(). This returns nullptr because mFace is nullptr.

Lee, where are we going wrong here? I can see that in UnscaledFontFontconfig::CreateScaledFont() we have code that creates the scaled font even if mFace == nullptr. Are we missing an implementation of that for android? Or should we not be getting that far in the first place?

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: Bug 1511131 - init UnscaledFontFreeType's face from file when creating scaled fonts. r?jnicol

Comment 6

[Lee Salzman [:lsalzman]]

Theoretically this patch should fix it. Can you test it?

Comment 7

[Jamie Nicol [:jnicol]]

This fixes it, thanks Lee

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5505ed4893d6
init UnscaledFontFreeType's face from file when creating scaled fonts. r=jnicol

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/5505ed4893d6