Skip to content

Commit

Permalink
BREAKING: Only enable incremental mount if parent incremental mount i…
Browse files Browse the repository at this point in the history
…s enabled

Summary:
We do not currently handle the case well where a parent ComponentTree (e.g. hscroll) has incremental mount turned off, but its children (e.g. the items in the hscroll) all have it turned on. In this case, the children mount what is visible on the first frame they appear, and never receive any other incremental mount callbacks.

The solution here is to provide a way to pass through whether the parent context has incremental mount disabled, and use that to set whether it's enabled/disabled on the child ComponentTree

Reviewed By: pasqualeanatriello

Differential Revision: D14875607

fbshipit-source-id: 3018a71839198af3cf44be034a4cde11ff236232
  • Loading branch information
astreet authored and facebook-github-bot committed Apr 11, 2019
1 parent 47baa36 commit c88a660
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 5 deletions.
30 changes: 27 additions & 3 deletions litho-core/src/main/java/com/facebook/litho/ComponentContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ public interface YogaNodeFactory {
@ThreadConfined(ThreadConfined.ANY)
private int mDefStyleAttr = 0;

// The parent in this case is the ComponentTree that this ComponentContext/ComponentTree is nested
// within. For example, in an HScroll, the ComponentTree for each item in the list has a parent
// of the ComponentTree for the HScroll itself. If it has incremental mount disabled, it won't
// work for the children either so we need to disable it there as well.
private final boolean mIsParentIncrementalMountDisabled;

private ComponentTree.LayoutStateFuture mLayoutStateFuture;

public ComponentContext(
Expand All @@ -86,7 +92,8 @@ public ComponentContext(
@Nullable StateHandler stateHandler,
@Nullable KeyHandler keyHandler,
@Nullable TreeProps treeProps,
YogaNodeFactory yogaNodeFactory) {
YogaNodeFactory yogaNodeFactory,
boolean isParentIncrementalMountDisabled) {

if (logger != null && logTag == null) {
throw new IllegalStateException("When a ComponentsLogger is set, a LogTag must be set");
Expand All @@ -100,6 +107,18 @@ public ComponentContext(
mStateHandler = stateHandler;
mKeyHandler = keyHandler;
mYogaNodeFactory = yogaNodeFactory;
mIsParentIncrementalMountDisabled = isParentIncrementalMountDisabled;
}

public ComponentContext(
Context context,
@Nullable String logTag,
ComponentsLogger logger,
@Nullable StateHandler stateHandler,
@Nullable KeyHandler keyHandler,
@Nullable TreeProps treeProps,
YogaNodeFactory yogaNodeFactory) {
this(context, logTag, logger, stateHandler, keyHandler, treeProps, yogaNodeFactory, false);
}

public ComponentContext(
Expand All @@ -126,6 +145,7 @@ public ComponentContext(
mKeyHandler = keyHandler != null ? keyHandler : context.mKeyHandler;
mTreeProps = treeProps != null ? treeProps : context.mTreeProps;
mLayoutStateFuture = layoutStateFuture == null ? context.mLayoutStateFuture : layoutStateFuture;
mIsParentIncrementalMountDisabled = context.mIsParentIncrementalMountDisabled;
}

public ComponentContext(
Expand Down Expand Up @@ -463,6 +483,10 @@ void applyStyle(InternalNode node, @AttrRes int defStyleAttr, @StyleRes int defS
}
}

boolean isParentIncrementalMountDisabled() {
return mIsParentIncrementalMountDisabled;
}

static ComponentContext withComponentTree(ComponentContext context, ComponentTree componentTree) {
ComponentContext componentContext =
new ComponentContext(context, new StateHandler(), null, null, null);
Expand Down Expand Up @@ -494,8 +518,8 @@ public static ComponentContext withComponentScope(ComponentContext context, Comp
* you require that incremental mount is enabled (e.g. you use visibility callbacks). This is
* static to avoid polluting the ComponentContext API.
*/
public static boolean isIncrementalMountEnabled(ComponentContext c) {
return c.getComponentTree().isIncrementalMountEnabled();
public static boolean isIncrementalMountDisabled(ComponentContext c) {
return c.mComponentTree != null && !c.mComponentTree.isIncrementalMountEnabled();
}

/** Whether the refactored implementation of nested tree resolution should be used. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,10 @@ protected ComponentTree(Builder builder) {
mContext = ComponentContext.withComponentTree(builder.context, this);
mRoot = wrapRootInErrorBoundary(builder.root);

mIncrementalMountEnabled = builder.incrementalMountEnabled;
// Incremental mount will not work if this ComponentTree is nested in a parent with it turned
// off, so always disable it in that case
mIncrementalMountEnabled =
builder.incrementalMountEnabled && !mContext.isParentIncrementalMountDisabled();
mIsLayoutDiffingEnabled = builder.isLayoutDiffingEnabled;
mLayoutThreadHandler = builder.layoutThreadHandler;
mShouldPreallocatePerMountSpec = builder.shouldPreallocatePerMountSpec;
Expand Down
41 changes: 41 additions & 0 deletions litho-it/src/test/java/com/facebook/litho/ComponentTreeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,47 @@ public void onNewLayoutStateReady(ComponentTree componentTree) {
componentTree.setRootAndSizeSpec(mComponent, mWidthSpec, mHeightSpec);
}

@Test
public void testIncrementalMountDisabledForChildTreeWhenDisabledInParentTree() {
final ComponentTree parentTree =
ComponentTree.create(mContext, TestLayoutComponent.create(mContext).build())
.incrementalMount(false)
.build();
final ComponentContext childContext =
new ComponentContext(
mContext.getAndroidContext(),
null,
mContext.getLogger(),
null,
null,
null,
mContext.getYogaNodeFactory(),
ComponentContext.isIncrementalMountDisabled(parentTree.getContext()));
final ComponentTree childTree =
ComponentTree.create(childContext, TestLayoutComponent.create(childContext).build())
.build();

assertThat(childTree.isIncrementalMountEnabled()).isFalse();

final ComponentTree parentTree2 =
ComponentTree.create(mContext, TestLayoutComponent.create(mContext).build()).build();
final ComponentContext childContext2 =
new ComponentContext(
mContext.getAndroidContext(),
null,
mContext.getLogger(),
null,
null,
null,
mContext.getYogaNodeFactory(),
ComponentContext.isIncrementalMountDisabled(parentTree2.getContext()));
final ComponentTree childTree2 =
ComponentTree.create(childContext2, TestLayoutComponent.create(childContext).build())
.build();

assertThat(childTree2.isIncrementalMountEnabled()).isTrue();
}

class MyTestComponent extends Component {

CountDownLatch unlockWaitingOnCreateLayout;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,8 @@ public RecyclerBinder build(ComponentContext c) {
null,
null,
c.getTreePropsCopy(),
c.getYogaNodeFactory());
c.getYogaNodeFactory(),
ComponentContext.isIncrementalMountDisabled(c));

if (layoutInfo == null) {
layoutInfo = new LinearLayoutInfo(c.getAndroidContext(), VERTICAL, false);
Expand Down

0 comments on commit c88a660

Please sign in to comment.