Skip to content

Commit

Permalink
BREAKING: Release ComponentTree through binder when RecyclerCollectio…
Browse files Browse the repository at this point in the history
…nComponent's @OnDetached is called.

Summary:
When RecyclerCollectionComponent's onDetached() callback is called, we should release ComponentTrees in the sections as well. Add a method called `release` in Binder.java that RecyclerCollectionComponent can access it to release ComponentTrees under the hood.

The release() method in ComponentTreeHolder has been removed in D14183719, but I think we need to add it back. It's not just used to triggering OnDetached method, but also used to remove the pending runnables posted in the handlers(https://fburl.com/4spmzpwu).

Reviewed By: pasqualeanatriello

Differential Revision: D13848269

fbshipit-source-id: 0060ddb8cc0341cf7963e0b5fb4982bc1f9675f4
  • Loading branch information
Jing-Wei Wu authored and facebook-github-bot committed Apr 15, 2019
1 parent 51b8d43 commit 8893049
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5046,6 +5046,49 @@ public void testOnAttachedOnDetachedWithViewportChanged() {
}
}

@Test
public void testItemsOnDetached() {
final int childHeightPx = 20;
final int widthPx = 200;
final int heightPx = 200;

mRecyclerBinder = new RecyclerBinder.Builder().rangeRatio(RANGE_RATIO).build(mComponentContext);
final RecyclerView rv = mock(RecyclerView.class);
mRecyclerBinder.mount(rv);

final List<RenderInfo> renderInfos = new ArrayList<>();
final List<TestComponent> components = new ArrayList<>();
for (int i = 0; i < 200; i++) {
final TestComponent component =
TestAttachDetachComponent.create(mComponentContext).heightPx(childHeightPx).build();
components.add(component);
final Component child = Column.create(mComponentContext).child(component).build();
renderInfos.add(ComponentRenderInfo.create().component(child).build());
}
mRecyclerBinder.insertRangeAt(0, renderInfos);
mRecyclerBinder.notifyChangeSetComplete(true, NO_OP_CHANGE_SET_COMPLETE_CALLBACK);

Size size = new Size();
int widthSpec = SizeSpec.makeSizeSpec(widthPx, SizeSpec.EXACTLY);
int heightSpec = SizeSpec.makeSizeSpec(heightPx, SizeSpec.EXACTLY);
mRecyclerBinder.measure(size, widthSpec, heightSpec, null);

mLayoutThreadShadowLooper.runToEndOfTasks();
ShadowLooper.runUiThreadTasks();

mRecyclerBinder.detach();

final int rangeSize = heightPx / childHeightPx;
final int rangeStart = 0;
final int rangeTotal = (int) (rangeSize + (RANGE_RATIO * rangeSize));
for (int i = rangeStart; i <= rangeStart + rangeTotal; i++) {
assertThat(components.get(i).wasOnDetachedCalled()).isTrue();
}
for (int i = rangeStart + rangeTotal + 1; i < 200; i++) {
assertThat(components.get(i).wasOnDetachedCalled()).isFalse();
}
}

private RecyclerBinder createRecyclerBinderWithMockAdapter(RecyclerView.Adapter adapterMock) {
return new RecyclerBinder.Builder()
.rangeRatio(RANGE_RATIO)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import com.facebook.litho.annotations.LayoutSpec;
import com.facebook.litho.annotations.OnCreateInitialState;
import com.facebook.litho.annotations.OnCreateLayout;
import com.facebook.litho.annotations.OnDetached;
import com.facebook.litho.annotations.OnEvent;
import com.facebook.litho.annotations.OnTrigger;
import com.facebook.litho.annotations.OnUpdateState;
Expand Down Expand Up @@ -399,6 +400,11 @@ static void onScroll(
sectionTree.requestFocusOnRoot(position);
}

@OnDetached
static void onDetached(ComponentContext c, @State Binder<RecyclerView> binder) {
binder.detach();
}

private static class RecyclerCollectionOnScrollListener extends OnScrollListener {

private final RecyclerCollectionEventsController mEventsController;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ public boolean supportsBackgroundChangeSets() {
return mUseBackgroundChangeSets;
}

@Override
public void detach() {
mRecyclerBinder.detach();
}

public void clear() {
if (mUseBackgroundChangeSets) {
mRecyclerBinder.clearAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,7 @@ void measure(
* view and the first item will need to be measured to determine the height of the view.
*/
void setCanMeasure(boolean canMeasure);

/** Detach items under the hood. */
void detach();
}
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,20 @@ public void setCanMeasure(boolean canMeasure) {
mCanMeasure = canMeasure;
}

@Override
public void detach() {
final List<ComponentTreeHolder> toDetach = new ArrayList<>();
synchronized (this) {
for (int i = 0, size = mComponentTreeHolders.size(); i < size; i++) {
toDetach.add(mComponentTreeHolders.get(i));
}
}

for (int i = 0, size = toDetach.size(); i < size; i++) {
toDetach.get(i).acquireStateAndReleaseTree();
}
}

@UiThread
public void notifyItemRenderCompleteAt(int position, final long timestampMillis) {
final ComponentTreeHolder holder = mComponentTreeHolders.get(position);
Expand Down

0 comments on commit 8893049

Please sign in to comment.