Commit a77a2f59 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot

[LayoutNG] Fix float double-painting crash

The crash trigger was Legacy and NG painting the same float.

This is how it happens:
NGBlockNode::CopyChildFragmentPosition adds float to ContainingBlock()
float list.
Float might propagate to ContainingBlock()'s ancestor and set the
ShouldPaint() flag. How this happens is still mysterious to me, the
obvious suspect UpdateAncestorShouldPaintFloatingObject does not
get called.

NG will paint the float if ContainingBlock has PaintFragment().

My fix is a bit of a hack: in Legacy, if we know that float will
get painted by NG, do not paint it.

This fixes the crash. Test still fails, but that is block layout
algorithm problem.

Bug: 864398
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I7ebe82a2188baaa076476e4f5cbd78bab7127875
Reviewed-on: https://chromium-review.googlesource.com/1244246
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594428}
parent fc23b592
......@@ -338,7 +338,7 @@ crbug.com/591099 external/wpt/xhr/send-authentication-prompt-2-manual.htm [ Fail
crbug.com/591099 external/wpt/xhr/send-content-type-string.htm [ Pass ]
crbug.com/591099 external/wpt/xhr/send-entity-body-document.htm [ Pass ]
crbug.com/591099 fast/backgrounds/quirks-mode-line-box-backgrounds.html [ Failure ]
crbug.com/591099 fast/block/float-avoids-padding-inline-ancestors.html [ Crash ]
crbug.com/591099 fast/block/float-avoids-padding-inline-ancestors.html [ Failure ]
crbug.com/591099 fast/block/float/nopaint-after-layer-destruction.html [ Failure ]
crbug.com/591099 fast/block/float/nopaint-after-layer-destruction2.html [ Failure ]
crbug.com/591099 fast/block/float/overlapping-floats-paint-hittest-order-1.html [ Failure ]
......
......@@ -10,6 +10,7 @@
#include "third_party/blink/renderer/core/layout/api/line_layout_api_shim.h"
#include "third_party/blink/renderer/core/layout/api/line_layout_box.h"
#include "third_party/blink/renderer/core/layout/layout_inline.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.h"
#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/core/paint/box_painter.h"
#include "third_party/blink/renderer/core/paint/line_box_list_painter.h"
......@@ -293,6 +294,12 @@ void BlockPainter::PaintBlockFlowContents(const PaintInfo& paint_info,
// TODO(wangxianzhu): Should this be a DCHECK?
if (floating_layout_object->HasSelfPaintingLayer())
continue;
// Do not paint floats that will be painted by NG.
LayoutBlock* containing_block = floating_layout_object->ContainingBlock();
if (containing_block->IsLayoutBlockFlow() &&
ToLayoutBlockFlow(containing_block)->PaintFragment()) {
continue;
}
ObjectPainter(*floating_layout_object)
.PaintAllPhasesAtomically(float_paint_info);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment