Skip to content

Commit

Permalink
Oilpanize blink::ListGrid::GridTrack
Browse files Browse the repository at this point in the history 
Changed GridTrack to on-heap object by inheriting GridLinkedListNodeBase
and replacing wtf::DoublyLinkedList with blink::GridLinkedList.

Bug: 1306747
Change-Id: Ib29bab7d0f1e5d3e6c312f599ce095d1b1608ee3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3567808
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Commit-Queue: Chika Okuda <okudachika@google.com>
Cr-Commit-Position: refs/heads/main@{#989362}
  • Loading branch information
Chika Okuda authored and Chromium LUCI CQ committed Apr 6, 2022
1 parent 7357e59 commit 92dff99
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 39 deletions.
28 changes: 10 additions & 18 deletions third_party/blink/renderer/core/layout/grid.cc
View file
Expand Up @@ -231,7 +231,7 @@ GridLinkedList<ListGrid::GridCell>::AddResult ListGrid::GridTrack::InsertAfter(
const GridItemList& ListGrid::Cell(wtf_size_t row_index,
wtf_size_t column_index) const {
DEFINE_STATIC_LOCAL(const GridItemList, empty_vector, ());
for (auto* row = rows_.Head(); row; row = row->Next()) {
for (auto* row = rows_->Head(); row; row = row->Next()) {
if (row->Index() == row_index) {
GridCell* cell = row->Find(column_index);
return cell ? cell->Items() : empty_vector;
Expand All @@ -243,7 +243,7 @@ const GridItemList& ListGrid::Cell(wtf_size_t row_index,
}

ListGrid::GridTrack* ListGrid::InsertTracks(
DoublyLinkedList<GridTrack>& tracks,
GridLinkedList<GridTrack>& tracks,
const GridSpan& span,
GridTrackSizingDirection direction) {
auto compare_tracks = [](ListGrid::GridTrack* first,
Expand All @@ -254,8 +254,8 @@ ListGrid::GridTrack* ListGrid::InsertTracks(
wtf_size_t start_line = span.StartLine();
wtf_size_t end_line = span.EndLine();

DoublyLinkedList<ListGrid::GridTrack>::AddResult result = tracks.Insert(
base::WrapUnique(new GridTrack(start_line, direction)), compare_tracks);
GridLinkedList<ListGrid::GridTrack>::AddResult result = tracks.Insert(
MakeGarbageCollected<GridTrack>(start_line, direction), compare_tracks);
auto* track = result.node;
DCHECK(track);

Expand All @@ -264,7 +264,7 @@ ListGrid::GridTrack* ListGrid::InsertTracks(
++track_index) {
if (!iter->Next() || track_index < iter->Next()->Index()) {
tracks.InsertAfter(
base::WrapUnique(new GridTrack(track_index, direction)), iter);
MakeGarbageCollected<GridTrack>(track_index, direction), iter);
}
iter = iter->Next();
}
Expand All @@ -277,9 +277,9 @@ void ListGrid::Insert(LayoutBox& item, const GridArea& area) {
area.columns.IsTranslatedDefinite());
EnsureGridSize(area.rows.EndLine(), area.columns.EndLine());

GridTrack* first_row = InsertTracks(rows_, area.rows, kForRows);
GridTrack* first_row = InsertTracks(*rows_, area.rows, kForRows);
DCHECK(first_row);
GridTrack* first_column = InsertTracks(columns_, area.columns, kForColumns);
GridTrack* first_column = InsertTracks(*columns_, area.columns, kForColumns);
DCHECK(first_column);

GridCell* above_cell = nullptr;
Expand Down Expand Up @@ -319,16 +319,8 @@ void ListGrid::EnsureGridSize(wtf_size_t maximum_row_size,

void ListGrid::ClearGridDataStructure() {
num_rows_ = num_columns_ = 0;
while (!rows_.IsEmpty())
delete rows_.RemoveHead();
DCHECK(rows_.IsEmpty());
while (!columns_.IsEmpty())
delete columns_.RemoveHead();
DCHECK(columns_.IsEmpty());
}

ListGrid::~ListGrid() {
ClearGridDataStructure();
rows_->Clear();
columns_->Clear();
}

void ListGrid::GridCell::SetTraversalMode(GridTrackSizingDirection direction) {
Expand Down Expand Up @@ -369,7 +361,7 @@ LayoutBox* ListGridIterator::NextGridItem() {

bool is_row_axis = direction_ == kForColumns;
if (!cell_node_) {
auto* track = is_row_axis ? grid_.columns_.Head() : grid_.rows_.Head();
auto* track = is_row_axis ? grid_.columns_->Head() : grid_.rows_->Head();
DCHECK(track);
const wtf_size_t fixed_index = is_row_axis ? column_index_ : row_index_;
while (track && track->Index() != fixed_index)
Expand Down
34 changes: 17 additions & 17 deletions third_party/blink/renderer/core/layout/grid.h
View file
Expand Up @@ -13,7 +13,6 @@
#include "third_party/blink/renderer/core/style/grid_positions_resolver.h"
#include "third_party/blink/renderer/platform/heap/persistent.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
#include "third_party/blink/renderer/platform/wtf/doubly_linked_list.h"
#include "third_party/blink/renderer/platform/wtf/linked_hash_set.h"
#include "third_party/blink/renderer/platform/wtf/vector.h"

Expand Down Expand Up @@ -160,7 +159,10 @@ class CORE_EXPORT Grid {
// the track index.
class CORE_EXPORT ListGrid final : public Grid {
public:
explicit ListGrid(const LayoutGrid* grid) : Grid(grid) {}
explicit ListGrid(const LayoutGrid* grid)
: Grid(grid),
rows_(MakeGarbageCollected<GridLinkedList<GridTrack>>()),
columns_(MakeGarbageCollected<GridLinkedList<GridTrack>>()) {}

wtf_size_t NumTracks(GridTrackSizingDirection direction) const override {
return direction == kForRows ? num_rows_ : num_columns_;
Expand All @@ -170,8 +172,6 @@ class CORE_EXPORT ListGrid final : public Grid {
void EnsureGridSize(wtf_size_t maximum_row_size,
wtf_size_t maximum_column_size) override;

~ListGrid() final;

// This is the class representing a cell in the grid. GridCell's are
// only created for those cells which do have items inside. Each
// GridCell will be part of two different DLL, one representing the
Expand Down Expand Up @@ -235,17 +235,20 @@ class CORE_EXPORT ListGrid final : public Grid {
// index of the cell in the orthogonal direction, i.e., the list of
// cells in a GridTrack representing a column will be sorted by
// their row index.
class CORE_EXPORT GridTrack final : public DoublyLinkedListNode<GridTrack> {
USING_FAST_MALLOC(GridTrack);
friend class WTF::DoublyLinkedListNode<GridTrack>;

class CORE_EXPORT GridTrack final : public GridLinkedListNodeBase<GridTrack> {
public:
GridTrack(wtf_size_t index, GridTrackSizingDirection direction)
: cells_(MakeGarbageCollected<GridLinkedList<GridCell>>()),
index_(index),
direction_(direction) {}

wtf_size_t Index() const { return index_; }

void Trace(Visitor* visitor) const final {
visitor->Trace(cells_);
GridLinkedListNodeBase<GridTrack>::Trace(visitor);
}

GridLinkedList<GridCell>::AddResult Insert(GridCell*);
GridLinkedList<GridCell>::AddResult InsertAfter(GridCell* cell,
GridCell* insertion_point);
Expand All @@ -255,28 +258,25 @@ class CORE_EXPORT ListGrid final : public Grid {
const GridLinkedList<GridCell>& Cells() const { return *cells_; }

private:
Persistent<GridLinkedList<GridCell>> cells_;
Member<GridLinkedList<GridCell>> cells_;
wtf_size_t index_;
GridTrackSizingDirection direction_;

GridTrack* prev_;
GridTrack* next_;
};

private:
friend class ListGridIterator;

// Returns a pointer to the first track.
GridTrack* InsertTracks(DoublyLinkedList<GridTrack>&,
GridTrack* InsertTracks(GridLinkedList<GridTrack>&,
const GridSpan&,
GridTrackSizingDirection);

void ClearGridDataStructure() override;
void ConsolidateGridDataStructure() override {}

const DoublyLinkedList<GridTrack>& Tracks(
const GridLinkedList<GridTrack>& Tracks(
GridTrackSizingDirection direction) const {
return direction == kForRows ? rows_ : columns_;
return direction == kForRows ? *rows_ : *columns_;
}

std::unique_ptr<GridIterator> CreateIterator(
Expand All @@ -287,8 +287,8 @@ class CORE_EXPORT ListGrid final : public Grid {
wtf_size_t num_rows_{0};
wtf_size_t num_columns_{0};

DoublyLinkedList<GridTrack> columns_;
DoublyLinkedList<GridTrack> rows_;
Persistent<GridLinkedList<GridTrack>> rows_;
Persistent<GridLinkedList<GridTrack>> columns_;
};

class ListGridIterator final : public Grid::GridIterator {
Expand Down
7 changes: 7 additions & 0 deletions third_party/blink/renderer/core/layout/grid_linked_list.h
View file
Expand Up @@ -102,6 +102,7 @@ class GridLinkedList : public GarbageCollected<GridLinkedList<NodeType>> {
NodeType* Tail() const { return tail_; }

bool IsEmpty() { return !head_; }
void Clear();

// Returns the size of the list. O(n).
int Size();
Expand Down Expand Up @@ -156,6 +157,12 @@ class GridLinkedList : public GarbageCollected<GridLinkedList<NodeType>> {
Member<NodeType> tail_;
};

template <typename NodeType>
void GridLinkedList<NodeType>::Clear() {
head_.Clear();
tail_.Clear();
}

template <typename NodeType>
int GridLinkedList<NodeType>::Size() {
int len = 0;
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/layout/grid_linked_list_test.cc
View file
Expand Up @@ -152,6 +152,11 @@ TEST_F(GridLinkedListTest, Insert) {
EXPECT_FALSE(gll->Insert(num2_again, IntNode::Compare));
EXPECT_EQ(gll->Insert(num2_again, IntNode::Compare).node, num2);
EXPECT_TRUE(IsSorted(gll, IntNode::Compare));

gll->Clear();
DCHECK(gll->IsEmpty());
ThreadState::Current()->CollectAllGarbageForTesting();
EXPECT_EQ(4, IntNode::destructor_calls);
}

} // namespace blink
8 changes: 4 additions & 4 deletions third_party/blink/renderer/core/layout/grid_test.cc
View file
Expand Up @@ -339,14 +339,14 @@ TEST_F(GridTest, CellInsert) {
if (RuntimeEnabledFeatures::LayoutNGEnabled())
return;

auto track = base::WrapUnique(new ListGrid::GridTrack(0, kForColumns));
ListGrid::GridCell* cell = MakeGarbageCollected<ListGrid::GridCell>(0, 0);
auto* track = MakeGarbageCollected<ListGrid::GridTrack>(0, kForColumns);
auto* cell = MakeGarbageCollected<ListGrid::GridCell>(0, 0);

auto result = track->Insert(cell);
EXPECT_TRUE(result.is_new_entry);
EXPECT_EQ(cell, result.node);

ListGrid::GridCell* cell2 = MakeGarbageCollected<ListGrid::GridCell>(1, 0);
auto* cell2 = MakeGarbageCollected<ListGrid::GridCell>(1, 0);
result = track->Insert(cell2);
EXPECT_TRUE(result.is_new_entry);
EXPECT_EQ(cell2, result.node);
Expand All @@ -355,7 +355,7 @@ TEST_F(GridTest, CellInsert) {
EXPECT_FALSE(result.is_new_entry);
EXPECT_EQ(cell2, result.node);

ListGrid::GridCell* cell3 = MakeGarbageCollected<ListGrid::GridCell>(2, 0);
auto* cell3 = MakeGarbageCollected<ListGrid::GridCell>(2, 0);
result = track->InsertAfter(cell3, cell2);
EXPECT_TRUE(result.is_new_entry);
EXPECT_EQ(cell3, result.node);
Expand Down

0 comments on commit 92dff99

Please sign in to comment.