Commit 2ed7f658 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[FragmentItem] Add move ctor for |NGFragmentItem|

r782445 crrev.com/c/2266052 changed to use copy instead of
move when reusing previous layout results because we can't
break them.

There is another case we copy |NGFragmentItem|, when
|NGFragmentItemsBuilder| creates a new |NGFragmentItems|.
In this case, it is safe to break |NGFragmentItem| in
|NGFragmentItems|.

This patch adds move constructors to support moving in
this case. The caller, |NGFragmentItems::NGFragmentItems|
already use |std::move|.
https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/layout/ng/inline/ng_fragment_items.cc;l=23?q=NGFragmentItems::NGFragmentItems

Bug: 982194
Change-Id: Iee89fb228459ab37144824f2809f33726a4a2543
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2286249
Commit-Queue: Koji Ishii <kojii@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786204}
parent fa6d6d34
......@@ -223,6 +223,38 @@ NGFragmentItem::NGFragmentItem(const NGFragmentItem& source)
}
}
NGFragmentItem::NGFragmentItem(NGFragmentItem&& source)
: layout_object_(source.layout_object_),
rect_(source.rect_),
ink_overflow_(source.InkOverflowType(), std::move(source.ink_overflow_)),
fragment_id_(source.fragment_id_),
delta_to_next_for_same_layout_object_(
source.delta_to_next_for_same_layout_object_),
type_(source.type_),
sub_type_(source.sub_type_),
style_variant_(source.style_variant_),
is_hidden_for_paint_(source.is_hidden_for_paint_),
text_direction_(source.text_direction_),
ink_overflow_type_(source.ink_overflow_type_),
is_dirty_(source.is_dirty_),
is_last_for_node_(source.is_last_for_node_) {
switch (Type()) {
case kText:
new (&text_) TextItem(std::move(source.text_));
break;
case kGeneratedText:
new (&generated_text_)
GeneratedTextItem(std::move(source.generated_text_));
break;
case kLine:
new (&line_) LineItem(std::move(source.line_));
break;
case kBox:
new (&box_) BoxItem(std::move(source.box_));
break;
}
}
NGFragmentItem::~NGFragmentItem() {
switch (Type()) {
case kText:
......
......@@ -76,8 +76,9 @@ class CORE_EXPORT NGFragmentItem {
// Create a line item.
explicit NGFragmentItem(const NGPhysicalLineBoxFragment& line);
// The copy constructor.
// The copy/move constructors.
NGFragmentItem(const NGFragmentItem&);
NGFragmentItem(NGFragmentItem&&);
~NGFragmentItem();
......@@ -372,6 +373,8 @@ class CORE_EXPORT NGFragmentItem {
String ToString() const;
private:
FRIEND_TEST_ALL_PREFIXES(NGFragmentItemTest, CopyMove);
// Create a text item.
NGFragmentItem(const NGInlineItem& inline_item,
scoped_refptr<const ShapeResultView> shape_result,
......
......@@ -83,6 +83,67 @@ class NGFragmentItemTest : public NGLayoutTest,
}
};
TEST_F(NGFragmentItemTest, CopyMove) {
SetBodyInnerHTML(R"HTML(
<style>
div {
font-size: 20px;
line-height: 10px;
}
</style>
<div id="container">
1234567
</div>
)HTML");
LayoutBlockFlow* container =
To<LayoutBlockFlow>(GetLayoutObjectByElementId("container"));
NGInlineCursor cursor(*container);
// Test copying a line item.
cursor.MoveToFirstLine();
const NGFragmentItem* line_item = cursor.Current().Item();
EXPECT_EQ(line_item->Type(), NGFragmentItem::kLine);
EXPECT_NE(line_item->LineBoxFragment(), nullptr);
NGFragmentItem copy_of_line(*line_item);
EXPECT_EQ(copy_of_line.LineBoxFragment(), line_item->LineBoxFragment());
// Ink overflow is not copied for line items. See |NGFragmentItem| copy ctor.
EXPECT_FALSE(copy_of_line.IsInkOverflowComputed());
// Test moving a line item.
NGFragmentItem move_of_line(std::move(copy_of_line));
EXPECT_EQ(move_of_line.LineBoxFragment(), line_item->LineBoxFragment());
// After the move, the source fragment should be released.
EXPECT_EQ(copy_of_line.LineBoxFragment(), nullptr);
EXPECT_FALSE(move_of_line.IsInkOverflowComputed());
// To test moving ink overflow, add an ink overflow to |move_of_line|.
PhysicalRect not_small_ink_overflow_rect(0, 0, 5000, 100);
move_of_line.ink_overflow_type_ = move_of_line.ink_overflow_.SetContents(
move_of_line.InkOverflowType(), not_small_ink_overflow_rect,
line_item->Size());
EXPECT_EQ(move_of_line.InkOverflowType(), NGInkOverflow::kContents);
NGFragmentItem move_of_line2(std::move(move_of_line));
EXPECT_EQ(move_of_line2.InkOverflowType(), NGInkOverflow::kContents);
EXPECT_EQ(move_of_line2.InkOverflow(), not_small_ink_overflow_rect);
// Test copying a text item.
cursor.MoveToFirstChild();
const NGFragmentItem* text_item = cursor.Current().Item();
EXPECT_EQ(text_item->Type(), NGFragmentItem::kText);
EXPECT_NE(text_item->TextShapeResult(), nullptr);
NGFragmentItem copy_of_text(*text_item);
EXPECT_EQ(copy_of_text.TextShapeResult(), text_item->TextShapeResult());
// Ink overflow is copied for text items. See |NGFragmentItem| copy ctor.
EXPECT_TRUE(copy_of_text.IsInkOverflowComputed());
// Test moving a text item.
NGFragmentItem move_of_text(std::move(copy_of_text));
EXPECT_EQ(move_of_text.TextShapeResult(), text_item->TextShapeResult());
// After the move, the source ShapeResult should be released.
EXPECT_EQ(copy_of_text.TextShapeResult(), nullptr);
EXPECT_TRUE(move_of_text.IsInkOverflowComputed());
}
TEST_F(NGFragmentItemTest, BasicText) {
LoadAhem();
SetBodyInnerHTML(R"HTML(
......
......@@ -66,6 +66,36 @@ NGInkOverflow::NGInkOverflow(Type source_type, const NGInkOverflow& source) {
SetType(source_type);
}
NGInkOverflow::NGInkOverflow(Type source_type, NGInkOverflow&& source) {
source.CheckType(source_type);
new (this) NGInkOverflow();
switch (source_type) {
case kNotSet:
case kNone:
break;
case kSmallSelf:
case kSmallContents:
static_assert(sizeof(outsets_) == sizeof(single_),
"outsets should be the size of a pointer");
single_ = source.single_;
#if DCHECK_IS_ON()
for (wtf_size_t i = 0; i < base::size(outsets_); ++i)
DCHECK_EQ(outsets_[i], source.outsets_[i]);
#endif
break;
case kSelf:
case kContents:
single_ = source.single_;
source.single_ = nullptr;
break;
case kSelfAndContents:
container_ = source.container_;
source.container_ = nullptr;
break;
}
SetType(source_type);
}
NGInkOverflow::Type NGInkOverflow::Reset(Type type) {
CheckType(type);
switch (type) {
......
......@@ -75,8 +75,9 @@ class CORE_EXPORT NGInkOverflow {
NGInkOverflow(const NGInkOverflow&) = delete;
NGInkOverflow& operator=(const NGInkOverflow&) = delete;
// To copy, |Type| is required.
// To copy/move, |Type| is required.
NGInkOverflow(Type source_type, const NGInkOverflow& source);
NGInkOverflow(Type source_type, NGInkOverflow&& source);
// Get ink overflow of various types.
PhysicalRect Self(Type type, const PhysicalSize& size) const;
......
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