New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 778352 link

Starred by 9 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Animating font-variation-settings eventually crashes the tab

Project Member Reported by n...@chromium.org, Oct 25 2017

Issue description

Chrome Version: 62.0.3202.62
OS: Mac

I have a demo (https://variable-lizards.glitch.me/) that animates the font-variation settings of 15 divs. The divs each contain a 🦎 with the Zycon font, and each div has 2 animations, one that animates font-variation-settings with a keyframe (and one that does a transform). If you leave this demo open after about 5 minutes the tab tends to crash. I initially reproed this in Canary, and then stable, so I don't think this is a recent regression. 

I don't really know how to debug this. It doesn't reliably all the time, unfortunately, but after about 2 minutes the animation definitely gets janky, so I wonder if it's leaking memory somewhere?
 

Comment 1 by n...@chromium.org, Oct 25 2017

I can make the tab crash more reliably if I just leave it open and start using the browser (navigation, checking calendar, etc). Eventually when I come back to it, it's crashed. I think this may be a crash ID?
b632db0ee65b033d (Local Crash ID: 9efeb780-cb49-431c-929b-7c36aa8304fd)

Comment 2 by n...@chromium.org, Oct 25 2017

Cc: mathias@chromium.org
Labels: Needs-Triage-M62

Comment 4 by e...@chromium.org, Oct 30 2017

Owner: drott@chromium.org
Status: Assigned (was: Untriaged)

Comment 5 by drott@chromium.org, Nov 29 2017

Running this with
https://chromium-review.googlesource.com/c/chromium/src/+/793819

and 

$ out/gndebug/content_shell --enable-logging=stderr --vmodule=FontObjectsCount=4,FontCache=4 https://variable-lizards.glitch.me/


and keeping it running for a bit:


[1:1:1129/132337.965806:INFO:FontCache.cpp(105)] Num FontCache entries: 239640
[1:1:0100/000000.970044:INFO:FontObjectsCount.h(54)] Num FontPlatformData objects: 478719
[1:1:0100/000000.987094:INFO:FontObjectsCount.h(54)] Num SimpleFontData objects: 239078

Related to issue 789180. So unfortunately we have lots of FontPlatformData leakage when animating variable fonts. I am currently drafting a better Font objects life cycle overall, and fixing this issue is one of the requirements.

Comment 6 by drott@chromium.org, Dec 13 2017

Re #1, this is indeed an OOM crash, reported by v8: v8::internal::V8::FatalProcessOutOfMemory(char const*, bool)

Comment 7 by drott@chromium.org, Dec 13 2017

One important memory reduction up in https://chromium-review.googlesource.com/c/chromium/src/+/824172

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 13 2017

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

commit 0d4a27248ff0b9aace5be15c041cd21e9a0443fa
Author: Dominik Röttsches <drott@chromium.org>
Date: Wed Dec 13 17:00:21 2017

Avoid unbounded growth of font_data_table in CSSFontFaceSource

Animating or using an ever increasing number of instances of variable
fonts can make the font_data_table_ in CSSFontFaceSource grow without
bounds. Set a limit, and remove least recently used entries from the
table. In my experiments this reduces the tab memory leakage for the URL
from issue 778352 to a third and should help keep the tab running a lot
longer.

Bug: 778352
Change-Id: I9a812afdffb318be977def0b2c0301321603ab15
Reviewed-on: https://chromium-review.googlesource.com/824172
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523793}
[modify] https://crrev.com/0d4a27248ff0b9aace5be15c041cd21e9a0443fa/third_party/WebKit/Source/core/css/CSSFontFaceSource.cpp
[modify] https://crrev.com/0d4a27248ff0b9aace5be15c041cd21e9a0443fa/third_party/WebKit/Source/core/css/CSSFontFaceSource.h
[modify] https://crrev.com/0d4a27248ff0b9aace5be15c041cd21e9a0443fa/third_party/WebKit/Source/core/css/CSSFontFaceSourceTest.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 14 2017

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

commit 9faa2ca77263b22ff002452f545d217b4c7a0f63
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Thu Dec 14 04:52:18 2017

Fix potential use-after-free in CSSFontFaceSource

This fixes a bug introduced by crrev.com/c/824172.

Bug: 778352
Change-Id: Id1dbc552ed5a94a0b5fadddec0b5bea50c3a7597
Reviewed-on: https://chromium-review.googlesource.com/826662
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524007}
[modify] https://crrev.com/9faa2ca77263b22ff002452f545d217b4c7a0f63/third_party/WebKit/Source/core/css/CSSFontFaceSource.cpp

Comment 10 by drott@chromium.org, Dec 14 2017

noms@, the improvement from #8 should be in Canary, would you be up for giving it a go?

Comment 11 by n...@chromium.org, Jan 2 2018

Ok, so it still crashes eventually, but I'd say it takes a longer time to do that. Trying to get a crash ID for you

Comment 12 by n...@chromium.org, Jan 2 2018

I can't get a crash ID, but i would say it takes about 11-15 minutes to crash the tab now (my repro steps are literally "load the demo in a tab on my second monitor, wait")

Comment 13 by drott@chromium.org, Jun 19 2018

 Issue 852855  has been merged into this issue.
Cc: grieger@google.com dcrossland@google.com rsheeter@google.com
 Issue 875127  has been merged into this issue.
Cc: phanindra.mandapaka@chromium.org
 Issue 891309  has been merged into this issue.
Is there any progress on this issue? Or a timeline for further work on it?

As it stands, animating variant fonts is currently practically impossible because of the huge memory leak. We had plans for a project using variant fonts, but the lack of action on this ticket is really disappointing.

If you need a test case, I have provided one here: 
https://bugs.chromium.org/p/chromium/issues/detail?id=891309
Labels: -Pri-3 OS-Mac Pri-2
Yes, there is a plan to work on this. This issue primarily affects Mac at the moment, animating variable fonts on other platforms is much less affected, and for example on ChromeOS works for a long time without issues.
Dear Dominik, thanks for the very fast response!
Indeed it seems this is mostly a Mac issue.

As we are currently in the concept phase for the above mentioned project -- which would make for a great showcase for variable fonts, I promise ;) -- it would be important to know for us roughly when this will be addressed. Is there anything you can say about that?

Thanks again, best,
Philip
It's high on my todo list but I can't promise a fixed timeline for a fix. Best is to keep following this issue for updates.
Thanks a lot for having an eye on this.

In the meantime it seems we found a feasible workaround:
https://codepen.io/anon/pen/oQMJRg

Instead of using css animations/transitions, if you define a fixed list of classes with different font variants from the same variable font, the font variants seem to be cached correctly so there is no memory leak.
Interesting approach - thanks for sharing! I am not an expert on animation performance, but perhaps combining your example with requestAnimationFrame() instead of setTimeout() helps with animation performance as well as with power efficiency when the tab is backgrounded.

Oh sure, sorry for not mentioning this, that was just a quick proof of concept to show that it helps against the memory leak, it is far from production ready ode.
Great test Philip, thank you! I assume that this works because it's re-using the exact same values, instead of "real" CSS animation which lands on different floating point values for every frame.

Sign in to add a comment