Skip to content

Commit a2f98aa

Browse files
committed
[Optimize][List] Implement list optimized anchor search algorithm
This commit mainly improves the accuracy of anchor point searching in list component: 1. Added weak_anchor_ref_ field in ItemHolder to store anchor reference ItemHolder for removed ItemHolders. 2. Update anchor reference ItemHolder for all removed on-screen ItemHolders before consuming diff info. 3. In list layout pass, if no valid anchor ItemHolder can be found from all on screen childre, we use the reference anchor ItemHolder as anchor ItemHolder in layout. 4. Add relative unittest and e2e tests. issue: m-6795355659
1 parent d07be3f commit a2f98aa

File tree

15 files changed

+602
-67
lines changed

15 files changed

+602
-67
lines changed

core/renderer/ui_component/list/item_holder.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <array>
99
#include <string>
1010

11+
#include "base/include/flex_optional.h"
1112
#include "base/include/fml/memory/weak_ptr.h"
1213
#include "core/renderer/ui_component/list/list_types.h"
1314

@@ -72,6 +73,15 @@ class ItemHolder : public fml::EnableWeakFromThis<ItemHolder> {
7273
sticky_top_ = sticky_top;
7374
sticky_bottom_ = sticky_bottom;
7475
}
76+
void SetWeakAnchorRef(ItemHolder* item_holder) {
77+
weak_anchor_ref_ = base::make_flex_optional(
78+
item_holder ? item_holder->WeakFromThis() : fml::WeakPtr<ItemHolder>());
79+
}
80+
void ResetWeakAnchorRef() { weak_anchor_ref_.reset(); }
81+
ItemHolder* GetAnchorRefHolder() const {
82+
return weak_anchor_ref_ && (*weak_anchor_ref_) ? (*weak_anchor_ref_).get()
83+
: nullptr;
84+
}
7585
void SetRecyclable(bool recyclable) { recyclable_ = recyclable; }
7686

