Correct LayoutMetrics comparison and make debuggable

Summary: Allow conversion of LayoutMetrics to DebuggableString. We also skipped a field in comparison. It probably isn't impactful in terms of production issues, but still wasn't correct. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D27709451 fbshipit-source-id: 987fc2de0a4562a295d6cbeffdd922cbf056b811
sasmit · Apr 14, 2021 · 88c3090 · 88c3090
1 parent 60a18c1
commit 88c3090
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 3 deletions.
diff --git a/ReactCommon/react/renderer/core/LayoutMetrics.cpp b/ReactCommon/react/renderer/core/LayoutMetrics.cpp
@@ -0,0 +1,65 @@
+/*
+ * 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 <react/renderer/debug/DebugStringConvertible.h>
+#include <react/renderer/debug/flags.h>
+
+#include "LayoutMetrics.h"
+
+namespace facebook {
+namespace react {
+
+#if RN_DEBUG_STRING_CONVERTIBLE
+
+std::string getDebugName(LayoutMetrics const &object) {
+  return "LayoutMetrics";
+}
+
+std::vector<DebugStringConvertibleObject> getDebugProps(
+    LayoutMetrics const &object,
+    DebugStringConvertibleOptions options) {
+  return {
+      {"frame",
+       "{x:" + getDebugDescription(object.frame.origin.x, {}) +
+           ",y:" + getDebugDescription(object.frame.origin.y, {}) +
+           ",width:" + getDebugDescription(object.frame.size.width, {}) +
+           ",height:" + getDebugDescription(object.frame.size.height, {}) +
+           "}"},
+      {"contentInsets",
+       "{top:" + getDebugDescription(object.contentInsets.top, {}) +
+           ",right:" + getDebugDescription(object.contentInsets.right, {}) +
+           ",bottom:" + getDebugDescription(object.contentInsets.bottom, {}) +
+           ",left:" + getDebugDescription(object.contentInsets.left, {}) + "}"},
+      {"borderWidth",
+       "{top:" + getDebugDescription(object.borderWidth.top, {}) +
+           ",right:" + getDebugDescription(object.borderWidth.right, {}) +
+           ",bottom:" + getDebugDescription(object.borderWidth.bottom, {}) +
+           ",left:" + getDebugDescription(object.borderWidth.left, {}) + "}"},
+      {"overflowInset",
+       "{top:" + getDebugDescription(object.overflowInset.top, {}) +
+           ",right:" + getDebugDescription(object.overflowInset.right, {}) +
+           ",bottom:" + getDebugDescription(object.overflowInset.bottom, {}) +
+           ",left:" + getDebugDescription(object.overflowInset.left, {}) + "}"},
+      {"displayType",
+       object.displayType == DisplayType::None
+           ? "None"
+           : (object.displayType == DisplayType::Flex ? "Flex" : "Inline")},
+      {"layoutDirection",
+       object.layoutDirection == LayoutDirection::Undefined
+           ? "Undefined"
+           : (object.layoutDirection == LayoutDirection::LeftToRight
+                  ? "LeftToRight"
+                  : "RightToLeft")},
+      {"pointScaleFactor",
+       getDebugDescription(object.pointScaleFactor, options)},
+  };
+}
+
+#endif
+
+} // namespace react
+} // namespace facebook
diff --git a/ReactCommon/react/renderer/core/LayoutMetrics.h b/ReactCommon/react/renderer/core/LayoutMetrics.h
@@ -9,6 +9,8 @@
 
 #include <folly/Hash.h>
 #include <react/renderer/core/LayoutPrimitives.h>
+#include <react/renderer/debug/DebugStringConvertible.h>
+#include <react/renderer/debug/flags.h>
 #include <react/renderer/graphics/Geometry.h>
 
 namespace facebook {
@@ -41,14 +43,16 @@ struct LayoutMetrics {
                this->borderWidth,
                this->displayType,
                this->layoutDirection,
-               this->pointScaleFactor) ==
+               this->pointScaleFactor,
+               this->overflowInset) ==
         std::tie(
                rhs.frame,
                rhs.contentInsets,
                rhs.borderWidth,
                rhs.displayType,
                rhs.layoutDirection,
-               rhs.pointScaleFactor);
+               rhs.pointScaleFactor,
+               rhs.overflowInset);
   }
 
   bool operator!=(const LayoutMetrics &rhs) const {
@@ -64,6 +68,15 @@ struct LayoutMetrics {
 static LayoutMetrics const EmptyLayoutMetrics = {
     /* .frame = */ {{0, 0}, {-1.0, -1.0}}};
 
