Commit 26a0ce3d authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Store fragmented NGPaintFragment as a list

This patch stores fragmented NGPaintFragment as a list linked
from LayoutNGMixin.

The code path doesn't run (break_token is always nullptr) in
phase 1. But without keeping them, virtual/layout_ng_experimental
will crash randomly when CL:1170726 caches NGPaintFragment in
LayoutObject.

Bug: 714962
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: I6c7e4934dcb366d7dcb39412bf1aac40ebad7429
Reviewed-on: https://chromium-review.googlesource.com/1178081
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583980}
parent caa44de3
...@@ -2535,9 +2535,11 @@ void LayoutBlockFlow::SetCachedLayoutResult(const NGConstraintSpace&, ...@@ -2535,9 +2535,11 @@ void LayoutBlockFlow::SetCachedLayoutResult(const NGConstraintSpace&,
scoped_refptr<NGLayoutResult>) {} scoped_refptr<NGLayoutResult>) {}
void LayoutBlockFlow::SetPaintFragment( void LayoutBlockFlow::SetPaintFragment(
const NGBreakToken*,
scoped_refptr<const NGPhysicalFragment>) {} scoped_refptr<const NGPhysicalFragment>) {}
void LayoutBlockFlow::UpdatePaintFragmentFromCachedLayoutResult( void LayoutBlockFlow::UpdatePaintFragmentFromCachedLayoutResult(
const NGBreakToken*,
scoped_refptr<const NGPhysicalFragment>) {} scoped_refptr<const NGPhysicalFragment>) {}
void LayoutBlockFlow::ComputeOverflow(LayoutUnit old_client_after_edge, void LayoutBlockFlow::ComputeOverflow(LayoutUnit old_client_after_edge,
......
...@@ -460,10 +460,11 @@ class CORE_EXPORT LayoutBlockFlow : public LayoutBlock { ...@@ -460,10 +460,11 @@ class CORE_EXPORT LayoutBlockFlow : public LayoutBlock {
NGBreakToken*, NGBreakToken*,
scoped_refptr<NGLayoutResult>); scoped_refptr<NGLayoutResult>);
virtual void WillCollectInlines() {} virtual void WillCollectInlines() {}
virtual void SetPaintFragment(scoped_refptr<const NGPhysicalFragment>); virtual void SetPaintFragment(const NGBreakToken*,
scoped_refptr<const NGPhysicalFragment>);
virtual void UpdatePaintFragmentFromCachedLayoutResult( virtual void UpdatePaintFragmentFromCachedLayoutResult(
const NGBreakToken*,
scoped_refptr<const NGPhysicalFragment>); scoped_refptr<const NGPhysicalFragment>);
virtual void ClearPaintFragment() {}
virtual const NGPhysicalBoxFragment* CurrentFragment() const { virtual const NGPhysicalBoxFragment* CurrentFragment() const {
return nullptr; return nullptr;
} }
......
...@@ -261,30 +261,72 @@ LayoutNGMixin<Base>::CachedLayoutResultForTesting() { ...@@ -261,30 +261,72 @@ LayoutNGMixin<Base>::CachedLayoutResultForTesting() {
template <typename Base> template <typename Base>
void LayoutNGMixin<Base>::SetPaintFragment( void LayoutNGMixin<Base>::SetPaintFragment(
scoped_refptr<const NGPhysicalFragment> fragment) { NGPaintFragment* last_paint_fragment,
DCHECK(fragment); std::unique_ptr<NGPaintFragment> paint_fragment) {
if (paint_fragment) {
// When paint fragment is replaced, the subtree needs paint invalidation to
// re-compute paint properties in NGPaintFragment.
Base::SetSubtreeShouldDoFullPaintInvalidation();
}
paint_fragment_ = NGPaintFragment::Create(std::move(fragment)); if (last_paint_fragment) {
last_paint_fragment->SetNext(std::move(paint_fragment));
} else {
paint_fragment_ = std::move(paint_fragment);
}
}
// When paint fragment is replaced, the subtree needs paint invalidation to template <typename Base>
// re-compute paint properties in NGPaintFragment. void LayoutNGMixin<Base>::SetPaintFragment(
Base::SetSubtreeShouldDoFullPaintInvalidation(); const NGBreakToken* break_token,
scoped_refptr<const NGPhysicalFragment> fragment) {
// TODO(kojii): There are cases where the first call has break_token.
// Investigate why and handle appropriately.
// DCHECK(!break_token || paint_fragment_);
NGPaintFragment* last_paint_fragment = nullptr;
if (break_token && paint_fragment_) {
last_paint_fragment = paint_fragment_->Last(*break_token);
// TODO(kojii): Sometimes an unknown break_token is given. Need to
// investigate why, and handle appropriately. For now, just keep it to avoid
// crashes and use-after-free.
if (!last_paint_fragment)
last_paint_fragment = paint_fragment_->Last();
DCHECK(last_paint_fragment);
}
SetPaintFragment(last_paint_fragment,
fragment ? NGPaintFragment::Create(fragment) : nullptr);
} }
template <typename Base> template <typename Base>
void LayoutNGMixin<Base>::UpdatePaintFragmentFromCachedLayoutResult( void LayoutNGMixin<Base>::UpdatePaintFragmentFromCachedLayoutResult(
const NGBreakToken* break_token,
scoped_refptr<const NGPhysicalFragment> fragment) { scoped_refptr<const NGPhysicalFragment> fragment) {
DCHECK(fragment); DCHECK(fragment);
// TODO(kojii): There are cases where the first call has break_token.
// Investigate why and handle appropriately.
// DCHECK(!break_token || paint_fragment_);
NGPaintFragment* paint_fragment = nullptr;
NGPaintFragment* last_paint_fragment = nullptr;
if (!break_token) {
paint_fragment = paint_fragment_.get();
} else if (paint_fragment_) {
last_paint_fragment = paint_fragment_->Last(*break_token);
// TODO(kojii): Sometimes an unknown break_token is given. Need to
// investigate why, and handle appropriately. For now, just keep it to avoid
// crashes and use-after-free.
if (!last_paint_fragment)
last_paint_fragment = paint_fragment_->Last();
DCHECK(last_paint_fragment);
paint_fragment = last_paint_fragment->Next();
}
if (!paint_fragment_) if (!paint_fragment) {
return SetPaintFragment(fragment); SetPaintFragment(last_paint_fragment,
NGPaintFragment::Create(std::move(fragment)));
paint_fragment_->UpdatePhysicalFragmentFromCachedLayoutResult(fragment); return;
} }
template <typename Base> paint_fragment->UpdatePhysicalFragmentFromCachedLayoutResult(fragment);
void LayoutNGMixin<Base>::ClearPaintFragment() {
paint_fragment_ = nullptr;
} }
template <typename Base> template <typename Base>
......
...@@ -70,10 +70,11 @@ class LayoutNGMixin : public Base { ...@@ -70,10 +70,11 @@ class LayoutNGMixin : public Base {
NGPaintFragment* PaintFragment() const override { NGPaintFragment* PaintFragment() const override {
return paint_fragment_.get(); return paint_fragment_.get();
} }
void SetPaintFragment(scoped_refptr<const NGPhysicalFragment>) final; void SetPaintFragment(const NGBreakToken*,
scoped_refptr<const NGPhysicalFragment>) final;
void UpdatePaintFragmentFromCachedLayoutResult( void UpdatePaintFragmentFromCachedLayoutResult(
const NGBreakToken*,
scoped_refptr<const NGPhysicalFragment>) final; scoped_refptr<const NGPhysicalFragment>) final;
void ClearPaintFragment() final;
protected: protected:
bool IsOfType(LayoutObject::LayoutObjectType) const override; bool IsOfType(LayoutObject::LayoutObjectType) const override;
...@@ -82,6 +83,8 @@ class LayoutNGMixin : public Base { ...@@ -82,6 +83,8 @@ class LayoutNGMixin : public Base {
private: private:
void AddScrollingOverflowFromChildren(); void AddScrollingOverflowFromChildren();
void SetPaintFragment(NGPaintFragment* last_paint_fragment,
std::unique_ptr<NGPaintFragment>);
protected: protected:
void AddOutlineRects( void AddOutlineRects(
......
...@@ -218,7 +218,7 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout( ...@@ -218,7 +218,7 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout(
if (!constraint_space.IsIntermediateLayout() && first_child && if (!constraint_space.IsIntermediateLayout() && first_child &&
first_child.IsInline()) { first_child.IsInline()) {
block_flow->UpdatePaintFragmentFromCachedLayoutResult( block_flow->UpdatePaintFragmentFromCachedLayoutResult(
layout_result->PhysicalFragment()); break_token, layout_result->PhysicalFragment());
} }
return layout_result; return layout_result;
} }
...@@ -261,7 +261,7 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout( ...@@ -261,7 +261,7 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout(
layout_result); layout_result);
if (layout_result->Status() == NGLayoutResult::kSuccess && if (layout_result->Status() == NGLayoutResult::kSuccess &&
!constraint_space.IsIntermediateLayout()) !constraint_space.IsIntermediateLayout())
block_flow->ClearPaintFragment(); block_flow->SetPaintFragment(break_token, nullptr);
} }
if (IsBlockLayoutComplete(constraint_space, *layout_result)) { if (IsBlockLayoutComplete(constraint_space, *layout_result)) {
...@@ -275,7 +275,8 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout( ...@@ -275,7 +275,8 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout(
scrollbars.block_start, scrollbars.block_start,
Style().IsFlippedBlocksWritingMode()); Style().IsFlippedBlocksWritingMode());
block_flow->SetPaintFragment(layout_result->PhysicalFragment()); block_flow->SetPaintFragment(break_token,
layout_result->PhysicalFragment());
} }
// TODO(kojii): Even when we paint fragments, there seem to be some data we // TODO(kojii): Even when we paint fragments, there seem to be some data we
......
...@@ -196,6 +196,28 @@ void NGPaintFragment::UpdatePhysicalFragmentFromCachedLayoutResult( ...@@ -196,6 +196,28 @@ void NGPaintFragment::UpdatePhysicalFragmentFromCachedLayoutResult(
physical_fragment_ = fragment; physical_fragment_ = fragment;
} }
NGPaintFragment* NGPaintFragment::Last(const NGBreakToken& break_token) {
for (NGPaintFragment* fragment = this; fragment;
fragment = fragment->next_fragmented_.get()) {
if (fragment->PhysicalFragment().BreakToken() == &break_token)
return fragment;
}
return nullptr;
}
NGPaintFragment* NGPaintFragment::Last() {
for (NGPaintFragment* fragment = this;;) {
NGPaintFragment* next = fragment->next_fragmented_.get();
if (!next)
return fragment;
fragment = next;
}
}
void NGPaintFragment::SetNext(std::unique_ptr<NGPaintFragment> fragment) {
next_fragmented_ = std::move(fragment);
}
bool NGPaintFragment::IsDescendantOfNotSelf( bool NGPaintFragment::IsDescendantOfNotSelf(
const NGPaintFragment& ancestor) const { const NGPaintFragment& ancestor) const {
for (const NGPaintFragment* fragment = Parent(); fragment; for (const NGPaintFragment* fragment = Parent(); fragment;
......
...@@ -49,6 +49,12 @@ class CORE_EXPORT NGPaintFragment : public DisplayItemClient, ...@@ -49,6 +49,12 @@ class CORE_EXPORT NGPaintFragment : public DisplayItemClient,
void UpdatePhysicalFragmentFromCachedLayoutResult( void UpdatePhysicalFragmentFromCachedLayoutResult(
scoped_refptr<const NGPhysicalFragment>); scoped_refptr<const NGPhysicalFragment>);
// Next/last fragment for when this is fragmented.
NGPaintFragment* Next() { return next_fragmented_.get(); }
void SetNext(std::unique_ptr<NGPaintFragment>);
NGPaintFragment* Last();
NGPaintFragment* Last(const NGBreakToken&);
// The parent NGPaintFragment. This is nullptr for a root; i.e., when parent // The parent NGPaintFragment. This is nullptr for a root; i.e., when parent
// is not for NGPaint. In the first phase, this means that this is a root of // is not for NGPaint. In the first phase, this means that this is a root of
// an inline formatting context. // an inline formatting context.
...@@ -237,6 +243,9 @@ class CORE_EXPORT NGPaintFragment : public DisplayItemClient, ...@@ -237,6 +243,9 @@ class CORE_EXPORT NGPaintFragment : public DisplayItemClient,
NGPaintFragment* parent_; NGPaintFragment* parent_;
Vector<std::unique_ptr<NGPaintFragment>> children_; Vector<std::unique_ptr<NGPaintFragment>> children_;
// The next fragment for when this is fragmented.
std::unique_ptr<NGPaintFragment> next_fragmented_;
NGPaintFragment* next_fragment_ = nullptr; NGPaintFragment* next_fragment_ = nullptr;
NGPhysicalOffset inline_offset_to_container_box_; NGPhysicalOffset inline_offset_to_container_box_;
......
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