New issue
Advanced search Search tips

Issue 916948 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

[LayoutNG] Crash in blink::NGInlineBoxFragmentPainter::PaintBackgroundBorderShadow

Project Member Reported by kojii@chromium.org, Dec 20

Issue description

Cc: ajha@chromium.org e...@chromium.org
Labels: -Type-Bug -Pri-3 ReleaseBlock-Stable Target-73 M-73 FoundIn-73 OS-Mac OS-Windows Pri-1 Type-Bug-Regression
Seeing sudden spike in the crashes with this magic signature on recent canary. Suspecting this spike is due to recently enabled LayoutNG experiment. 

Link to the list of the builds with this crash: https://goto.google.com/hemkp
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 4

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

commit df6641f245478c0da2a03a0d7458df620d4ae1dd
Author: Emil A Eklund <eae@chromium.org>
Date: Fri Jan 04 18:44:17 2019

[LayoutNG] Handle empty FragmentRange in PaintBackgroundBorderShadow

Update NGInlineBoxFragmentPainter::PaintBackgroundBorderShadow to safely
handle cases where the FragmentRange is empty. This shouldn't happen but
assuming the layout object has a single fragment in those cases is safe.

Bug:  916948 
Change-Id: I2e57b468a6d5137f843a61dbd9a02de117af3c20
Reviewed-on: https://chromium-review.googlesource.com/c/1396323
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620010}
[modify] https://crrev.com/df6641f245478c0da2a03a0d7458df620d4ae1dd/third_party/blink/renderer/core/paint/ng/ng_inline_box_fragment_painter.cc

Status: Fixed (was: Assigned)
Cc: vamshi.kommuri@chromium.org
Labels: Needs-Feedback
Tried checking the issue on chrome version 73.0.3659.0(expecting this version without fix) using Mac 10.14.1 with the below mentioned steps.
1. Launched Chrome
2. Navigated to https://www.windowscentral.com/these-are-my-essential-surface-laptop-accessories-everyday-use
3. Scrolled multiple times.
In the process we didn't observe any crash.

@kojii: Please let us know if we have missed anything in the process and Could you please check the issue on latest chrome & help us in verifying the fix.

Thanks!


Labels: -Needs-Feedback -ReleaseBlock-Stable
It only crashes (or used to crash) with the LayoutNG experiment on.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 10

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

commit 0af0ddd9c4e8b244eba706ef6a019808065a8c23
Author: Koji Ishii <kojii@chromium.org>
Date: Thu Jan 10 09:21:47 2019

[LayoutNG] Clear association of fragments before new one is created

This patch changes the timing of clearing associations
between LayoutObject and NGPaintFragment to before new
NGPaintFragment is created.

Before this change, it was cleared before layout. However,
fragment caching may skip creating new NGPaintFragment, and
in such cases, association will not be created. When this
happens, some LayoutObject look as if they did not produce
fragments.

Also this change should reduce the number of clearing
associations since |NGInlineNode::Layout()| runs for each
line while |LayoutNGMixin::SetPaintFragment()| runs for each
inline formatting context (per fragmentainer.)

This patch also improves support for block fragmentations but
only to pass tests that pass before this change.

Bug:  916948 
Change-Id: Ie7d364cab32c5c56f558a1525e967b760272360d
Reviewed-on: https://chromium-review.googlesource.com/c/1391250
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621515}
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/layout_block_flow.cc
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/layout_block_flow.h
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.cc
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.h
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.h
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/ng/ng_block_break_token.cc
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/ng/ng_block_break_token.h
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/layout/ng/ng_block_node.cc
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/paint/ng/ng_inline_box_fragment_painter.cc
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/paint/ng/ng_paint_fragment.cc
[modify] https://crrev.com/0af0ddd9c4e8b244eba706ef6a019808065a8c23/third_party/blink/renderer/core/paint/ng/ng_paint_fragment.h

Sign in to add a comment