7787
const std::string& item_key() const { return item_key_; }
@@ -87,6 +97,9 @@ class ItemHolder : public fml::EnableWeakFromThis<ItemHolder> {
8797
bool sticky() const { return sticky_top_ || sticky_bottom_; }
8898
bool sticky_top() const { return sticky_top_; }
8999
bool sticky_bottom() const { return sticky_bottom_; }
100+
base::flex_optional<fml::WeakPtr<ItemHolder>> weak_anchor_ref() const {
101+
return weak_anchor_ref_;
102+
}
90103
bool recyclable() const { return recyclable_; }
91104

92105
// Note: The comparator of ItemHolder should allow objects with the same
@@ -160,6 +173,7 @@ class ItemHolder : public fml::EnableWeakFromThis<ItemHolder> {
160173
// All layout Info of item holder.
161174
// The ItemHolder's index in data source.
162175
int index_{list::kInvalidIndex};
176+
base::flex_optional<fml::WeakPtr<ItemHolder>> weak_anchor_ref_;
163177
// The ItemHold's key.
164178
std::string item_key_;
165179
// Whether the ItemHolder can be recycled.

core/renderer/ui_component/list/list_adapter.cc

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,11 @@ void ListAdapter::UpdateItemHolderToLatest(
258258
if (!list_children_helper) {
259259
return;
260260
}
261+
// Update anchor ref for removed on screen children.
262+
// Note: This logic should be invoked before latest diff info being updated to
263+
// all ItemHolders, so here UpdateAnchorRefItem() is invoked in the begin of
264+
// UpdateItemHolderToLatest()
265+
UpdateAnchorRefItem(list_children_helper);
261266

262267
const auto& children = list_children_helper->children();
263268
const auto& attached_children = list_children_helper->attached_children();
@@ -317,6 +322,88 @@ void ListAdapter::UpdateItemHolderToLatest(
317322
}
318323
}
319324

325+
void ListAdapter::UpdateAnchorRefItem(
326+
ListChildrenHelper* list_children_helper) {
327+
const auto& children = list_children_helper->children();
328+
const auto& on_screen_children = list_children_helper->on_screen_children();
329+
bool has_valid_diff = list_adapter_helper()->HasValidDiff();
330+
bool should_search_ref_anchor = list_container_->ShouldSearchRefAnchor();
331+
list::SearchRefAnchorStrategy search_strategy =
332+
list_container_->search_ref_anchor_strategy();
333+
std::unordered_map<ItemHolder*, ItemHolder*> tmp_anchor_ref_map;
334+
if (has_valid_diff && should_search_ref_anchor &&
335+
!on_screen_children.empty() && !children.empty()) {
336+
list_children_helper->ForEachChild(
337+
on_screen_children,
338+
[this, list_children_helper, &children, &tmp_anchor_ref_map,
339+
search_strategy](ItemHolder* on_screen_child) {
340+
// We only need to update removed on screen child.
341+
if (IsRemoved(on_screen_child)) {
342+
auto weak_anchor_ref = on_screen_child->weak_anchor_ref();
343+
if (!weak_anchor_ref) {
344+
// (1) Removed child's weak_anchor_ref is never set.
345+
if (auto it = children.find(on_screen_child);
346+
it != children.end()) {
347+
ItemHolder* anchor_ref_child =
348+
list_children_helper->GetFirstChildFrom(
349+
children, on_screen_child,
350+
[this, on_screen_child](const ItemHolder* item_holder) {
351+
return item_holder != on_screen_child &&
352+
!IsRemoved(item_holder);
353+
},
354+
search_strategy ==
355+
list::SearchRefAnchorStrategy::kToStart);
356+
on_screen_child->SetWeakAnchorRef(anchor_ref_child);
357+
} else {
358+
// Unexpected case: child is not in children set.
359+
NLIST_LOGE(
360+
"ListAdapter::UpdateItemHolderToLatest: on_screen_child is "
361+
"not at children set and it's weak_anchor_ref is never "
362+
"set: "
363+
"index="
364+
<< on_screen_child->index()
365+
<< ", item-key=" << on_screen_child->item_key());
366+
on_screen_child->SetWeakAnchorRef(nullptr);
367+
}
368+
} else if (weak_anchor_ref && (*weak_anchor_ref)) {
369+
// (2) Update weak_anchor_ref if needed.
370+
ItemHolder* current_anchor_ref_child = (*weak_anchor_ref).get();
371+
if (IsRemoved(current_anchor_ref_child)) {
372+
// Current anchor ref child is removed, need to find a new
373+
// anchor ref.
374+
if (auto it = tmp_anchor_ref_map.find(current_anchor_ref_child);
375+
it != tmp_anchor_ref_map.end()) {
376+
// Fast find new ref anchor child from tmp_anchor_ref_map
377+
on_screen_child->SetWeakAnchorRef(it->second);
378+
} else {
379+
if (auto it = children.find(current_anchor_ref_child);
380+
it != children.end()) {
381+
ItemHolder* new_anchor_ref_child =
382+
list_children_helper->GetFirstChildFrom(
383+
children, current_anchor_ref_child,
384+
[this, current_anchor_ref_child](
385+
const ItemHolder* item_holder) {
386+
return item_holder != current_anchor_ref_child &&
387+
!IsRemoved(item_holder);
388+
},
389+
search_strategy ==
390+
list::SearchRefAnchorStrategy::kToStart);
391+
tmp_anchor_ref_map[current_anchor_ref_child] =
392+
new_anchor_ref_child;
393+
on_screen_child->SetWeakAnchorRef(new_anchor_ref_child);
394+
} else {
395+
tmp_anchor_ref_map[current_anchor_ref_child] = nullptr;
396+
on_screen_child->SetWeakAnchorRef(nullptr);
397+
}
398+
}
399+
}
400+
}
401+
}
402+
return false;
403+
});
404+
}
405+
}
406+
320407
// Mark all child ItemHolders's diff status.
321408
void ListAdapter::MarkChildHolderDirty() {
322409
TRACE_EVENT(LYNX_TRACE_CATEGORY, LIST_ADAPTER_MARK_CHILD_DIRTY);

core/renderer/ui_component/list/list_adapter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ class ListAdapter : public AdapterHelper::Delegate {
212212

213213
bool HasExpectedDiffAnimation() const;
214214

215+
void UpdateAnchorRefItem(ListChildrenHelper* list_children_helper);
216+
215217
protected:
216218
Element* list_element_{nullptr};
217219
ListContainerImpl* list_container_{nullptr};

core/renderer/ui_component/list/list_adapter_unittest.cc

Lines changed: 147 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// Licensed under the Apache License Version 2.0 that can be found in the
33
// LICENSE file in the root directory of this source tree.
44

5+
#define private public
6+
#define protected public
7+
58
#include <memory>
69
#include <unordered_map>
710
#include <utility>
@@ -13,7 +16,6 @@
1316
#include "core/renderer/dom/fiber/component_element.h"
1417
#include "core/renderer/dom/fiber/list_element.h"
1518
#include "core/renderer/tasm/react/testing/mock_painting_context.h"
16-
#include "core/renderer/ui_component/list/default_list_adapter.h"
1719
#include "core/renderer/ui_component/list/list_container_impl.h"
1820
#include "core/renderer/ui_component/list/testing/mock_diff_result.h"
1921
#include "core/renderer/ui_component/list/testing/mock_list_element.h"
@@ -36,9 +38,8 @@ class ListAdapterTest : public ::testing::Test {
3638
std::unique_ptr<lynx::tasm::ElementManager> manager;
3739
std::shared_ptr<::testing::NiceMock<test::MockTasmDelegate>> tasm_mediator;
3840
fml::RefPtr<list::MockListElement> list_element_ref;
39-
std::unique_ptr<DefaultListAdapter> default_list_adapter;
4041
std::unique_ptr<ListContainerImpl> list_container;
41-
std::unique_ptr<ListChildrenHelper> list_children_helper;
42+
ListChildrenHelper* list_children_helper;
4243

4344
protected:
4445
ListAdapterTest() = default;
@@ -63,22 +64,7 @@ class ListAdapterTest : public ::testing::Test {
6364
enqueue_component, component_at_indexes));
6465
list_container =
6566
std::make_unique<ListContainerImpl>(list_element_ref.get());
66-
default_list_adapter = std::make_unique<DefaultListAdapter>(
67-
list_container.get(), list_element_ref.get());
68-
list_children_helper = std::make_unique<ListChildrenHelper>();
69-
}
70-
71-
public:
72-
fml::RefPtr<Element> CreateListItemElement() {
73-
static int32_t base_css_id = 0;
74-
base::String component_id(std::to_string(++base_css_id));
75-
base::String entry_name("__ListItem__");
76-
base::String component_name("__ListItem__");
77-
base::String path("/index/components/ListItem");
78-
auto list_item_ref = manager->CreateFiberComponent(
79-
component_id, base_css_id, entry_name, component_name, path);
80-
list_element_ref->AddChildAt(list_item_ref, -1);
81-
return list_item_ref;
67+
list_children_helper = list_container->list_children_helper();
8268
}
8369
}; // ListAdapterTest
8470

@@ -94,37 +80,46 @@ TEST_F(ListAdapterTest, DiffCase) {
9480
.sticky_bottoms = {7, 8},
9581
.full_spans = {0, 1, 7, 8},
9682
};
97-
default_list_adapter->UpdateDataSource(
83+
list_container->list_adapter_->UpdateDataSource(
9884
lepus_value(diff_result.GenerateDiffResult()));
99-
EXPECT_EQ(default_list_adapter->list_adapter_helper()->item_keys().size(),
85+
EXPECT_EQ(
86+
list_container->list_adapter_->list_adapter_helper()->item_keys().size(),
87+
diff_result.GetItemCount());
88+
EXPECT_EQ(list_container->list_adapter_->list_adapter_helper()
89+
->item_key_map()
90+
.size(),
10091
diff_result.GetItemCount());
101-
EXPECT_EQ(default_list_adapter->list_adapter_helper()->item_key_map().size(),
92+
EXPECT_EQ(list_container->list_adapter_->GetDataCount(),
10293
diff_result.GetItemCount());
103-
EXPECT_EQ(default_list_adapter->GetDataCount(), diff_result.GetItemCount());
104-
EXPECT_EQ(default_list_adapter->list_adapter_helper()
94+
EXPECT_EQ(list_container->list_adapter_->list_adapter_helper()
10595
->estimated_heights_px()
10696
.size(),
10797
diff_result.GetItemCount());
98+
EXPECT_EQ(list_container->list_adapter_->list_adapter_helper()
99+
->estimated_sizes_px()
100+
.size(),
101+
diff_result.GetItemCount());
108102
EXPECT_EQ(
109-
default_list_adapter->list_adapter_helper()->estimated_sizes_px().size(),
110-
diff_result.GetItemCount());
111-
EXPECT_EQ(default_list_adapter->list_adapter_helper()->full_spans().size(),
112-
diff_result.full_spans.size());
113-
EXPECT_EQ(default_list_adapter->list_adapter_helper()->sticky_tops().size(),
103+
list_container->list_adapter_->list_adapter_helper()->full_spans().size(),
104+
diff_result.full_spans.size());
105+
EXPECT_EQ(list_container->list_adapter_->list_adapter_helper()
106+
->sticky_tops()
107+
.size(),
114108
diff_result.sticky_tops.size());
115-
EXPECT_EQ(default_list_adapter->GetStickyTops().size(),
109+
EXPECT_EQ(list_container->list_adapter_->GetStickyTops().size(),
116110
diff_result.sticky_tops.size());
117-
EXPECT_EQ(
118-
default_list_adapter->list_adapter_helper()->sticky_bottoms().size(),
119-
diff_result.sticky_bottoms.size());
120-
EXPECT_EQ(default_list_adapter->GetStickyBottoms().size(),
111+
EXPECT_EQ(list_container->list_adapter_->list_adapter_helper()
112+
->sticky_bottoms()
113+
.size(),
114+
diff_result.sticky_bottoms.size());
115+
EXPECT_EQ(list_container->list_adapter_->GetStickyBottoms().size(),
121116
diff_result.sticky_bottoms.size());
122-
EXPECT_TRUE(default_list_adapter->HasFullSpanItems());
123-
default_list_adapter->UpdateItemHolderToLatest(list_children_helper.get());
117+
EXPECT_TRUE(list_container->list_adapter_->HasFullSpanItems());
118+
list_container->list_adapter_->UpdateItemHolderToLatest(list_children_helper);
124119
for (int index = 0; index < static_cast<int>(diff_result.GetItemCount());
125120
++index) {
126121
ItemHolder* item_holder =
127-
default_list_adapter->GetItemHolderForIndex(index);
122+
list_container->list_adapter_->GetItemHolderForIndex(index);
128123
EXPECT_NE(item_holder, nullptr);
129124
if (index == 0 || index == 1) {
130125
EXPECT_TRUE(item_holder->sticky_top());
@@ -134,6 +129,121 @@ TEST_F(ListAdapterTest, DiffCase) {
134129
}
135130
}
136131

132+
TEST_F(ListAdapterTest, UpdateAnchorRefItemTest) {
133+
list_container->search_ref_anchor_strategy_ =
134+
list::SearchRefAnchorStrategy::kToStart;
135+
list::InsertAction insert_action;
136+
list::RemoveAction remove_action;
137+
list::UpdateAction update_action;
138+
insert_action = {.insert_ops_ = {
139+
{.position_ = 0, "A_0", 100, false, false, false, false},
140+
{.position_ = 1, "B_1", 100, false, false, false, false},
141+
{.position_ = 2, "C_2", 100, false, false, false, false},
142+
{.position_ = 3, "D_3", 100, false, false, false, false},
143+
{.position_ = 4, "E_4", 100, false, false, false, false},
144+
{.position_ = 5, "F_5", 100, false, false, false, false},
145+
{.position_ = 6, "G_6", 100, false, false, false, false},
146+
{.position_ = 7, "H_7", 100, false, false, false, false},
147+
{.position_ = 8, "I_8", 100, false, false, false, false},
148+
{.position_ = 9, "J_9", 100, false, false, false, false},
149+
}};
150+
list::FiberDiffResult fiber_diff_result_0{
151+
.insert_action_ = insert_action,
152+
};
153+
list_container->list_adapter_->UpdateFiberDataSource(
154+
lepus::Value(fiber_diff_result_0.Resolve()));
155+
list_container->list_adapter_->UpdateItemHolderToLatest(list_children_helper);
156+
157+
// init on screen children: E_4, F_5, G_6, H_7
158+
list_children_helper->on_screen_children_.insert(
159+
list_container->GetItemHolderForIndex(4));
160+
list_children_helper->on_screen_children_.insert(
161+
list_container->GetItemHolderForIndex(5));
162+
list_children_helper->on_screen_children_.insert(
163+
list_container->GetItemHolderForIndex(6));
164+
list_children_helper->on_screen_children_.insert(
165+
list_container->GetItemHolderForIndex(7));
166+
167+
// remove C_2, E_4, F_5, H_7
168+
// insert New_A_0, New_B_1, New_C_2, New_D_3
169+
insert_action = {
170+
.insert_ops_ = {
171+
{.position_ = 2, "New_A_0", 100, false, false, false, false},
172+
{.position_ = 3, "New_B_1", 100, false, false, false, false},
173+
{.position_ = 4, "New_C_2", 100, false, false, false, false},
174+
{.position_ = 5, "New_D_3", 100, false, false, false, false},
175+
}};
176+
remove_action = {
177+
.remove_ops_ = {2, 4, 5, 7},
178+
};
179+
list::FiberDiffResult fiber_diff_result_1{
180+
.insert_action_ = insert_action,
181+
.remove_action_ = remove_action,
182+
};
183+
list_container->list_adapter_->UpdateFiberDataSource(
184+
lepus::Value(fiber_diff_result_1.Resolve()));
185+
list_container->list_adapter_->UpdateItemHolderToLatest(list_children_helper);
186+
187+
ItemHolder* holder_C2 =
188+
list_container->list_adapter_->item_holder_map_->at("C_2").get();
189+
ItemHolder* holder_E4 =
190+
list_container->list_adapter_->item_holder_map_->at("E_4").get();
191+
ItemHolder* holder_F5 =
192+
list_container->list_adapter_->item_holder_map_->at("F_5").get();
193+
ItemHolder* holder_G6 =
194+
list_container->list_adapter_->item_holder_map_->at("G_6").get();
195+
ItemHolder* holder_H7 =
196+
list_container->list_adapter_->item_holder_map_->at("H_7").get();
197+
// C_2 is not on screen children set, so it has no anchor ref item.
198+
EXPECT_EQ(holder_C2->weak_anchor_ref().has_value(), false);
199+
// E_4 and F_5 are on screen children set and they are removed in this diff,
200+
// so they have anchor ref item with D_3.
201+
EXPECT_EQ(holder_E4->weak_anchor_ref().has_value(), true);
202+
EXPECT_TRUE(holder_E4->weak_anchor_ref().value()->item_key() == "D_3");
203+
EXPECT_EQ(holder_F5->weak_anchor_ref().has_value(), true);
204+
EXPECT_TRUE(holder_F5->weak_anchor_ref().value().get()->item_key() == "D_3");
205+
// G_6 is not removed in this diff, so it has no anchor ref item.
206+
EXPECT_EQ(holder_G6->weak_anchor_ref().has_value(), false);
207+
// H_7 is on screen children set and removed in this diff, so it has anchor
208+
// ref item with G_6.
209+
EXPECT_EQ(holder_H7->weak_anchor_ref().has_value(), true);
210+
EXPECT_TRUE(holder_H7->weak_anchor_ref().value().get()->item_key() == "G_6");
211+
212+
// remove D_3
213+
remove_action = {
214+
.remove_ops_ = {6},
215+
};
216+
list::FiberDiffResult fiber_diff_result_2{
217+
.remove_action_ = remove_action,
218+
};
219+
list_container->list_adapter_->UpdateFiberDataSource(
220+
lepus::Value(fiber_diff_result_2.Resolve()));
221+
list_container->list_adapter_->UpdateItemHolderToLatest(list_children_helper);
222+
// D_3 is removed in this diff, so E_4 and F_5 should update anchor ref item
223+
// to New_D_3.
224+
EXPECT_EQ(holder_E4->weak_anchor_ref().has_value(), true);
225+
EXPECT_TRUE(holder_E4->weak_anchor_ref().value()->item_key() == "New_D_3");
226+
EXPECT_EQ(holder_F5->weak_anchor_ref().has_value(), true);
227+
EXPECT_TRUE(holder_F5->weak_anchor_ref().value()->item_key() == "New_D_3");
228+
229+
// remove G_6
230+
remove_action = {
231+
.remove_ops_ = {6},
232+
};
233+
list::FiberDiffResult fiber_diff_result_3{
234+
.remove_action_ = remove_action,
235+
};
236+
list_container->list_adapter_->UpdateFiberDataSource(
237+
lepus::Value(fiber_diff_result_3.Resolve()));
238+
list_container->list_adapter_->UpdateItemHolderToLatest(list_children_helper);
239+
// G_6 is removed in this diff, so G_6 and H_7 should update anchor ref item
240+
// to New_G_6.
241+
EXPECT_EQ(holder_G6->weak_anchor_ref().has_value(), true);
242+
EXPECT_TRUE(holder_G6->weak_anchor_ref().value()->item_key() == "New_D_3");
243+
EXPECT_EQ(holder_H7->weak_anchor_ref().has_value(), true);
244+
EXPECT_TRUE(holder_H7->weak_anchor_ref().value()->item_key() == "New_D_3");
245+
}
246+
137247
} // namespace testing
138248
} // namespace tasm
139249
} // namespace lynx

0 commit comments

Comments
 (0)