From 297d27f2d72c7e9785b7bb189cc656acf8eb9126 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Thu, 9 Sep 2021 17:31:19 -0400 Subject: [PATCH 1/4] [DevTools] Handle case when cached network events have null content --- .../react-devtools-extensions/src/main.js | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index 8781d6e22dbb..dfd8243b1ff3 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -25,7 +25,7 @@ const LOCAL_STORAGE_SUPPORTS_PROFILING_KEY = const isChrome = getBrowserName() === 'Chrome'; -const cachedNetworkEvents = new Map(); +const cachedSources = new Map(); // Cache JavaScript resources as the page loads them. // This helps avoid unnecessary duplicate requests when hook names are parsed. @@ -33,13 +33,17 @@ const cachedNetworkEvents = new Map(); // This lets us avoid a possible (expensive) cache miss. // For more info see: github.com/facebook/react/pull/22198 chrome.devtools.network.onRequestFinished.addListener( - function onRequestFinished(event) { - if (event.request.method === 'GET') { - switch (event.response.content.mimeType) { + function onRequestFinished(request) { + if (request.request.method === 'GET') { + switch (request.response.content.mimeType) { case 'application/javascript': case 'application/x-javascript': case 'text/javascript': - cachedNetworkEvents.set(event.request.url, event); + request.getContent(content => { + if (content != null) { + cachedSources.set(request.request.url, content); + } + }); break; } } @@ -242,13 +246,13 @@ function createPanelIfReactLoaded() { // This helper function allows the extension to request files to be fetched // by the content script (running in the page) to increase the likelihood of a cache hit. fetchFileWithCaching = url => { - const event = cachedNetworkEvents.get(url); - if (event != null) { + const content = cachedSources.get(url); + if (content != null) { // If this resource has already been cached locally, // skip the network queue (which might not be a cache hit anyway) // and just use the cached response. return new Promise(resolve => { - event.getContent(content => resolve(content)); + resolve(content); }); } @@ -442,7 +446,7 @@ function createPanelIfReactLoaded() { // Re-initialize DevTools panel when a new page is loaded. chrome.devtools.network.onNavigated.addListener(function onNavigated() { // Clear cached requests when a new page is opened. - cachedNetworkEvents.clear(); + cachedSources.clear(); // Re-initialize saved filters on navigation, // since global values stored on window get reset in this case. @@ -461,7 +465,7 @@ function createPanelIfReactLoaded() { // Load (or reload) the DevTools extension when the user navigates to a new page. function checkPageForReact() { // Clear cached requests when a new page is opened. - cachedNetworkEvents.clear(); + cachedSources.clear(); syncSavedPreferences(); createPanelIfReactLoaded(); From dc8c512bb410ad9a00712528a88d64f6e7662462 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Fri, 10 Sep 2021 11:10:03 -0400 Subject: [PATCH 2/4] [DevTools] Add DEBUG logging for fetching cached source files --- .../react-devtools-extensions/src/main.js | 44 ++++++++++++++++++- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index dfd8243b1ff3..3d8fd3650338 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -6,6 +6,7 @@ import Bridge from 'react-devtools-shared/src/bridge'; import Store from 'react-devtools-shared/src/devtools/store'; import {getBrowserName, getBrowserTheme} from './utils'; import {LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY} from 'react-devtools-shared/src/constants'; +import {__DEBUG__} from 'react-devtools-shared/src/constants'; import { getAppendComponentStack, getBreakOnConsoleErrors, @@ -39,9 +40,29 @@ chrome.devtools.network.onRequestFinished.addListener( case 'application/javascript': case 'application/x-javascript': case 'text/javascript': + const url = request.request.url; + if (__DEBUG__) { + console.log( + '[main] onRequestFinished() Request finished for ', + url, + ); + } request.getContent(content => { if (content != null) { - cachedSources.set(request.request.url, content); + if (__DEBUG__) { + console.log( + '[main] onRequestFinished() Cached source file content for ', + url, + ); + } + cachedSources.set(url, content); + } else { + if (__DEBUG__) { + console.log( + '[main] onRequestFinished() Invalid content returned by getContent() for ', + url, + ); + } } }); break; @@ -248,6 +269,13 @@ function createPanelIfReactLoaded() { fetchFileWithCaching = url => { const content = cachedSources.get(url); if (content != null) { + if (__DEBUG__) { + console.log( + '[main] fetchFileWithCaching() Found a cached source file for ', + url, + ); + } + // If this resource has already been cached locally, // skip the network queue (which might not be a cache hit anyway) // and just use the cached response. @@ -256,10 +284,22 @@ function createPanelIfReactLoaded() { }); } + if (__DEBUG__) { + console.log( + '[main] fetchFileWithCaching() No cached source file found for ', + url, + ); + } + // If DevTools was opened after the page started loading, // we may have missed some requests. // So fall back to a fetch() and hope we get a cached response. - + if (__DEBUG__) { + console.log( + '[main] fetchFileWithCaching() Fetching from page: ', + url, + ); + } return new Promise((resolve, reject) => { function onPortMessage({payload, source}) { if (source === 'react-devtools-content-script') { From ef1f2931d83e086777f80e50bf2e0ece36971a18 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Fri, 10 Sep 2021 13:33:20 -0400 Subject: [PATCH 3/4] [DevTools] Dont eagerly getContent for requests, only handle nullability of content --- .../react-devtools-extensions/src/main.js | 102 +++++++++--------- 1 file changed, 52 insertions(+), 50 deletions(-) diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index 3d8fd3650338..06c4a5927cdb 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -26,7 +26,7 @@ const LOCAL_STORAGE_SUPPORTS_PROFILING_KEY = const isChrome = getBrowserName() === 'Chrome'; -const cachedSources = new Map(); +const cachedRequests = new Map(); // Cache JavaScript resources as the page loads them. // This helps avoid unnecessary duplicate requests when hook names are parsed. @@ -47,24 +47,7 @@ chrome.devtools.network.onRequestFinished.addListener( url, ); } - request.getContent(content => { - if (content != null) { - if (__DEBUG__) { - console.log( - '[main] onRequestFinished() Cached source file content for ', - url, - ); - } - cachedSources.set(url, content); - } else { - if (__DEBUG__) { - console.log( - '[main] onRequestFinished() Invalid content returned by getContent() for ', - url, - ); - } - } - }); + cachedRequests.set(url, request); break; } } @@ -267,40 +250,59 @@ function createPanelIfReactLoaded() { // This helper function allows the extension to request files to be fetched // by the content script (running in the page) to increase the likelihood of a cache hit. fetchFileWithCaching = url => { - const content = cachedSources.get(url); - if (content != null) { + if (__DEBUG__) { + console.log('[main] fetchFileWithCaching() for ', url); + } + return new Promise((resolve, reject) => { + const request = cachedRequests.get(url); + let cachedContent = null; + if (request != null) { + if (__DEBUG__) { + console.log( + '[main] fetchFileWithCaching() Found a cached network request for ', + url, + ); + } + + request.getContent(content => { + if (content != null) { + cachedContent = content; + } else { + if (__DEBUG__) { + console.log( + '[main] fetchFileWithCaching() Invalid source file content returned by getContent() for ', + url, + ); + } + } + }); + } + + // If this request has already been cached locally, + // skip the network queue (which might not be a cache hit anyway) + // and use the cached response. + if (cachedContent != null) { + if (__DEBUG__) { + console.log( + '[main] fetchFileWithCaching() Reusing cached source file content for ', + url, + ); + } + resolve(cachedContent); + return; + } + + // If we didn't find a cached network request, or were unable to + // obtain valid content for the cached request, fall back to a fetch() + // and hope we get a cached response. + // We might miss caching some network requests if DevTools was opened + // after the page started loading. if (__DEBUG__) { console.log( - '[main] fetchFileWithCaching() Found a cached source file for ', + '[main] fetchFileWithCaching() Fetching from page: ', url, ); } - - // If this resource has already been cached locally, - // skip the network queue (which might not be a cache hit anyway) - // and just use the cached response. - return new Promise(resolve => { - resolve(content); - }); - } - - if (__DEBUG__) { - console.log( - '[main] fetchFileWithCaching() No cached source file found for ', - url, - ); - } - - // If DevTools was opened after the page started loading, - // we may have missed some requests. - // So fall back to a fetch() and hope we get a cached response. - if (__DEBUG__) { - console.log( - '[main] fetchFileWithCaching() Fetching from page: ', - url, - ); - } - return new Promise((resolve, reject) => { function onPortMessage({payload, source}) { if (source === 'react-devtools-content-script') { switch (payload?.type) { @@ -486,7 +488,7 @@ function createPanelIfReactLoaded() { // Re-initialize DevTools panel when a new page is loaded. chrome.devtools.network.onNavigated.addListener(function onNavigated() { // Clear cached requests when a new page is opened. - cachedSources.clear(); + cachedRequests.clear(); // Re-initialize saved filters on navigation, // since global values stored on window get reset in this case. @@ -505,7 +507,7 @@ function createPanelIfReactLoaded() { // Load (or reload) the DevTools extension when the user navigates to a new page. function checkPageForReact() { // Clear cached requests when a new page is opened. - cachedSources.clear(); + cachedRequests.clear(); syncSavedPreferences(); createPanelIfReactLoaded(); From de9dbe55c1effc03abab1b41b3f3b70dc2dc6569 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Fri, 10 Sep 2021 14:36:59 -0400 Subject: [PATCH 4/4] Correctly wait for content for cached request to be loaded --- .../react-devtools-extensions/src/main.js | 101 +++++++++--------- 1 file changed, 52 insertions(+), 49 deletions(-) diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index 06c4a5927cdb..8fda12d685e4 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -43,7 +43,7 @@ chrome.devtools.network.onRequestFinished.addListener( const url = request.request.url; if (__DEBUG__) { console.log( - '[main] onRequestFinished() Request finished for ', + '[main] onRequestFinished() Caching request that finished for ', url, ); } @@ -245,6 +245,39 @@ function createPanelIfReactLoaded() { // never reaches the chrome.runtime.onMessage event listener. let fetchFileWithCaching = null; if (isChrome) { + const fetchFromPage = (url, resolve, reject) => { + if (__DEBUG__) { + console.log('[main] fetchFromPage()', url); + } + + function onPortMessage({payload, source}) { + if (source === 'react-devtools-content-script') { + switch (payload?.type) { + case 'fetch-file-with-cache-complete': + chrome.runtime.onMessage.removeListener(onPortMessage); + resolve(payload.value); + break; + case 'fetch-file-with-cache-error': + chrome.runtime.onMessage.removeListener(onPortMessage); + reject(payload.value); + break; + } + } + } + + chrome.runtime.onMessage.addListener(onPortMessage); + + chrome.devtools.inspectedWindow.eval(` + window.postMessage({ + source: 'react-devtools-extension', + payload: { + type: 'fetch-file-with-cache', + url: "${url}", + }, + }); + `); + }; + // Fetching files from the extension won't make use of the network cache // for resources that have already been loaded by the page. // This helper function allows the extension to request files to be fetched @@ -255,8 +288,12 @@ function createPanelIfReactLoaded() { } return new Promise((resolve, reject) => { const request = cachedRequests.get(url); - let cachedContent = null; if (request != null) { + // If this request has already been cached locally, check if + // we can obtain the cached response, and if so + // skip making a network request (which might not be a cache hit anyway) + // and use our cached response. + if (__DEBUG__) { console.log( '[main] fetchFileWithCaching() Found a cached network request for ', @@ -266,7 +303,13 @@ function createPanelIfReactLoaded() { request.getContent(content => { if (content != null) { - cachedContent = content; + if (__DEBUG__) { + console.log( + '[main] fetchFileWithCaching() Reusing cached source file content for ', + url, + ); + } + resolve(content); } else { if (__DEBUG__) { console.log( @@ -274,61 +317,21 @@ function createPanelIfReactLoaded() { url, ); } + + // Edge case where getContent() returned null; fall back to fetch. + fetchFromPage(url, resolve, reject); } }); - } - // If this request has already been cached locally, - // skip the network queue (which might not be a cache hit anyway) - // and use the cached response. - if (cachedContent != null) { - if (__DEBUG__) { - console.log( - '[main] fetchFileWithCaching() Reusing cached source file content for ', - url, - ); - } - resolve(cachedContent); return; } // If we didn't find a cached network request, or were unable to - // obtain valid content for the cached request, fall back to a fetch() - // and hope we get a cached response. + // obtain a valid cached response to that request, fall back to a fetch() + // and hope we get a cached response from the browser. // We might miss caching some network requests if DevTools was opened // after the page started loading. - if (__DEBUG__) { - console.log( - '[main] fetchFileWithCaching() Fetching from page: ', - url, - ); - } - function onPortMessage({payload, source}) { - if (source === 'react-devtools-content-script') { - switch (payload?.type) { - case 'fetch-file-with-cache-complete': - chrome.runtime.onMessage.removeListener(onPortMessage); - resolve(payload.value); - break; - case 'fetch-file-with-cache-error': - chrome.runtime.onMessage.removeListener(onPortMessage); - reject(payload.value); - break; - } - } - } - - chrome.runtime.onMessage.addListener(onPortMessage); - - chrome.devtools.inspectedWindow.eval(` - window.postMessage({ - source: 'react-devtools-extension', - payload: { - type: 'fetch-file-with-cache', - url: "${url}", - }, - }); - `); + fetchFromPage(url, resolve, reject); }); }; }