SkiaBitLocker should avoid allocating huge offscreen buffer |
||||||
Issue descriptionI discovered this problem when I worked on a SPv2 workaround that disables layer culling. A few layout tests crashed, but only on Mac. Turns out the test contains a huge text area and we allocate bitmap on Mac for the framework to paint native borders for it. The partial stack trace looked like this: worker/1 virtual/spv2/fast/overflow/overflow-height-float-not-removed-crash2.html output stderr lines: [17488:1287:0419/212622:8790474006246:FATAL:skia_utils_mac.mm(445)] Check failed: result. 0 Content Shell Framework 0x000000010ebd99e3 _ZN4base5debug10StackTraceC1Ev + 19 1 Content Shell Framework 0x000000010ebf3629 _ZN7logging10LogMessageD2Ev + 73 2 Content Shell Framework 0x000000010f30cec7 _ZN4skia13SkiaBitLocker9cgContextEv + 1127 3 Content Shell Framework 0x000000010fe89057 _ZN5blink27LocalCurrentGraphicsContextC2EP8SkCanvasfPKNS_7IntRectERS4_ + 199 4 Content Shell Framework 0x0000000111677767 _ZN5blink15ThemePainterMac13paintTextAreaERKNS_12LayoutObjectERKNS_9PaintInfoERKNS_7IntRectE + 39 5 Content Shell Framework 0x0000000111674c5f _ZN5blink12ThemePainter15paintBorderOnlyERKNS_12LayoutObjectERKNS_9PaintInfoERKNS_7IntRectE + 79 6 Content Shell Framework 0x000000011161aa9a _ZN5blink10BoxPainter36paintBoxDecorationBackgroundWithRectERKNS_9PaintInfoERKNS_11LayoutPointERKNS_10LayoutRectE + 1322 This problem can be reproduced in vanilla Chromium with this minimal test case: http://jsbin.com/vinazi/ The test case exploits non-composited transforms to explode cull rect to some huge value. IMO in LocalCurrentGraphicsContext we should compute bitmapScaleFactor_ based on canvas CTM. Also we should have some sane limit on offscreen buffer size and fail gracefully when the limit is exceeded.
,
Apr 22 2016
Issue 427743 has been merged into this issue.
,
Apr 22 2016
This issue came up when developing SPv1. The workaround was to clip at the interest rect. This looks like another instance.
,
Apr 22 2016
The recording canvas CTM is not going to reflect the actual device space CTM though - this is the fundamental problem of not knowing what the device space transform is during recording, no? For that fiddle, by the time we hit the theme paint code we have a recording canvas sized 1000000x1000000 with a scale of 1. The .01 downscale is applied externally, outside of the current DrawingDisplayItem, so it doesn't help. And Mike points out that even if we did have access to the screen CTM, one can always construct a large enough text area that crashes the allocation even without a scale. I think clamping makes sense, but ideally we should try to avoiding these native allocations (and record time rasterization) altogether.
,
Sep 8 2016
Tangentially, SkiaBitLocker is mac-only, and skia::ScopedPlatformPaint does more-or-less the same thing on all platforms. There's likely duplicate code here we can get rid of.
,
May 11 2017
Unassigning self from bugs that I don't expect to be able to get to soon in case someone else is able to pick them up.
,
May 11 2017
Does SkiaBitLocker still exist? Can we close this bug?
,
May 11 2017
I don't see it in code base any more. Possibly it's been moved/renamed? There are still these two tests in TestExpectations. We need to see whether they are passing and if not decide if impactful. crbug.com/605812 [ Mac ] virtual/spv2/fast/overflow/overflow-height-float-not-removed-crash.html [ Skip ] crbug.com/605812 [ Mac ] virtual/spv2/fast/overflow/overflow-height-float-not-removed-crash2.html [ Skip ]
,
May 11 2017
It was there a year ago in skia/ext/skia_utils_mac.mm, but it looks like Enne moved it to Blink in https://codereview.chromium.org/270572300.
,
May 14 2018
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 14 2018
We don't crash. I think we just let this be. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by reed@google.com
, Apr 22 2016