Repository: facebook/react
Pull request: [Perf Tracks] Prevent crash when accessing `$$typeof`

Alternate to https://github.com/facebook/react/issues/34840

Fixes https://github.com/facebook/react/issues/34840
Closes https://github.com/facebook/react/issues/34840

## Summary

Guards dotting into `$$typeof` unconditionally. The try-catch proposed in https://github.com/facebook/react/issues/34840 feels too heavy.

Original plan was to skip diffing DOM nodes entirely since they're mutable boxes anyway. That felt like too much of a stretch unless we move the check to the host config. However, that wouldn't fix the linked issue. The linked issue is caused by having a cross-origin window in props not the iframe. Perf tracks wouldn't diff `contentWindow` since it's not enumerable.

The proposed implementation is host agnostic and feels like a better fit overall. And it guards against cases where accessing an unknown `$$typeof` property would also throw.

## Test plan

- added tests (see 1st commit for crash)
- [ ] https://codesandbox.io/p/sandbox/green-fire-84qs2x?file=%2Fsrc%2Findex.js no longer crashes: https://codesandbox.io/p/sandbox/69yhcw?file=%2Fsrc%2Findex.js%3A16%2C15 (needs to be downloaded. remaining error is from CodeSandbox)

Changed files:
filename: packages/react-reconciler/src/__tests__/ReactPerformanceTrack-test.js
patch: @@ -523,4 +523,110 @@ describe('ReactPerformanceTracks', () => {
       ],
     ]);
   });
+
+  // @gate __DEV__ && enableComponentPerformanceTrack
+  it('diffs HTML-like objects', async () => {
+    const App = function App({container}) {
+      Scheduler.unstable_advanceTime(10);
+      React.useEffect(() => {}, [container]);
+    };
+
+    class Window {}
+    const createOpaqueOriginWindow = () => {
+      return new Proxy(new Window(), {
+        get(target, prop) {
+          if (prop === Symbol.toStringTag) {
+            return target[Symbol.toStringTag];
+          }
+          // Some properties are allowed if JS itself is accessign those e.g.
+          // Symbol.toStringTag.
+          // Just make sure React isn't accessing arbitrary properties.
+          throw new Error(
+            `Failed to read named property '${String(prop)}' from Window`,
+          );
+        },
+      });
+    };
+
+    class OpaqueOriginHTMLIFrameElement {
+      constructor(textContent) {
+        this.textContent = textContent;
+      }
+      contentWindow = createOpaqueOriginWindow();
+      nodeType = 1;
+      [Symbol.toStringTag] = 'HTMLIFrameElement';
+    }
+
+    Scheduler.unstable_advanceTime(1);
+    await act(() => {
+      ReactNoop.render(
+        <App
+          container={new OpaqueOriginHTMLIFrameElement('foo')}
+          contentWindow={createOpaqueOriginWindow()}
+        />,
+      );
+    });
+
+    expect(performanceMeasureCalls).toEqual([
+      [
+        'Mount',
+        {
+          detail: {
+            devtools: {
+              color: 'warning',
+              properties: null,
+              tooltipText: 'Mount',
+              track: 'Components ⚛',
+            },
+          },
+          end: 11,
+          start: 1,
+        },
+      ],
+    ]);
+    performanceMeasureCalls.length = 0;
+
+    Scheduler.unstable_advanceTime(10);
+
+    await act(() => {
+      ReactNoop.render(
+        <App
+          container={new OpaqueOriginHTMLIFrameElement('bar')}
+          contentWindow={createOpaqueOriginWindow()}
+        />,
+      );
+    });
+
+    expect(performanceMeasureCalls).toEqual([
+      [
+        '​App',
+        {
+          detail: {
+            devtools: {
+              color: 'primary-dark',
+              properties: [
+                ['Changed Props', ''],
+                ['- container', 'HTMLIFrameElement'],
+                ['-   contentWindow', 'Window'],
+                ['-   nodeType', '1'],
+                ['-   textContent', '"foo"'],
+                ['+ container', 'HTMLIFrameElement'],
+                ['+   contentWindow', 'Window'],
+                ['+   nodeType', '1'],
+                ['+   textContent', '"bar"'],
+                [
+                  '  contentWindow',
+                  'Referentially unequal but deeply equal objects. Consider memoization.',
+                ],
+              ],
+              tooltipText: 'App',
+              track: 'Components ⚛',
+            },
+          },
+          end: 31,
+          start: 21,
+        },
+      ],
+    ]);
+  });
 });

filename: packages/shared/ReactPerformanceTrackProperties.js
patch: @@ -82,6 +82,13 @@ export function addObjectToProperties(
   }
 }
 
+function readReactElementTypeof(value: Object): mixed {
+  // Prevents dotting into $$typeof in opaque origin windows.
+  return '$$typeof' in value && hasOwnProperty.call(value, '$$typeof')
+    ? value.$$typeof
+    : undefined;
+}
+
 export function addValueToProperties(
   propertyName: string,
   value: mixed,
@@ -96,7 +103,7 @@ export function addValueToProperties(
         desc = 'null';
         break;
       } else {
-        if (value.$$typeof === REACT_ELEMENT_TYPE) {
+        if (readReactElementTypeof(value) === REACT_ELEMENT_TYPE) {
           // JSX
           const typeName = getComponentNameFromType(value.type) || '\u2026';
           const key = value.key;
@@ -352,9 +359,10 @@ export function addObjectDiffToProperties(
           typeof nextValue === 'object' &&
           prevValue !== null &&
           nextValue !== null &&
-          prevValue.$$typeof === nextValue.$$typeof
+          readReactElementTypeof(prevValue) ===
+            readReactElementTypeof(nextValue)
         ) {
-          if (nextValue.$$typeof === REACT_ELEMENT_TYPE) {
+          if (readReactElementTypeof(nextValue) === REACT_ELEMENT_TYPE) {
             if (
               prevValue.type === nextValue.type &&
               prevValue.key === nextValue.key