Fabric: Fixing arbitrary-node-replacing (aka `RootShadowNode::clone`) algorithm

Summary:
This diff changes how arbitrary-node-replacing (aka `RootShadowNode::clone`) algorithm works.
Original implementation worked this way: We specified a node that needs to be replaced and a new replacement node. Then the algorithm finds a node with *the same family* as given the to-be-replaced node and replaced that with a given replacement.
The problem with this approach is that we are replacing a node that we have very little info about (we only know that it shares the same family as specified one). At the same time, we build the replacement based on the exact node that we have. Then imagine the case in which the "target" node can progress/change between a moment where we get a reference to it and a moment where we are trying to clone the tree. In this case, we will replace a "progressed" node with a modified version of the obsolete node.
Practically speaking, it was possible that during a state update we were replacing a node that just got new children with a bit older version of that with old children but with a new state.

How to deal with it? This diff introduces a new interface for this method that allows separating the target node and the actual act of cloning the corresponding node. Instead of specifying actual replacement, we now specify a function that performs the cloning/transformation on-demand on the very exact node that was in the tree at the moment of cloning.

This change does not change/affect any ownership-related relationships between trees and/or nodes.

Ideally, probably, the interface should accept ShadowNodeFamily instance instead of a ShadowNode instance to make the behavior very clear but that requires a bunch of low-level changes that it out of the scope of this fix.

```

                                                                   The old approach.

                     ┌─────────────────┐                          ┌─────────────────┐                          ┌─────────────────┐
                     │                 │                          │                 │                          │                 │
                     │      A(r0)      │                          │      A(r1)      │                          │      A(r2)      │
                     │                 │                          │                 │                          │                 │
                     └─────────────────┘                          └─────────────────┘                          └─────────────────┘
                              │                                            │                                            │
                              │          Let's update the                  │   Meanwhile the node B                     │
                              ▼           state of this                    ▼    gets new children.                      ▼
                     ┌─────────────────┐   node to s1.            ┌─────────────────┐                          ┌─────────────────┐
                     │                 │                          │                 │                          │                 │   Created
                     │    B(r0, s0)    │         ───────▶         │    B(r1, s0)    │         ───────▶         │    B(r2, s1)    │     from
                     │                 │                          │                 │                          │                 │    B(r0).
                     └─────────────────┘                          └─────────────────┘                          └─────────────────┘
                              │                                            │                                            │
                       ┌──────┴──────┐                       ┌─────────────┼─────────────┐                       ┌──────┴──────┐
                       │             │                       │             │             │                       │             │    What just
                       ▼             ▼                       ▼             ▼             ▼                       ▼             ▼     happe..?
                 ┌───────────┐ ┌───────────┐           ┌───────────┐ ┌───────────┐ ┌───────────┐           ┌───────────┐ ┌───────────┐
                 │           │ │           │           │           │ │           │ │           │           │           │ │           │
                 │   C(r0)   │ │   D(r0)   │           │   C(r1)   │ │   D(r1)   │ │   X(r0)   │           │   C(r0)   │ │   D(r0)   │
                 │           │ │           │           │           │ │           │ │           │           │           │ │           │
                 └───────────┘ └───────────┘           └───────────┘ └───────────┘ └───────────┘           └───────────┘ └───────────┘

                                                                   The new approach.

                     ┌─────────────────┐                          ┌─────────────────┐                           ┌─────────────────┐
                     │                 │                          │                 │                           │                 │
                     │      A(r0)      │                          │      A(r1)      │                           │      A(r2)      │
                     │                 │                          │                 │                           │                 │
                     └─────────────────┘                          └─────────────────┘                           └─────────────────┘
                              │                                            │                                             │
                              │         Let's update the                   │                                             │
                              ▼          state of this                     ▼                                             ▼
                     ┌─────────────────┐  node to s1.             ┌─────────────────┐                           ┌─────────────────┐
                     │                 │                          │                 │                           │                 │   Created
                     │    B(r0, s0)    │         ───────▶         │    B(r1, s0)    │         ───────▶          │    B(r2, s1)    │     from
                     │                 │                          │                 │                           │                 │    B(r1).
                     └─────────────────┘                          └─────────────────┘                           └─────────────────┘
                              │                                            │                                             │
                       ┌──────┴──────┐                       ┌─────────────┼─────────────┐                 ┌─────────────┼─────────────┐
                       │             │                       │             │             │                 │             │             │
                       ▼             ▼                       ▼             ▼             ▼                 ▼             ▼             ▼
                 ┌───────────┐ ┌───────────┐           ┌───────────┐ ┌───────────┐ ┌───────────┐     ┌───────────┐ ┌───────────┐ ┌───────────┐
                 │           │ │           │           │           │ │           │ │           │     │           │ │           │ │           │
                 │   C(r0)   │ │   D(r0)   │           │   C(r1)   │ │   D(r1)   │ │   X(r0)   │     │   C(r1)   │ │   D(r1)   │ │   X(r0)   │
                 │           │ │           │           │           │ │           │ │           │     │           │ │           │ │           │
                 └───────────┘ └───────────┘           └───────────┘ └───────────┘ └───────────┘     └───────────┘ └───────────┘ └───────────┘

```

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross

Differential Revision: D18229704

