mirror of
https://github.com/electron/electron.git
synced 2026-05-02 03:00:22 -04:00
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 <mr.omghante1@gmail.com>
* 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 <mr.omghante1@gmail.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
This commit is contained in:
@@ -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<v8::Promise> StopRecording(gin::Arguments* const args) {
|
||||
gin_helper::Promise<base::FilePath> promise{args->isolate()};
|
||||
v8::Local<v8::Promise> 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<v8::Promise> GetCategories(v8::Isolate* isolate) {
|
||||
gin_helper::Promise<const std::set<std::string>&> promise(isolate);
|
||||
v8::Local<v8::Promise> 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<const std::set<std::string>&>::ResolvePromise,
|
||||
@@ -134,6 +147,12 @@ v8::Local<v8::Promise> StartTracing(
|
||||
gin_helper::Promise<void> promise(isolate);
|
||||
v8::Local<v8::Promise> 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<void>::ResolvePromise,
|
||||
@@ -165,6 +184,12 @@ v8::Local<v8::Promise> GetTraceBufferUsage(v8::Isolate* isolate) {
|
||||
gin_helper::Promise<gin_helper::Dictionary> promise(isolate);
|
||||
v8::Local<v8::Promise> 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)));
|
||||
|
||||
28
spec/fixtures/crash-cases/content-tracing-before-ready/index.js
vendored
Normal file
28
spec/fixtures/crash-cases/content-tracing-before-ready/index.js
vendored
Normal file
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user