Skip to content

Commit

Permalink
refactor[devtools/extension]: handle ports disconnection, instead of …
Browse files Browse the repository at this point in the history 
…frequent reconnections
  • Loading branch information
@hoxyq
hoxyq committed Sep 5, 2023
1 parent 9b4f847 commit 00b7c43
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 111 deletions.
62 changes: 40 additions & 22 deletions packages/react-devtools-extensions/src/background/dynamicallyInjectContentScripts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,46 @@

import {IS_FIREFOX} from '../utils';

async function dynamicallyInjectContentScripts() {
const contentScriptsToInject = [
{
id: '@react-devtools/hook',
js: ['build/installHook.js'],
matches: ['<all_urls>'],
persistAcrossSessions: true,
runAt: 'document_start',
world: chrome.scripting.ExecutionWorld.MAIN,
},
{
id: '@react-devtools/renderer',
js: ['build/renderer.js'],
matches: ['<all_urls>'],
persistAcrossSessions: true,
runAt: 'document_start',
world: chrome.scripting.ExecutionWorld.MAIN,
},
];
// Firefox doesn't support ExecutionWorld.MAIN yet
// equivalent logic for Firefox is in prepareInjection.js
const contentScriptsToInject = IS_FIREFOX
? [
{
id: '@react-devtools/proxy',
js: ['build/proxy.js'],
matches: ['<all_urls>'],
persistAcrossSessions: true,
runAt: 'document_idle',
},
]
: [
{
id: '@react-devtools/proxy',
js: ['build/proxy.js'],
matches: ['<all_urls>'],
persistAcrossSessions: true,
runAt: 'document_end',
world: chrome.scripting.ExecutionWorld.ISOLATED,
},
{
id: '@react-devtools/hook',
js: ['build/installHook.js'],
matches: ['<all_urls>'],
persistAcrossSessions: true,
runAt: 'document_start',
world: chrome.scripting.ExecutionWorld.MAIN,
},
{
id: '@react-devtools/renderer',
js: ['build/renderer.js'],
matches: ['<all_urls>'],
persistAcrossSessions: true,
runAt: 'document_start',
world: chrome.scripting.ExecutionWorld.MAIN,
},
];

async function dynamicallyInjectContentScripts() {
try {
const alreadyRegisteredContentScripts =
await chrome.scripting.getRegisteredContentScripts();
Expand All @@ -48,6 +68,4 @@ async function dynamicallyInjectContentScripts() {
}
}

if (!IS_FIREFOX) {
dynamicallyInjectContentScripts();
}
dynamicallyInjectContentScripts();
79 changes: 19 additions & 60 deletions packages/react-devtools-extensions/src/background/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {IS_FIREFOX, EXTENSION_CONTAINED_VERSIONS} from '../utils';
import './dynamicallyInjectContentScripts';
import './tabsManager';
import setExtensionIconAndPopup from './setExtensionIconAndPopup';
import injectProxy from './injectProxy';

/*
{
Expand Down Expand Up @@ -39,18 +38,6 @@ function registerExtensionPort(port, tabId) {
ports[tabId].disconnectPipe?.();

delete ports[tabId].extension;

const proxyPort = ports[tabId].proxy;
if (proxyPort) {
// Do not disconnect proxy port, we will inject this content script again
// If extension port has disconnected, it probably means that user did in-tab navigation
clearReconnectionTimeout(proxyPort);

proxyPort.postMessage({
source: 'react-devtools-service-worker',
stop: true,
});
}
});
}

Expand All @@ -59,36 +46,12 @@ function registerProxyPort(port, tabId) {

// In case proxy port was disconnected from the other end, from content script
// This can happen if content script was detached, when user does in-tab navigation
// Or if when we notify proxy port to stop reconnecting, when extension port dies
// This listener should never be called when we call port.shutdown() from this (background/index.js) script
// This listener should never be called when we call port.disconnect() from this (background/index.js) script
port.onDisconnect.addListener(() => {
ports[tabId].disconnectPipe?.();

delete ports[tabId].proxy;
});

port._reconnectionTimeoutId = setTimeout(
reconnectProxyPort,
25_000,
port,
tabId,
);
}

function clearReconnectionTimeout(port) {
if (port._reconnectionTimeoutId) {
clearTimeout(port._reconnectionTimeoutId);
delete port._reconnectionTimeoutId;
}
}

function reconnectProxyPort(port, tabId) {
// IMPORTANT: port.onDisconnect will only be emitted if disconnect() was called from the other end
// We need to do it manually here if we disconnect proxy port from service worker
ports[tabId].disconnectPipe?.();

// It should be reconnected automatically by proxy content script, look at proxy.js
port.disconnect();
}

function isNumeric(str: string): boolean {
Expand All @@ -100,42 +63,38 @@ chrome.runtime.onConnect.addListener(port => {
// Proxy content script is executed in tab, so it should have it specified.
const tabId = port.sender.tab.id;

if (ports[tabId]?.proxy) {
port.disconnect();
return;
}

registerTab(tabId);
registerProxyPort(port, tabId);

connectExtensionAndProxyPorts(
ports[tabId].extension,
ports[tabId].proxy,
tabId,
);
if (ports[tabId].extension) {
connectExtensionAndProxyPorts(
ports[tabId].extension,
ports[tabId].proxy,
tabId,
);
}

return;
}

if (isNumeric(port.name)) {
// Extension port doesn't have tab id specified, because its sender is the extension.
const tabId = +port.name;
const extensionPortAlreadyConnected = ports[tabId]?.extension != null;

// Handle the case when extension port was disconnected and we were not notified
if (extensionPortAlreadyConnected) {
ports[tabId].disconnectPipe?.();
}

registerTab(tabId);
registerExtensionPort(port, tabId);

if (extensionPortAlreadyConnected) {
const proxyPort = ports[tabId].proxy;

// Avoid re-injecting the content script, we might end up in a situation
// where we would have multiple proxy ports opened and trying to reconnect
if (proxyPort) {
clearReconnectionTimeout(proxyPort);
reconnectProxyPort(proxyPort, tabId);
}
} else {
injectProxy(tabId);
if (ports[tabId].proxy) {
connectExtensionAndProxyPorts(
ports[tabId].extension,
ports[tabId].proxy,
tabId,
);
}

return;
Expand Down
12 changes: 0 additions & 12 deletions packages/react-devtools-extensions/src/contentScripts/proxy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,6 @@ function sayHelloToBackendManager() {
}

function handleMessageFromDevtools(message) {
if (message.source === 'react-devtools-service-worker' && message.stop) {
window.removeEventListener('message', handleMessageFromPage);

// Calling disconnect here should not emit onDisconnect event inside this script
// This port will not attempt to reconnect again
// It will connect only once this content script will be injected again
port?.disconnect();
port = null;

return;
}

window.postMessage(
{
source: 'react-devtools-content-script',
Expand Down
12 changes: 12 additions & 0 deletions packages/react-devtools-extensions/src/main/debounce.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/* global chrome */

function debounce(fn, timeout) {
let executionTimeoutId = null;

return (...args) => {
clearTimeout(executionTimeoutId);
executionTimeoutId = setTimeout(fn, timeout, ...args);
};
}

export default debounce;
60 changes: 43 additions & 17 deletions packages/react-devtools-extensions/src/main/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,21 @@ import injectBackendManager from './injectBackendManager';
import syncSavedPreferences from './syncSavedPreferences';
import registerEventsLogger from './registerEventsLogger';
import getProfilingFlags from './getProfilingFlags';
import debounce from './debounce';
import './requestAnimationFramePolyfill';

// Try polling for at least 5 seconds, in case if it takes too long to load react
const REACT_POLLING_TICK_COOLDOWN = 250;
const REACT_POLLING_ATTEMPTS_THRESHOLD = 20;

let reactPollingTimeoutId = null;
function clearReactPollingTimeout() {
export function clearReactPollingTimeout() {
clearTimeout(reactPollingTimeoutId);
reactPollingTimeoutId = null;
}

function executeIfReactHasLoaded(callback, attempt = 1) {
reactPollingTimeoutId = null;
export function executeIfReactHasLoaded(callback, attempt = 1) {
clearReactPollingTimeout();

if (attempt > REACT_POLLING_ATTEMPTS_THRESHOLD) {
return;
Expand Down Expand Up @@ -81,21 +82,26 @@ function executeIfReactHasLoaded(callback, attempt = 1) {
);
}

let lastSubscribedBridgeListener = null;

function createBridge() {
bridge = new Bridge({
listen(fn) {
const listener = message => fn(message);
const bridgeListener = message => fn(message);
// Store the reference so that we unsubscribe from the same object.
const portOnMessage = port.onMessage;
portOnMessage.addListener(listener);
portOnMessage.addListener(bridgeListener);

lastSubscribedBridgeListener = bridgeListener;

return () => {
portOnMessage.removeListener(listener);
port?.onMessage.removeListener(bridgeListener);
lastSubscribedBridgeListener = null;
};
},

send(event: string, payload: any, transferable?: Array<any>) {
port.postMessage({event, payload}, transferable);
port?.postMessage({event, payload}, transferable);
},
});

Expand Down Expand Up @@ -469,9 +475,6 @@ function performInTabNavigationCleanup() {
bridge = null;
render = null;
root = null;

port?.disconnect();
port = null;
}

function performFullCleanup() {
Expand Down Expand Up @@ -499,23 +502,37 @@ function performFullCleanup() {
}

function connectExtensionPort() {
if (port) {
throw new Error('DevTools port was already connected');
}

const tabId = chrome.devtools.inspectedWindow.tabId;
port = chrome.runtime.connect({
name: String(tabId),
});

// If DevTools port was reconnected and Bridge was already created
// We should subscribe bridge to this port events
// This could happen if service worker dies and all ports are disconnected,
// but later user continues the session and Chrome reconnects all ports
// Bridge object is still in-memory, though
if (lastSubscribedBridgeListener) {
port.onMessage.addListener(lastSubscribedBridgeListener);
}

// This port may be disconnected by Chrome at some point, this callback
// will be executed only if this port was disconnected from the other end
// so, when we call `port.disconnect()` from this script,
// this should not trigger this callback and port reconnection
port.onDisconnect.addListener(connectExtensionPort);
port.onDisconnect.addListener(() => {
port = null;
connectExtensionPort();
});
}

function mountReactDevTools() {
registerEventsLogger();

connectExtensionPort();

createBridgeAndStore();

setReactSelectionFromBrowser(bridge);
Expand All @@ -532,7 +549,7 @@ function mountReactDevToolsWhenReactHasLoaded() {
mountReactDevTools();
}

executeIfReactHasLoaded(onReactReady);
executeIfReactHasLoaded(onReactReady, 1);
}

let bridge = null;
Expand All @@ -555,11 +572,18 @@ let port = null;
// since global values stored on window get reset in this case.
chrome.devtools.network.onNavigated.addListener(syncSavedPreferences);

// Cleanup previous page state and remount everything
chrome.devtools.network.onNavigated.addListener(() => {
// In case when multiple navigation events emitted in a short period of time
// This debounced callback primarily used to avoid mounting React DevTools multiple times, which results
// into subscribing to the same events from Bridge and window multiple times
// In this case, we will handle `operations` event twice or more and user will see
// `Cannot add node "1" because a node with that id is already in the Store.`
const debouncedOnNavigatedListener = debounce(() => {
performInTabNavigationCleanup();
mountReactDevToolsWhenReactHasLoaded();
});
}, 500);

// Cleanup previous page state and remount everything
chrome.devtools.network.onNavigated.addListener(debouncedOnNavigatedListener);

// Should be emitted when browser DevTools are closed
if (IS_FIREFOX) {
Expand All @@ -569,5 +593,7 @@ if (IS_FIREFOX) {
window.addEventListener('beforeunload', performFullCleanup);
}

connectExtensionPort();

syncSavedPreferences();
mountReactDevToolsWhenReactHasLoaded();

0 comments on commit 00b7c43

Please sign in to comment.