Assertion failure in ElementAnimations on Android Release Build |
||||||
Issue descriptionVersion: ToT, Monday 03/14, Git bc71ad0 OS: Android What steps will reproduce the problem? Build a GN android release build GN args: target_os = "android" is_clang = true Run the attached HTML test case Observe crash with Assertion failure. 03-18 11:08:41.192 4289 4304 W WebKit : ASSERTION FAILED: *m_baseComputedStyle == *computedStyle 03-18 11:08:41.192 4289 4304 W WebKit : ../../third_party/WebKit/Source/core/animation/ElementAnimations.cpp(128) : void blink::ElementAnimations::updateBaseComputedStyle(const blink::ComputedStyle *) --------- beginning of crash 03-18 11:08:41.192 4289 4304 F libc : Fatal signal 11 (SIGSEGV), code 1, fault addr 0xfbadbeef in tid 4304 (CrRendererMain) 03-18 11:08:41.253 368 368 I SELinux : SELinux: Loaded file_contexts contexts from /file_contexts. 03-18 11:08:41.254 368 368 F DEBUG : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** 03-18 11:08:41.255 368 368 F DEBUG : Build fingerprint: 'google/shamu/shamu:6.0.1/MMB29K/2419427:user/release-keys' 03-18 11:08:41.255 368 368 F DEBUG : Revision: '0' 03-18 11:08:41.255 368 368 F DEBUG : ABI: 'arm' 03-18 11:08:41.255 368 368 F DEBUG : pid: 4289, tid: 4304, name: CrRendererMain >>> org.chromium.chrome:sandboxed_process0 <<< 03-18 11:08:41.255 368 368 F DEBUG : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0xfbadbeef 03-18 11:08:41.268 368 368 F DEBUG : r0 fbadbeef r1 00000000 r2 00004001 r3 00000001 03-18 11:08:41.268 368 368 F DEBUG : r4 2e0af448 r5 26aec6f8 r6 4eb04198 r7 b380abd8 03-18 11:08:41.268 368 368 F DEBUG : r8 b380b344 r9 2e0af448 sl 00000000 fp 32263588 03-18 11:08:41.268 368 368 F DEBUG : ip 938dba6d sp b380ab80 lr 9511ee1d pc 9511ee26 cpsr 60070030 Now reproduced on a public build, created a new issue report for this after the previous one was on an internal build.
,
Mar 18 2016
mikelawther@ believes this should be looked at by font people. But before I dig deeper, rune@, would you have any idea about this one, since dstockwell@ points out you've worked on a similar issue, changing font-cache update triggers.
,
Mar 18 2016
,
Mar 18 2016
,
Mar 18 2016
drott@, yes, the general problem is around fonts and operator== for ComputedStyle. baseLayoutStyle is an optimization we have where we during an animation find out that the only style changes happening for an element is for an ongoing animation. We then store the ComputedStyle for the element without animations applied, and for every animation frame clone that computed style applying animations on top. Thus, we avoid doing selector matching and style application for the rest of the computed style. With asserts enabled, we store the baseLayoutStyle, but do the full styleForElement to check that the optimization is sound. I've been through various fixes for ComputedStyle::operator==() where it would return false when the computed style conceptually is the same while there were some flags/members which were different, yet the optimization was sound. Some of the font comparisons have been moved out of operator== and compared explicitly outside of that operator in places like comparing loading status for custom fonts [1]. I can not reproduce in Linux content_shell. Have you been able to reproduce with an android build? I have just started to fetch for doing an android build. [1] https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/dom/Element.cpp&q=Element::recalcOw&sq=package:chromium&type=cs&l=1633
,
Mar 18 2016
Thanks for looking into this and your explanations. So far I was only able to reproduce this on Android. Because of issue 555855 I can only reproduce color emoji issues on Linux when I do a release build. On Android the assertions are enabled, on Linux they aren't. Looking into gn args --list, I will now try using dcheck_always_on = true on the Linux release build and try to reproduce there.
,
Mar 18 2016
Unfortunately, I can't reproduce this on Linux even with "dcheck_always_on = true" and having verified that the #if ENABLE(ASSERT) parts of ElementAnimations.cpp are compiled in.
,
Mar 21 2016
I've made a debug content shell apk which triggers this. Looking now.
,
Mar 21 2016
Text auto sizing is changing the computed style under our feet.
,
Mar 21 2016
,
Mar 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/32a7b19e92a9ff0fca3aba45c1fdd3896fdd5f1d commit 32a7b19e92a9ff0fca3aba45c1fdd3896fdd5f1d Author: rune <rune@opera.com> Date: Mon Mar 21 19:17:22 2016 Clear baseComputedStyle when text-autosizing changes. baseComputedStyle is an optimization for animations where the computed style before animations are applied is cached and cloned to have cheaper style recalcs for per-frame animation changes. An assumption is that the computed style for the layout object only changes in styleForElement or pseudoStyleForElement. That assumption is not true for text autosizing as the computed style may be changed during layout. Then, for the next animation frame, the text autosizing factor may be different even though the style has not been marked for recalc, and the sanity check for an unchanged baseComputedStyle will trigger an assert. Make sure we clear the baseComputedStyle for an element when the text autosizing factor changes. R=pdr@chromium.org,drott@chromium.org BUG= 596018 Review URL: https://codereview.chromium.org/1816103002 Cr-Commit-Position: refs/heads/master@{#382350} [add] https://crrev.com/32a7b19e92a9ff0fca3aba45c1fdd3896fdd5f1d/third_party/WebKit/LayoutTests/fast/text-autosizing/basecomputedstyle-assert-expected.txt [add] https://crrev.com/32a7b19e92a9ff0fca3aba45c1fdd3896fdd5f1d/third_party/WebKit/LayoutTests/fast/text-autosizing/basecomputedstyle-assert.html [modify] https://crrev.com/32a7b19e92a9ff0fca3aba45c1fdd3896fdd5f1d/third_party/WebKit/Source/core/layout/LayoutObject.cpp [modify] https://crrev.com/32a7b19e92a9ff0fca3aba45c1fdd3896fdd5f1d/third_party/WebKit/Source/core/layout/LayoutObject.h [modify] https://crrev.com/32a7b19e92a9ff0fca3aba45c1fdd3896fdd5f1d/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
,
Mar 22 2016
Thanks! |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by drott@chromium.org
, Mar 18 2016