Code quality: Refactor ShadowViewMutation::UpdateMutation to remove index, parentShadowView parameters

Summary:
The `index` parameter for UpdateMutation is optional, and is normally just -1. It's not useful, so remove it. `parentShadowView` is also not relevant and is not used; in some existing use-cases the actual parent view of the updated view is available, and in some contexts the parent view is not set.

The function now will always set the index to -1 for UpdateMutations, and `{}` for ParentShadowView.

This should have no impact on iOS or Android, as this parameter is not used. It could theoretically have an impact on lifetimes of objects retained (now not retained) by not passing parentShadowView into the mutation. For example, any shared props or state associated with the parent will not be retained in the Update mutation now.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D25342943

fbshipit-source-id: 0ddbef76a6e2eefc2629c9729f721d8674d7737e
This commit is contained in:
Joshua Gross
2020-12-07 13:37:10 -08:00
committed by Facebook GitHub Bot
parent 0d4985900b
commit e8770d7bb2
5 changed files with 15 additions and 37 deletions
@@ -74,7 +74,7 @@ void LayoutAnimationDriver::animationMutationsForFrame(
// Create the mutation instruction
auto updateMutation = ShadowViewMutation::UpdateMutation(
keyframe.parentView, keyframe.viewPrev, mutatedShadowView, -1);
keyframe.viewPrev, mutatedShadowView);
// All generated Update mutations must have an "old" and "new"
// ShadowView. Checking for nonzero tag doesn't guarantee that the views
@@ -1252,10 +1252,7 @@ LayoutAnimationKeyFrameManager::pullTransaction(
1, keyFrame.viewStart, keyFrame.viewEnd);
auto generatedPenultimateMutation =
ShadowViewMutation::UpdateMutation(
keyFrame.parentView,
keyFrame.viewPrev,
mutatedShadowView,
-1);
keyFrame.viewPrev, mutatedShadowView);
assert(generatedPenultimateMutation.oldChildShadowView.tag != 0);
assert(generatedPenultimateMutation.newChildShadowView.tag != 0);
PrintMutationInstruction(
@@ -1264,7 +1261,7 @@ LayoutAnimationKeyFrameManager::pullTransaction(
finalConflictingMutations.push_back(generatedPenultimateMutation);
auto generatedMutation = ShadowViewMutation::UpdateMutation(
keyFrame.parentView, mutatedShadowView, keyFrame.viewEnd, -1);
mutatedShadowView, keyFrame.viewEnd);
assert(generatedMutation.oldChildShadowView.tag != 0);
assert(generatedMutation.newChildShadowView.tag != 0);
PrintMutationInstruction(
@@ -1437,10 +1434,7 @@ LayoutAnimationKeyFrameManager::pullTransaction(
1, keyFrame.viewStart, keyFrame.viewEnd);
auto generatedPenultimateMutation =
ShadowViewMutation::UpdateMutation(
keyFrame.parentView,
keyFrame.viewPrev,
mutatedShadowView,
-1);
keyFrame.viewPrev, mutatedShadowView);
assert(generatedPenultimateMutation.oldChildShadowView.tag != 0);
assert(generatedPenultimateMutation.newChildShadowView.tag != 0);
PrintMutationInstruction(
@@ -1450,7 +1444,7 @@ LayoutAnimationKeyFrameManager::pullTransaction(
generatedPenultimateMutation);
auto generatedMutation = ShadowViewMutation::UpdateMutation(
keyFrame.parentView, mutatedShadowView, keyFrame.viewEnd, -1);
mutatedShadowView, keyFrame.viewEnd);
assert(generatedMutation.oldChildShadowView.tag != 0);
assert(generatedMutation.newChildShadowView.tag != 0);
PrintMutationInstruction(
@@ -486,10 +486,7 @@ static void calculateShadowViewMutationsFlattener(
newTreeNodePair.isConcreteView && oldTreeNodePair.isConcreteView) {
mutationInstructionContainer.updateMutations.push_back(
ShadowViewMutation::UpdateMutation(
parentShadowView,
oldTreeNodePair.shadowView,
newTreeNodePair.shadowView,
newTreeNodePair.mountIndex));
oldTreeNodePair.shadowView, newTreeNodePair.shadowView));
}
// Update children if appropriate.
@@ -850,10 +847,7 @@ static void calculateShadowViewMutationsV2(
if (newChildPair.isConcreteView &&
oldChildPair.shadowView != newChildPair.shadowView) {
updateMutations.push_back(ShadowViewMutation::UpdateMutation(
parentShadowView,
oldChildPair.shadowView,
newChildPair.shadowView,
newChildPair.mountIndex));
oldChildPair.shadowView, newChildPair.shadowView));
}
// Recursively update tree if ShadowNode pointers are not equal
@@ -995,10 +989,7 @@ static void calculateShadowViewMutationsV2(
// concrete view. The case where they're different is handled above.
if (oldChildPair.shadowView != newChildPair.shadowView) {
updateMutations.push_back(ShadowViewMutation::UpdateMutation(
parentShadowView,
oldChildPair.shadowView,
newChildPair.shadowView,
newChildPair.mountIndex));
oldChildPair.shadowView, newChildPair.shadowView));
}
// Remove from newRemainingPairs
@@ -1251,10 +1242,7 @@ static void calculateShadowViewMutationsV2(
if (oldChildPair.shadowView != newChildPair.shadowView) {
updateMutations.push_back(ShadowViewMutation::UpdateMutation(
parentShadowView,
oldChildPair.shadowView,
newChildPair.shadowView,
newChildPair.mountIndex));
oldChildPair.shadowView, newChildPair.shadowView));
}
}
if (!oldChildPair.flattened &&
@@ -1516,7 +1504,7 @@ ShadowViewMutation::List calculateShadowViewMutations(
if (oldRootShadowView != newRootShadowView) {
mutations.push_back(ShadowViewMutation::UpdateMutation(
ShadowView(), oldRootShadowView, newRootShadowView, -1));
oldRootShadowView, newRootShadowView));
}
calculateShadowViewMutationsV2(
@@ -57,16 +57,14 @@ ShadowViewMutation ShadowViewMutation::RemoveMutation(
}
ShadowViewMutation ShadowViewMutation::UpdateMutation(
ShadowView parentShadowView,
ShadowView oldChildShadowView,
ShadowView newChildShadowView,
int index) {
ShadowView newChildShadowView) {
return {
/* .type = */ Update,
/* .parentShadowView = */ parentShadowView,
/* .parentShadowView = */ {},
/* .oldChildShadowView = */ oldChildShadowView,
/* .newChildShadowView = */ newChildShadowView,
/* .index = */ index,
/* .index = */ -1,
};
}
@@ -55,10 +55,8 @@ struct ShadowViewMutation final {
* Creates and returns an `Update` mutation.
*/
static ShadowViewMutation UpdateMutation(
ShadowView parentShadowView,
ShadowView oldChildShadowView,
ShadowView newChildShadowView,
int index);
ShadowView newChildShadowView);
#pragma mark - Type
@@ -70,7 +68,7 @@ struct ShadowViewMutation final {
ShadowView parentShadowView = {};
ShadowView oldChildShadowView = {};
ShadowView newChildShadowView = {};
int index = {};
int index = -1;
};
using ShadowViewMutationList = std::vector<ShadowViewMutation>;