Fixed potential race cases in bridge/store/backend initialization

shells/browser/shared/src/contentScript.js

@@ -1,21 +1,17 @@
 /* global chrome */
 
-// proxy from main page to devtools (via the background page)
-var port = chrome.runtime.connect({
-  name: 'content-script',
-});
+let backendDisconnected: boolean = false;
+let backendInitialized: boolean = false;
 
-port.onMessage.addListener(handleMessageFromDevtools);
-port.onDisconnect.addListener(handleDisconnect);
-window.addEventListener('message', handleMessageFromPage);
-
-window.postMessage(
-  {
-    source: 'react-devtools-content-script',
-    hello: true,
-  },
-  '*'
-);
+function sayHelloToBackend() {
+  window.postMessage(
+    {
+      source: 'react-devtools-content-script',
+      hello: true,
+    },
+    '*'
+  );
+}
 
 function handleMessageFromDevtools(message) {
   window.postMessage(
@@ -33,12 +29,17 @@ function handleMessageFromPage(evt) {
     evt.data &&
     evt.data.source === 'react-devtools-bridge'
   ) {
+    backendInitialized = true;
+
     port.postMessage(evt.data.payload);
   }
 }
 
 function handleDisconnect() {
+  backendDisconnected = true;
+
   window.removeEventListener('message', handleMessageFromPage);
+
   window.postMessage(
     {
       source: 'react-devtools-content-script',
@@ -50,3 +51,23 @@ function handleDisconnect() {
     '*'
   );
 }
+
+// proxy from main page to devtools (via the background page)
+var port = chrome.runtime.connect({
+  name: 'content-script',
+});
+port.onMessage.addListener(handleMessageFromDevtools);
+port.onDisconnect.addListener(handleDisconnect);
+
+window.addEventListener('message', handleMessageFromPage);
+
+// The backend waits to install the global hook until notified by the content script.
+// In the event of a page reload, the content script might be loaded before the backend is injected.
+// Because of this we need to poll the backend until it has been initialized.
+const intervalID = setInterval(() => {
+  if (backendInitialized || backendDisconnected) {
+    clearInterval(intervalID);
+  } else {
+    sayHelloToBackend();
+  }
+}, 500);

shells/browser/shared/src/inject.js

@@ -1,7 +1,7 @@
 /* global chrome */
 
-export default function inject(scriptName: string, done: () => void) {
-  const src = `
+export default function inject(scriptName: string, done: ?Function) {
+  const source = `
   // the prototype stuff is in case document.createElement has been modified
   (function () {
     var script = document.constructor.prototype.createElement.call(document, 'script');
@@ -12,10 +12,13 @@ export default function inject(scriptName: string, done: () => void) {
   })()
   `;
 
-  chrome.devtools.inspectedWindow.eval(src, function(res, err) {
-    if (err) {
-      console.log(err);
+  chrome.devtools.inspectedWindow.eval(source, function(response, error) {
+    if (error) {
+      console.log(error);
+    }
+
+    if (typeof done === 'function') {
+      done();
     }
-    done();
   });
 }

shells/browser/shared/src/panel.js

@@ -1,7 +1,7 @@
 /* global chrome */
 
 import { createElement } from 'react';
-import { render, unmountComponentAtNode } from 'react-dom';
+import { createRoot, flushSync } from 'react-dom';
 import Bridge from 'src/bridge';
 import Elements from 'src/devtools/views/Elements';
 import inject from './inject';
@@ -34,58 +34,55 @@ if (IS_CHROME) {
   }
 }
 
-const node = ((document.getElementById('container'): any): HTMLElement);
-
-function reloadDevTools() {
-  setTimeout(() => {
-    unmountComponentAtNode(node);
-
-    node.innerHTML = '';
-
-    injectAndInit();
-  }, 100);
-}
+const container = ((document.getElementById('container'): any): HTMLElement);
 
 function injectAndInit() {
-  inject(chrome.runtime.getURL('build/backend.js'), () => {
-    let disconnected = false;
+  let disconnected = false;
 
-    const port = chrome.runtime.connect({
-      name: '' + chrome.devtools.inspectedWindow.tabId,
-    });
-    port.onDisconnect.addListener(() => {
-      disconnected = true;
-    });
-
-    const bridge = new Bridge({
-      listen(fn) {
-        port.onMessage.addListener(message => fn(message));
-      },
-      send(event: string, payload: any, transferable?: Array<any>) {
-        if (disconnected) {
-          return;
-        }
-        port.postMessage({ event, payload }, transferable);
-      },
-    });
-
-    // Reload the DevTools extension when the user navigates to a new page.
-    function onNavigated() {
-      chrome.devtools.network.onNavigated.removeListener(onNavigated);
-      bridge.send('shutdown');
-      reloadDevTools();
-    }
-    chrome.devtools.network.onNavigated.addListener(onNavigated);
-
-    render(
-      createElement(Elements, {
-        bridge,
-        browserName,
-        themeName,
-      }),
-      node
-    );
+  const port = chrome.runtime.connect({
+    name: '' + chrome.devtools.inspectedWindow.tabId,
   });
+  port.onDisconnect.addListener(() => {
+    disconnected = true;
+  });
+
+  const bridge = new Bridge({
+    listen(fn) {
+      port.onMessage.addListener(message => fn(message));
+    },
+    send(event: string, payload: any, transferable?: Array<any>) {
+      if (disconnected) {
+        return;
+      }
+      port.postMessage({ event, payload }, transferable);
+    },
+  });
+
+  // Clear the "React not found" initial message before rendering.
+  container.innerHTML = '';
+
+  const root = createRoot(container);
+  root.render(
+    createElement(Elements, {
+      bridge,
+      browserName,
+      themeName,
+    })
+  );
+
+  // Initialize the backend only once the DevTools frontend Store has been initialized.
+  // Otherwise the Store may miss important initial tree op codes.
+  inject(chrome.runtime.getURL('build/backend.js'));
+
+  // Reload the DevTools extension when the user navigates to a new page.
+  function onNavigated() {
+    chrome.devtools.network.onNavigated.removeListener(onNavigated);
+
+    bridge.send('shutdown');
+
+    flushSync(() => root.unmount(injectAndInit));
+  }
+  chrome.devtools.network.onNavigated.addListener(onNavigated);
 }
 
 injectAndInit();

shells/dev/src/devtools.js

@@ -37,9 +37,9 @@ mountButton.addEventListener('click', function() {
   }
 });
 
-initDevTools({
-  connect(cb) {
-    inject('./build/backend.js', () => {
+inject('./build/App.js', () => {
+  initDevTools({
+    connect(cb) {
       const bridge = new Bridge({
         listen(fn) {
           contentWindow.parent.addEventListener('message', ({ data }) => {
@@ -54,23 +54,29 @@ initDevTools({
       cb(bridge);
 
       const root = createRoot(container);
-      root.render(
+      const batch = root.createBatch();
+      batch.render(
         createElement(Elements, {
           bridge,
           browserName: 'Chrome',
           themeName: 'light',
         })
       );
-    });
-  },
+      batch.then(() => {
+        batch.commit();
 
-  onReload(reloadFn) {
-    iframe.onload = reloadFn;
-  },
+        // Initialize the backend only once the DevTools frontend Store has been initialized.
+        // Otherwise the Store may miss important initial tree op codes.
+        inject('./build/backend.js');
+      });
+    },
+
+    onReload(reloadFn) {
+      iframe.onload = reloadFn;
+    },
+  });
 });
 
-inject('./build/App.js');
-
 function inject(sourcePath, callback) {
   const script = contentDocument.createElement('script');
   script.onload = callback;

src/bridge.js

@@ -15,13 +15,12 @@ export default class Bridge extends EventEmitter {
   _messageQueue: Array<any> = [];
   _time: number | null = null;
   _timeoutID: TimeoutID | null = null;
-
-  wall: Wall;
+  _wall: Wall;
 
   constructor(wall: Wall) {
     super();
 
-    this.wall = wall;
+    this._wall = wall;
 
     wall.listen((message: Message) => {
       this._emit(message);
@@ -32,7 +31,7 @@ export default class Bridge extends EventEmitter {
     const time = this._time;
 
     if (time === null) {
-      this.wall.send(event, payload, transferable);
+      this._wall.send(event, payload, transferable);
       this._time = Date.now();
     } else {
       this._messageQueue.push(event, payload, transferable);
@@ -46,13 +45,9 @@ export default class Bridge extends EventEmitter {
     }
   }
 
-  log(message: string): void {
-    this.send('log', message);
-  }
-
   _flush() {
     while (this._messageQueue.length) {
-      this.wall.send.apply(this.wall, this._messageQueue.splice(0, 3));
+      this._wall.send.apply(this._wall, this._messageQueue.splice(0, 3));
     }
 
     if (this._timeoutID !== null) {
@@ -64,11 +59,7 @@ export default class Bridge extends EventEmitter {
     this._time = null;
   }
 
-  _emit(message: string | Message) {
-    if (typeof message === 'string') {
-      this.emit(message);
-    } else {
-      this.emit(message.event, message.payload);
-    }
+  _emit(message: Message) {
+    this.emit(message.event, message.payload);
   }
 }