+#ifdef RN_DEBUG_STRING_CONVERTIBLE
+
+std::string getDebugName(LayoutMetrics const &object);
+std::vector<DebugStringConvertibleObject> getDebugProps(
+    LayoutMetrics const &object,
+    DebugStringConvertibleOptions options);
+
+#endif
+
 } // namespace react
 } // namespace facebook
 

diff --git a/ReactCommon/react/renderer/mounting/ShadowView.cpp b/ReactCommon/react/renderer/mounting/ShadowView.cpp
@@ -7,6 +7,7 @@
 
 #include "ShadowView.h"
 
+#include <react/renderer/core/LayoutMetrics.h>
 #include <react/renderer/core/LayoutableShadowNode.h>
 
 namespace facebook {
@@ -52,7 +53,7 @@ bool ShadowView::operator!=(const ShadowView &rhs) const {
   return !(*this == rhs);
 }
 
-#if RN_DEBUG_STRING_CONVERTIBLE
+#ifdef RN_DEBUG_STRING_CONVERTIBLE
 
 std::string getDebugName(ShadowView const &object) {
   return object.componentHandle == 0 ? "Invalid" : object.componentName;
@@ -64,6 +65,7 @@ std::vector<DebugStringConvertibleObject> getDebugProps(
   return {
       {"surfaceId", getDebugDescription(object.surfaceId, options)},
       {"tag", getDebugDescription(object.tag, options)},
+      {"componentName", object.componentName},
       {"props", getDebugDescription(object.props, options)},
       {"eventEmitter", getDebugDescription(object.eventEmitter, options)},
       {"layoutMetrics", getDebugDescription(object.layoutMetrics, options)},

diff --git a/ReactCommon/react/renderer/mounting/ShadowView.h b/ReactCommon/react/renderer/mounting/ShadowView.h
@@ -14,6 +14,7 @@
 #include <react/renderer/core/Props.h>
 #include <react/renderer/core/ReactPrimitives.h>
 #include <react/renderer/core/ShadowNode.h>
+#include <react/renderer/debug/flags.h>
 
 namespace facebook {
 namespace react {

diff --git a/ReactCommon/react/renderer/mounting/StubViewTree.cpp b/ReactCommon/react/renderer/mounting/StubViewTree.cpp
@@ -70,6 +70,22 @@ void StubViewTree::mutate(ShadowViewMutationList const &mutations) {
         /* Disable this assert until T76057501 is resolved.
         react_native_assert(registry.find(tag) != registry.end());
         auto stubView = registry[tag];
+        if ((ShadowView)(*stubView) != mutation.oldChildShadowView) {
+          LOG(ERROR)
+              << "StubView: ASSERT FAILURE: DELETE mutation assertion failure:
+oldChildShadowView doesn't match stubView: ["
+              << mutation.oldChildShadowView.tag << "] stub hash: ##"
+              << std::hash<ShadowView>{}((ShadowView)*stubView)
+              << " old mutation hash: ##"
+              << std::hash<ShadowView>{}(mutation.oldChildShadowView);
+#ifdef RN_DEBUG_STRING_CONVERTIBLE
+          LOG(ERROR) << "StubView: "
+                     << getDebugPropsDescription((ShadowView)*stubView, {});
+          LOG(ERROR) << "OldChildShadowView: "
+                     << getDebugPropsDescription(
+                            mutation.oldChildShadowView, {});
+#endif
+        }
         react_native_assert(
             (ShadowView)(*stubView) == mutation.oldChildShadowView);
         */