mirror of
https://github.com/electron/electron.git
synced 2026-05-02 03:00:22 -04:00
fix: use bundled devtools frontend URL for remote debugging (#51413)
* fix: use bundled devtools frontend URL for remote debugging (#51236) fix: add ShouldUseBundledFrontendResources delegate for remote debugging Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> Co-authored-by: Om Ghante <mr.omghante1@gmail.com> * chore: e patches all (trivial only) --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Om Ghante <mr.omghante1@gmail.com> Co-authored-by: Charles Kerr <charles@charleskerr.com>
This commit is contained in:
@@ -173,3 +173,4 @@ patch_osr_control_screen_info.patch
|
||||
cherry-pick-cve-2026-6920.patch
|
||||
fix_make_macos_text_replacement_work_on_contenteditable.patch
|
||||
fix-dcheck-failure-when-starting-heap-profiler-for-renderer.patch
|
||||
fix_use_bundled_devtools_frontend_url_for_remote_debugging.patch
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Om Ghante <omghante@users.noreply.github.com>
|
||||
Date: Tue, 21 Apr 2026 23:30:00 +0530
|
||||
Subject: fix: add ShouldUseBundledFrontendResources delegate for remote
|
||||
debugging
|
||||
|
||||
Adds a new `ShouldUseBundledFrontendResources()` virtual method to
|
||||
`DevToolsManagerDelegate` that embedders can override to control whether
|
||||
the bundled DevTools frontend URL is used in the `/json` response,
|
||||
regardless of `CHROMIUM_GIT_REVISION`.
|
||||
|
||||
This is needed because embedders like Electron ship bundled frontend
|
||||
resources, but the remote frontend on the CDN may not be available for
|
||||
their custom Chromium revisions or may expire before their extended
|
||||
release cycles end.
|
||||
|
||||
Without this, `GetFrontendURLInternal()` generates a
|
||||
`devtoolsFrontendUrl` pointing to
|
||||
`chrome-devtools-frontend.appspot.com`, which returns 404 for
|
||||
embedder-specific Chromium builds, causing Chrome's `chrome://inspect`
|
||||
page to show an empty DevTools window.
|
||||
|
||||
The default implementation returns `false` to preserve existing
|
||||
behavior for Chrome and other embedders.
|
||||
|
||||
Fixes: https://github.com/electron/electron/issues/51035
|
||||
|
||||
diff --git a/content/browser/devtools/devtools_http_handler.cc b/content/browser/devtools/devtools_http_handler.cc
|
||||
index 79bf965d5dd4a9466a518f29f55d4769d9cbb060..869db7f6d090627ed8f0c301c8a11f33b2a9c944 100644
|
||||
--- a/content/browser/devtools/devtools_http_handler.cc
|
||||
+++ b/content/browser/devtools/devtools_http_handler.cc
|
||||
@@ -538,7 +538,8 @@ std::string DevToolsHttpHandler::GetFrontendURLInternal(
|
||||
const std::string& host) {
|
||||
std::string frontend_url;
|
||||
const std::string git_revision = CHROMIUM_GIT_REVISION;
|
||||
- if (git_revision == kMissingGitRevision &&
|
||||
+ if ((git_revision == kMissingGitRevision ||
|
||||
+ delegate_->ShouldUseBundledFrontendResources()) &&
|
||||
delegate_->HasBundledFrontendResources()) {
|
||||
frontend_url = "/devtools/inspector.html";
|
||||
} else {
|
||||
diff --git a/content/public/browser/devtools_manager_delegate.cc b/content/public/browser/devtools_manager_delegate.cc
|
||||
index 02e70247e20fda68510d0e1eb4e7114a1b29d153..8c191135fb73e4141041b93ac1bd8348fbdbbe2b 100644
|
||||
--- a/content/public/browser/devtools_manager_delegate.cc
|
||||
+++ b/content/public/browser/devtools_manager_delegate.cc
|
||||
@@ -106,6 +106,10 @@ bool DevToolsManagerDelegate::HasBundledFrontendResources() {
|
||||
return false;
|
||||
}
|
||||
|
||||
+bool DevToolsManagerDelegate::ShouldUseBundledFrontendResources() {
|
||||
+ return false;
|
||||
+}
|
||||
+
|
||||
bool DevToolsManagerDelegate::IsBrowserTargetDiscoverable() {
|
||||
return false;
|
||||
}
|
||||
diff --git a/content/public/browser/devtools_manager_delegate.h b/content/public/browser/devtools_manager_delegate.h
|
||||
index e6e5ba7be2551ff1482dd18cf7f7492aa0f5b609..778426dc9ced79d663c0f75d1436a9aa6d118318 100644
|
||||
--- a/content/public/browser/devtools_manager_delegate.h
|
||||
+++ b/content/public/browser/devtools_manager_delegate.h
|
||||
@@ -148,6 +148,13 @@ class CONTENT_EXPORT DevToolsManagerDelegate {
|
||||
// Returns whether frontend resources are bundled within the binary.
|
||||
virtual bool HasBundledFrontendResources();
|
||||
|
||||
+ // Returns whether the bundled frontend resources should be preferred over
|
||||
+ // the remote frontend served from the CDN. Embedders that ship bundled
|
||||
+ // frontend resources and whose Chromium revisions may not be available on
|
||||
+ // the CDN (or may expire before their release cycle ends) should override
|
||||
+ // this to return true.
|
||||
+ virtual bool ShouldUseBundledFrontendResources();
|
||||
+
|
||||
// Makes browser target easily discoverable for remote debugging.
|
||||
// This should only return true when remote debugging endpoint is not
|
||||
// accessible by the web (for example in Chrome for Android where it is
|
||||
@@ -138,6 +138,10 @@ bool DevToolsManagerDelegate::HasBundledFrontendResources() {
|
||||
return true;
|
||||
}
|
||||
|
||||
bool DevToolsManagerDelegate::ShouldUseBundledFrontendResources() {
|
||||
return true;
|
||||
}
|
||||
|
||||
content::BrowserContext* DevToolsManagerDelegate::GetDefaultBrowserContext() {
|
||||
return ElectronBrowserContext::GetDefaultBrowserContext();
|
||||
}
|
||||
|
||||
@@ -37,6 +37,7 @@ class DevToolsManagerDelegate : public content::DevToolsManagerDelegate {
|
||||
bool new_window) override;
|
||||
std::string GetDiscoveryPageHTML() override;
|
||||
bool HasBundledFrontendResources() override;
|
||||
bool ShouldUseBundledFrontendResources() override;
|
||||
content::BrowserContext* GetDefaultBrowserContext() override;
|
||||
};
|
||||
|
||||
|
||||
@@ -556,6 +556,67 @@ describe('command line switches', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('should use bundled devtools frontend URL in /json response', async () => {
|
||||
// Regression test for https://github.com/electron/electron/issues/51035
|
||||
// Verifies that devtoolsFrontendUrl points to the local bundled path
|
||||
// (/devtools/inspector.html) rather than the remote CDN URL
|
||||
// (https://chrome-devtools-frontend.appspot.com), which would 404 for
|
||||
// Electron's custom Chromium builds.
|
||||
const electronPath = process.execPath;
|
||||
let stderr = '';
|
||||
appProcess = ChildProcess.spawn(electronPath, ['--remote-debugging-port=0']);
|
||||
|
||||
const port = await new Promise<string>((resolve, reject) => {
|
||||
appProcess!.stderr.on('data', (data: Buffer) => {
|
||||
stderr += data.toString();
|
||||
const m = /DevTools listening on ws:\/\/127\.0\.0\.1:(\d+)\//.exec(stderr);
|
||||
if (m) {
|
||||
appProcess!.stderr.removeAllListeners('data');
|
||||
resolve(m[1]);
|
||||
}
|
||||
});
|
||||
appProcess!.on('exit', () =>
|
||||
reject(new Error(`Process exited before DevTools port was found. stderr: ${stderr}`))
|
||||
);
|
||||
});
|
||||
|
||||
const jsonTargets = await new Promise<any[]>((resolve, reject) => {
|
||||
http
|
||||
.get(`http://127.0.0.1:${port}/json`, (res) => {
|
||||
let body = '';
|
||||
res.on('data', (chunk: Buffer) => {
|
||||
body += chunk.toString();
|
||||
});
|
||||
res.on('end', () => {
|
||||
try {
|
||||
resolve(JSON.parse(body));
|
||||
} catch {
|
||||
reject(new Error(`Failed to parse /json response: ${body}`));
|
||||
}
|
||||
});
|
||||
res.on('error', reject);
|
||||
})
|
||||
.on('error', reject);
|
||||
});
|
||||
|
||||
expect(jsonTargets).to.be.an('array');
|
||||
expect(jsonTargets.length).to.be.greaterThan(0);
|
||||
|
||||
// Every target's devtoolsFrontendUrl must use the bundled path,
|
||||
// not the remote CDN (chrome-devtools-frontend.appspot.com).
|
||||
for (const target of jsonTargets) {
|
||||
expect(target).to.have.property('devtoolsFrontendUrl');
|
||||
expect(target.devtoolsFrontendUrl).to.match(
|
||||
/^\/devtools\//,
|
||||
`devtoolsFrontendUrl should start with /devtools/ (bundled), got: ${target.devtoolsFrontendUrl}`
|
||||
);
|
||||
expect(target.devtoolsFrontendUrl).to.not.include(
|
||||
'chrome-devtools-frontend.appspot.com',
|
||||
'devtoolsFrontendUrl should not point to the remote CDN'
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('--trace-startup switch', () => {
|
||||
|
||||
Reference in New Issue
Block a user