From fc1f0df4659d331e74aa7c523e1f4e344eb386cd Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 28 Jan 2021 11:18:41 -0800 Subject: [PATCH] Introducing: Props forwarding whe cloning shadow node Summary: Changelog: [internal] If ShadowNode has not been mounted, forward rawProps from `sourceShadowNode` to newly cloned shadow node. This is Android specific change, on iOS the logic should remain unchanged. Reviewed By: JoshuaGross Differential Revision: D26049264 fbshipit-source-id: 7c201bc2d4e99eec024065714d2172c5c817153c --- ReactCommon/react/renderer/core/Constants.cpp | 24 ++++++++ ReactCommon/react/renderer/core/Constants.h | 23 ++++++++ .../react/renderer/core/ShadowNode.cpp | 35 +++++++++++- ReactCommon/react/renderer/core/ShadowNode.h | 8 ++- .../core/tests/FollyDynamicMergePatchTest.cpp | 56 +++++++++++++++++++ .../react/renderer/scheduler/Scheduler.cpp | 3 + packages/rn-tester/Podfile.lock | 2 +- 7 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 ReactCommon/react/renderer/core/Constants.cpp create mode 100644 ReactCommon/react/renderer/core/Constants.h create mode 100644 ReactCommon/react/renderer/core/tests/FollyDynamicMergePatchTest.cpp diff --git a/ReactCommon/react/renderer/core/Constants.cpp b/ReactCommon/react/renderer/core/Constants.cpp new file mode 100644 index 00000000000..150473be288 --- /dev/null +++ b/ReactCommon/react/renderer/core/Constants.cpp @@ -0,0 +1,24 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "Constants.h" + +namespace facebook { +namespace react { + +static bool isPropsForwardingEnabled = false; + +void Constants::setPropsForwardingEnabled(bool propsForwardingEnabled) { + isPropsForwardingEnabled = propsForwardingEnabled; +} + +bool Constants::getPropsForwardingEnabled() { + return isPropsForwardingEnabled; +} + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/react/renderer/core/Constants.h b/ReactCommon/react/renderer/core/Constants.h new file mode 100644 index 00000000000..79595290241 --- /dev/null +++ b/ReactCommon/react/renderer/core/Constants.h @@ -0,0 +1,23 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +namespace facebook { +namespace react { + +struct Constants { + /* + Flag controling props forwarding when shadow node is cloned on Android. + Has no effect on iOS. + */ + static void setPropsForwardingEnabled(bool propsForwardingEnabled); + static bool getPropsForwardingEnabled(); +}; + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/react/renderer/core/ShadowNode.cpp b/ReactCommon/react/renderer/core/ShadowNode.cpp index a5839fc820d..3fb034f3b22 100644 --- a/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -6,6 +6,7 @@ */ #include "ShadowNode.h" +#include "Constants.h" #include "ShadowNodeFragment.h" #include @@ -24,6 +25,33 @@ SharedShadowNodeSharedList ShadowNode::emptySharedShadowNodeSharedList() { return emptySharedShadowNodeSharedList; } +/* + * On iOS, this method returns `props` if provided, `sourceShadowNode`'s props + * otherwise. On Android, we forward props in case `sourceShadowNode` hasn't + * been mounted. `Props::rawProps` are merged from `props` to a copy of + * `sourceShadowNode.props_` and returned. This is necessary to enable + * Background Executor and should be removed once reimplementation of JNI layer + * is finished. + */ +SharedProps ShadowNode::propsForClonedShadowNode( + ShadowNode const &sourceShadowNode, + Props::Shared const &props) { +#ifdef ANDROID + if (Constants::getPropsForwardingEnabled()) { + bool hasBeenMounted = sourceShadowNode.hasBeenMounted_; + bool sourceNodeHasRawProps = !sourceShadowNode.getProps()->rawProps.empty(); + if (!hasBeenMounted && sourceNodeHasRawProps && props) { + auto copiedProps = sourceShadowNode.getProps()->rawProps; + copiedProps.merge_patch(props->rawProps); + auto &castedProps = const_cast(*props); + castedProps.rawProps = copiedProps; + return props; + } + } +#endif + return props ? props : sourceShadowNode.getProps(); +} + bool ShadowNode::sameFamily(const ShadowNode &first, const ShadowNode &second) { return first.family_ == second.family_; } @@ -60,13 +88,13 @@ ShadowNode::ShadowNode( } ShadowNode::ShadowNode( - const ShadowNode &sourceShadowNode, - const ShadowNodeFragment &fragment) + ShadowNode const &sourceShadowNode, + ShadowNodeFragment const &fragment) : #if RN_DEBUG_STRING_CONVERTIBLE revision_(sourceShadowNode.revision_ + 1), #endif - props_(fragment.props ? fragment.props : sourceShadowNode.props_), + props_(propsForClonedShadowNode(sourceShadowNode, fragment.props)), children_( fragment.children ? fragment.children : sourceShadowNode.children_), state_( @@ -214,6 +242,7 @@ void ShadowNode::cloneChildrenIfShared() { void ShadowNode::setMounted(bool mounted) const { if (mounted) { family_->setMostRecentState(getState()); + hasBeenMounted_ = mounted; } family_->eventEmitter_->setEnabled(mounted); diff --git a/ReactCommon/react/renderer/core/ShadowNode.h b/ReactCommon/react/renderer/core/ShadowNode.h index 30f5774b7d5..7c385ab7f2b 100644 --- a/ReactCommon/react/renderer/core/ShadowNode.h +++ b/ReactCommon/react/renderer/core/ShadowNode.h @@ -176,8 +176,6 @@ class ShadowNode : public Sealable, public DebugStringConvertible { */ void setMounted(bool mounted) const; - int getStateRevision() const; - #pragma mark - DebugStringConvertible #if RN_DEBUG_STRING_CONVERTIBLE @@ -214,6 +212,12 @@ class ShadowNode : public Sealable, public DebugStringConvertible { */ ShadowNodeFamily::Shared family_; + mutable std::atomic hasBeenMounted_{false}; + + static SharedProps propsForClonedShadowNode( + ShadowNode const &sourceShadowNode, + Props::Shared const &props); + protected: /* * Traits associated with the particular `ShadowNode` class and an instance of diff --git a/ReactCommon/react/renderer/core/tests/FollyDynamicMergePatchTest.cpp b/ReactCommon/react/renderer/core/tests/FollyDynamicMergePatchTest.cpp new file mode 100644 index 00000000000..0362d6e0174 --- /dev/null +++ b/ReactCommon/react/renderer/core/tests/FollyDynamicMergePatchTest.cpp @@ -0,0 +1,56 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +#include +#include +#include + +using namespace folly; + +/* + Tests that verify expected behaviour from `folly::dynamic::merge_patch`. + `merge_patch` is used for props forwarding on Android to enable Background + Executor and will be removed once JNI layer is reimplmeneted. + */ +TEST(FollyDynamicMergePatchTest, handleNestedObjects) { + dynamic map1 = dynamic::object; + map1["style"] = dynamic::object("backgroundColor", "red"); + + dynamic map2 = dynamic::object; + map2["style"] = dynamic::object("backgroundColor", "blue")("color", "black"); + map2["height"] = 100; + + map2.merge_patch(map1); + + EXPECT_TRUE(map2["style"].isObject()); + EXPECT_TRUE(map2["style"]["backgroundColor"].isString()); + EXPECT_TRUE(map2["style"]["color"].isString()); + EXPECT_TRUE(map2["height"].isInt()); + + EXPECT_EQ(map2["style"]["backgroundColor"], "red"); + EXPECT_EQ(map2["style"]["color"], "black"); + EXPECT_EQ(map2["height"], 100); +} + +TEST(FollyDynamicMergePatchTest, handleEmptyObject) { + dynamic map1 = dynamic::object; + + dynamic map2 = dynamic::object; + map2["height"] = 100; + + map2.merge_patch(map1); + + EXPECT_TRUE(map2["height"].isInt()); + EXPECT_EQ(map2["height"], 100); + + map1.merge_patch(map2); + + EXPECT_TRUE(map1["height"].isInt()); + EXPECT_EQ(map1["height"], 100); +} diff --git a/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/ReactCommon/react/renderer/scheduler/Scheduler.cpp index e92553efb32..25a73923827 100644 --- a/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -112,6 +113,8 @@ Scheduler::Scheduler( #ifdef ANDROID removeOutstandingSurfacesOnDestruction_ = reactNativeConfig_->getBool( "react_fabric:remove_outstanding_surfaces_on_destruction_android"); + Constants::setPropsForwardingEnabled(reactNativeConfig_->getBool( + "react_fabric:enable_props_forwarding_android")); #else removeOutstandingSurfacesOnDestruction_ = reactNativeConfig_->getBool( "react_fabric:remove_outstanding_surfaces_on_destruction_ios"); diff --git a/packages/rn-tester/Podfile.lock b/packages/rn-tester/Podfile.lock index 7c64af416f7..6b5c99abc7d 100644 --- a/packages/rn-tester/Podfile.lock +++ b/packages/rn-tester/Podfile.lock @@ -798,7 +798,7 @@ SPEC CHECKSUMS: CocoaLibEvent: 2fab71b8bd46dd33ddb959f7928ec5909f838e3f DoubleConversion: cde416483dac037923206447da6e1454df403714 FBLazyVector: 91e874a8823933a268c38765a88cbd5dba1fa024 - FBReactNativeSpec: 9b595c8d6225c3406ac3b61cad0e0dcd9c4ce82c + FBReactNativeSpec: f413828a0c0ca7fb738e67dd90281e454d353a46 Flipper: be611d4b742d8c87fbae2ca5f44603a02539e365 Flipper-DoubleConversion: 38631e41ef4f9b12861c67d17cb5518d06badc41 Flipper-Folly: e4493b013c02d9347d5e0cb4d128680239f6c78a