fbshipit-source-id: face6d0e5c240224ce49e93e783cff3172b60529
This commit is contained in:
Valentin Shergin 2019-10-31 09:36:42 -07:00 коммит произвёл Facebook Github Bot
Родитель 6ffc93d657
Коммит b5080f7d77
3 изменённых файлов: 47 добавлений и 29 удалений

Просмотреть файл

@ -49,14 +49,21 @@ RootShadowNode::Unshared RootShadowNode::clone(
}
RootShadowNode::Unshared RootShadowNode::clone(
ShadowNode const &oldShadowNode,
ShadowNode::Shared const &newShadowNode) const {
auto ancestors = oldShadowNode.getAncestors(*this);
ShadowNode const &shadowNode,
std::function<ShadowNode::Unshared(ShadowNode const &oldShadowNode)>
callback) const {
auto ancestors = shadowNode.getAncestors(*this);
if (ancestors.size() == 0) {
return RootShadowNode::Unshared{nullptr};
}
auto &parent = ancestors.back();
auto &oldShadowNode = parent.first.get().getChildren().at(parent.second);
assert(ShadowNode::sameFamily(shadowNode, *oldShadowNode));
auto newShadowNode = callback(*oldShadowNode);
auto childNode = newShadowNode;
for (auto it = ancestors.rbegin(); it != ancestors.rend(); ++it) {

Просмотреть файл

@ -47,14 +47,17 @@ class RootShadowNode final
LayoutContext const &layoutContext) const;
/*
* Clones the node replacing a given old shadow node with a new one in the
* tree by cloning all nodes on the path to the root node and then complete
* the tree. Returns `nullptr` if the operation cannot be finished
* successfully.
* Clones the node (and partially the tree starting from the node) by
* replacing a `oldShadowNode` (which corresponds to a given `shadowNode`)
* with a node that `callback` returns. `oldShadowNode` might not be the same
* as `shadowNode` but they must share the same family.
*
* Returns `nullptr` if the operation cannot be performed successfully.
*/
RootShadowNode::Unshared clone(
ShadowNode const &oldShadowNode,
ShadowNode::Shared const &newShadowNode) const;
ShadowNode const &shadowNode,
std::function<ShadowNode::Unshared(ShadowNode const &oldShadowNode)>
callback) const;
private:
using YogaLayoutableShadowNode::layout;

Просмотреть файл

@ -131,17 +131,20 @@ void UIManager::setNativeProps(
auto &componentDescriptor = shadowNode.getComponentDescriptor();
auto props = componentDescriptor.cloneProps(shadowNode.getProps(), rawProps);
auto newShadowNode = shadowNode.clone({
/* .tag = */ ShadowNodeFragment::tagPlaceholder(),
/* .surfaceId = */ ShadowNodeFragment::surfaceIdPlaceholder(),
/* .props = */ props,
});
shadowTreeRegistry_.visit(
shadowNode.getSurfaceId(), [&](ShadowTree const &shadowTree) {
shadowTree.tryCommit(
[&](RootShadowNode::Shared const &oldRootShadowNode) {
return oldRootShadowNode->clone(shadowNode, newShadowNode);
return oldRootShadowNode->clone(
shadowNode, [&](ShadowNode const &oldShadowNode) {
return oldShadowNode.clone({
/* .tag = */ ShadowNodeFragment::tagPlaceholder(),
/* .surfaceId = */
ShadowNodeFragment::surfaceIdPlaceholder(),
/* .props = */ props,
});
});
});
});
}
@ -181,22 +184,27 @@ void UIManager::updateState(
auto &componentDescriptor = shadowNode.getComponentDescriptor();
auto state =
componentDescriptor.createState(shadowNode.getState(), rawStateData);
auto newShadowNode = shadowNode.clone({
/* .tag = */ ShadowNodeFragment::tagPlaceholder(),
/* .surfaceId = */ ShadowNodeFragment::surfaceIdPlaceholder(),
/* .props = */ ShadowNodeFragment::propsPlaceholder(),
/* .eventEmitter = */ ShadowNodeFragment::eventEmitterPlaceholder(),
/* .children = */ ShadowNodeFragment::childrenPlaceholder(),
/* .localData = */ ShadowNodeFragment::localDataPlaceholder(),
/* .state = */ state,
});
shadowTreeRegistry_.visit(
shadowNode.getSurfaceId(), [&](const ShadowTree &shadowTree) {
shadowTree.tryCommit(
[&](RootShadowNode::Shared const &oldRootShadowNode) {
return oldRootShadowNode->clone(shadowNode, newShadowNode);
});
shadowNode.getSurfaceId(), [&](ShadowTree const &shadowTree) {
shadowTree.tryCommit([&](RootShadowNode::Shared const
&oldRootShadowNode) {
return oldRootShadowNode->clone(
shadowNode, [&](ShadowNode const &oldShadowNode) {
return oldShadowNode.clone({
/* .tag = */ ShadowNodeFragment::tagPlaceholder(),
/* .surfaceId = */
ShadowNodeFragment::surfaceIdPlaceholder(),
/* .props = */ ShadowNodeFragment::propsPlaceholder(),
/* .eventEmitter = */
ShadowNodeFragment::eventEmitterPlaceholder(),
/* .children = */ ShadowNodeFragment::childrenPlaceholder(),
/* .localData = */
ShadowNodeFragment::localDataPlaceholder(),
/* .state = */ state,
});
});
});
});
}