From 6fd63cdba85cfafab06a97124546d985ef31fa58 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 11:08:11 -0500 Subject: [PATCH] fix: prevent crash when calling contentTracing APIs before app is ready (#51352) * fix: prevent crash when calling contentTracing APIs before app is ready Added Browser::Get()->is_ready() guards to all contentTracing API functions (startRecording, stopRecording, getCategories, getTraceBufferUsage) so they reject their returned Promises with a clear error message instead of crashing when called before app.whenReady(). Added a crash-case fixture test that validates all four APIs reject properly before readiness and work normally after. Co-authored-by: om-ghante * chore: fix linter error in `spec/fixtures/crash-cases/content-tracing-before-ready/` (#51356) chore: fix linter error in spec/fixtures/crash-cases/content-tracing-before-ready/ introduced earlier today in 6f2e5cd4 * chore: make linter happy --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: om-ghante Co-authored-by: Charles Kerr --- .../api/electron_api_content_tracing.cc | 25 +++++++++++++++++ .../content-tracing-before-ready/index.js | 28 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 spec/fixtures/crash-cases/content-tracing-before-ready/index.js diff --git a/shell/browser/api/electron_api_content_tracing.cc b/shell/browser/api/electron_api_content_tracing.cc index 8b19e08f26..fe5a7de291 100644 --- a/shell/browser/api/electron_api_content_tracing.cc +++ b/shell/browser/api/electron_api_content_tracing.cc @@ -13,6 +13,7 @@ #include "base/threading/thread_restrictions.h" #include "base/trace_event/trace_config.h" #include "content/public/browser/tracing_controller.h" +#include "shell/browser/browser.h" #include "shell/browser/javascript_environment.h" #include "shell/common/gin_converters/callback_converter.h" #include "shell/common/gin_converters/file_path_converter.h" @@ -102,6 +103,12 @@ v8::Local StopRecording(gin::Arguments* const args) { gin_helper::Promise promise{args->isolate()}; v8::Local handle = promise.GetHandle(); + if (!electron::Browser::Get()->is_ready()) { + promise.RejectWithErrorMessage( + "contentTracing cannot be used before app is ready"); + return handle; + } + base::FilePath path; if (args->GetNext(&path) && !path.empty()) { StopTracing(std::move(promise), std::make_optional(path)); @@ -120,6 +127,12 @@ v8::Local GetCategories(v8::Isolate* isolate) { gin_helper::Promise&> promise(isolate); v8::Local handle = promise.GetHandle(); + if (!electron::Browser::Get()->is_ready()) { + promise.RejectWithErrorMessage( + "contentTracing cannot be used before app is ready"); + return handle; + } + // Note: This method always succeeds. TracingController::GetInstance()->GetCategories(base::BindOnce( gin_helper::Promise&>::ResolvePromise, @@ -134,6 +147,12 @@ v8::Local StartTracing( gin_helper::Promise promise(isolate); v8::Local handle = promise.GetHandle(); + if (!electron::Browser::Get()->is_ready()) { + promise.RejectWithErrorMessage( + "contentTracing cannot be used before app is ready"); + return handle; + } + if (!TracingController::GetInstance()->StartTracing( trace_config, base::BindOnce(gin_helper::Promise::ResolvePromise, @@ -165,6 +184,12 @@ v8::Local GetTraceBufferUsage(v8::Isolate* isolate) { gin_helper::Promise promise(isolate); v8::Local handle = promise.GetHandle(); + if (!electron::Browser::Get()->is_ready()) { + promise.RejectWithErrorMessage( + "contentTracing cannot be used before app is ready"); + return handle; + } + // Note: This method always succeeds. TracingController::GetInstance()->GetTraceBufferUsage( base::BindOnce(&OnTraceBufferUsageAvailable, std::move(promise))); diff --git a/spec/fixtures/crash-cases/content-tracing-before-ready/index.js b/spec/fixtures/crash-cases/content-tracing-before-ready/index.js new file mode 100644 index 0000000000..d0c2d02aa1 --- /dev/null +++ b/spec/fixtures/crash-cases/content-tracing-before-ready/index.js @@ -0,0 +1,28 @@ +const { app, contentTracing } = require('electron'); + +const assert = require('node:assert/strict'); + +(async () => { + // Before app is ready, all contentTracing methods should reject + // instead of crashing. + if (!app.isReady()) { + await assert.rejects(() => contentTracing.startRecording({ included_categories: ['*'] }), /before app is ready/); + + await assert.rejects(() => contentTracing.stopRecording(), /before app is ready/); + + await assert.rejects(() => contentTracing.getCategories(), /before app is ready/); + + await assert.rejects(() => contentTracing.getTraceBufferUsage(), /before app is ready/); + } + + await app.whenReady(); + + // After app is ready, startRecording should work normally. + await contentTracing.startRecording({ included_categories: ['*'] }); + await contentTracing.stopRecording(); +})() + .then(app.quit) + .catch((err) => { + console.error(err); + app.exit(1); + });