From 6729a3e0bfc01119c8513dfcbb1f5fbe5fe81263 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Fri, 28 Aug 2020 10:20:21 -0700 Subject: [PATCH] Fabric: Reviving of RN_SHADOW_TREE_INTROSPECTION in DEBUG mode Summary: Shadow tree introspection was disabled for a while, now we need it back working. This diff also restructures the logic of `MountingCoordinator::pullTransaction()` splitting it into two sections, first one for the base case and the second for the overriding case. Changelog: [Internal] Fabric-specific internal change. Reviewed By: mdvacca Differential Revision: D23374948 fbshipit-source-id: 0b5f1c598975bceb3dcb6a0eaee67ff58ef9dda1 --- .../renderer/mounting/MountingCoordinator.cpp | 150 +++++++++--------- .../renderer/mounting/MountingCoordinator.h | 7 +- 2 files changed, 79 insertions(+), 78 deletions(-) diff --git a/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp b/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp index 4a356174c54391..7ad2b08e5e5700 100644 --- a/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp +++ b/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp @@ -80,98 +80,98 @@ void MountingCoordinator::resetLatestRevision() const { lastRevision_.reset(); } -#ifdef RN_SHADOW_TREE_INTROSPECTION -void MountingCoordinator::validateTransactionAgainstStubViewTree( - ShadowViewMutationList const &mutations, - bool assertEquality) const { - std::string line; - - std::stringstream ssMutations(getDebugDescription(mutations, {})); - while (std::getline(ssMutations, line, '\n')) { - LOG(ERROR) << "Mutations:" << line; - } - - stubViewTree_.mutate(mutations); - auto stubViewTree = - stubViewTreeFromShadowNode(lastRevision_->getRootShadowNode()); +better::optional MountingCoordinator::pullTransaction() + const { + std::lock_guard lock(mutex_); - std::stringstream ssOldTree( - baseRevision_.getRootShadowNode().getDebugDescription()); - while (std::getline(ssOldTree, line, '\n')) { - LOG(ERROR) << "Old tree:" << line; - } + auto transaction = better::optional{}; - std::stringstream ssNewTree( - lastRevision_->getRootShadowNode().getDebugDescription()); - while (std::getline(ssNewTree, line, '\n')) { - LOG(ERROR) << "New tree:" << line; - } + // Base case + if (lastRevision_.has_value()) { + number_++; - if (assertEquality) { - assert(stubViewTree_ == stubViewTree); - } -} -#endif + auto telemetry = lastRevision_->getTelemetry(); -better::optional MountingCoordinator::pullTransaction() - const { - std::lock_guard lock(mutex_); + telemetry.willDiff(); - auto mountingOverrideDelegate = mountingOverrideDelegate_.lock(); + auto mutations = calculateShadowViewMutations( + baseRevision_.getRootShadowNode(), lastRevision_->getRootShadowNode()); - bool shouldOverridePullTransaction = mountingOverrideDelegate && - mountingOverrideDelegate->shouldOverridePullTransaction(); + telemetry.didDiff(); - if (!shouldOverridePullTransaction && !lastRevision_.has_value()) { - return {}; - } + baseRevision_ = std::move(*lastRevision_); + lastRevision_.reset(); - number_++; - - ShadowViewMutation::List diffMutations{}; - auto telemetry = - (lastRevision_.hasValue() ? lastRevision_->getTelemetry() - : TransactionTelemetry{}); - if (!lastRevision_.hasValue()) { - telemetry.willLayout(); - telemetry.didLayout(); - telemetry.willCommit(); - telemetry.didCommit(); - } - telemetry.willDiff(); - if (lastRevision_.hasValue()) { - diffMutations = calculateShadowViewMutations( - baseRevision_.getRootShadowNode(), - lastRevision_->getRootShadowNode(), - enableReparentingDetection_); + transaction = MountingTransaction{ + surfaceId_, number_, std::move(mutations), telemetry}; } - telemetry.didDiff(); - better::optional transaction{}; + // Override case + auto mountingOverrideDelegate = mountingOverrideDelegate_.lock(); + auto shouldOverridePullTransaction = mountingOverrideDelegate && + mountingOverrideDelegate->shouldOverridePullTransaction(); - // The override delegate can provide custom mounting instructions, - // even if there's no `lastRevision_`. Consider cases of animation frames - // in between React tree updates. if (shouldOverridePullTransaction) { + auto mutations = ShadowViewMutation::List{}; + auto telemetry = TransactionTelemetry{}; + + if (transaction.has_value()) { + mutations = transaction->getMutations(); + telemetry = transaction->getTelemetry(); + } else { + telemetry.willLayout(); + telemetry.didLayout(); + telemetry.willCommit(); + telemetry.didCommit(); + } + transaction = mountingOverrideDelegate->pullTransaction( - surfaceId_, number_, telemetry, std::move(diffMutations)); - } else if (lastRevision_.hasValue()) { - transaction = MountingTransaction{ - surfaceId_, number_, std::move(diffMutations), telemetry}; + surfaceId_, number_, telemetry, std::move(mutations)); } - if (lastRevision_.hasValue()) { #ifdef RN_SHADOW_TREE_INTROSPECTION - // Only validate non-animated transactions - it's garbage to validate - // animated transactions, since the stub view tree likely won't match - // the committed tree during an animation. - this->validateTransactionAgainstStubViewTree( - transaction->getMutations(), !shouldOverridePullTransaction); -#endif - - baseRevision_ = std::move(*lastRevision_); - lastRevision_.reset(); + if (transaction.has_value()) { + // We have something to validate. + auto mutations = transaction->getMutations(); + + // No matter what the source of the transaction is, it must be able to + // mutate the existing stub view tree. + stubViewTree_.mutate(mutations); + + // If the transaction was overridden, we don't have a model of the shadow + // tree therefore we cannot validate the validity of the mutation + // instructions. + if (!shouldOverridePullTransaction) { + auto line = std::string{}; + + auto stubViewTree = + stubViewTreeFromShadowNode(baseRevision_.getRootShadowNode()); + + if (stubViewTree_ != stubViewTree) { + std::stringstream ssOldTree( + baseRevision_.getRootShadowNode().getDebugDescription()); + while (std::getline(ssOldTree, line, '\n')) { + LOG(ERROR) << "Old tree:" << line; + } + + std::stringstream ssMutations(getDebugDescription(mutations, {})); + while (std::getline(ssMutations, line, '\n')) { + LOG(ERROR) << "Mutations:" << line; + } + + std::stringstream ssNewTree( + lastRevision_->getRootShadowNode().getDebugDescription()); + while (std::getline(ssNewTree, line, '\n')) { + LOG(ERROR) << "New tree:" << line; + } + } + + assert( + (stubViewTree_ == stubViewTree) && + "Incorrect set of mutations detected."); + } } +#endif return transaction; } diff --git a/ReactCommon/react/renderer/mounting/MountingCoordinator.h b/ReactCommon/react/renderer/mounting/MountingCoordinator.h index 60884ee88bd304..54bb0a3a63d4a7 100644 --- a/ReactCommon/react/renderer/mounting/MountingCoordinator.h +++ b/ReactCommon/react/renderer/mounting/MountingCoordinator.h @@ -17,6 +17,10 @@ #include #include "ShadowTreeRevision.h" +#ifndef NDEBUG +#define RN_SHADOW_TREE_INTROSPECTION 1 +#endif + #ifdef RN_SHADOW_TREE_INTROSPECTION #include #endif @@ -113,9 +117,6 @@ class MountingCoordinator final { bool enableReparentingDetection_{false}; // temporary #ifdef RN_SHADOW_TREE_INTROSPECTION - void validateTransactionAgainstStubViewTree( - ShadowViewMutationList const &mutations, - bool assertEquality) const; mutable StubViewTree stubViewTree_; // Protected by `mutex_`. #endif };