Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: potential async_hooks crash in NotifyWindowRestore on Windows #41144

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 18 additions & 5 deletions shell/browser/api/electron_api_auto_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,26 @@ void AutoUpdater::OnError(const std::string& message) {
v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Object> wrapper;

// We do not use gin::EmitEvent here because we do not want to
// put this in its own CallbackScope and delegate to Node.js'
// specialized handling for Error objects.
if (GetWrapper(isolate).ToLocal(&wrapper)) {
auto error = v8::Exception::Error(gin::StringToV8(isolate, message));
gin_helper::EmitEvent(
isolate, wrapper, "error",
error->ToObject(isolate->GetCurrentContext()).ToLocalChecked(),
// Message is also emitted to keep compatibility with old code.
message);
std::vector<v8::Local<v8::Value>> args = {
gin::StringToV8(isolate, "error"),
gin::ConvertToV8(
isolate,
error->ToObject(isolate->GetCurrentContext()).ToLocalChecked()),
gin::StringToV8(isolate, message),
};

gin_helper::MicrotasksScope microtasks_scope(
isolate, wrapper->GetCreationContextChecked()->GetMicrotaskQueue(),
true);

node::MakeCallback(isolate, wrapper, "emit", args.size(), args.data(),
{0, 0});
}
}

Expand Down
8 changes: 8 additions & 0 deletions shell/common/gin_helper/event_emitter_caller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ v8::Local<v8::Value> CallMethodWithArgs(v8::Isolate* isolate,
v8::Local<v8::Object> obj,
const char* method,
ValueVector* args) {
// Only set up the node::CallbackScope if there's a node environment.
std::unique_ptr<node::CallbackScope> callback_scope;
if (node::Environment::GetCurrent(isolate)) {
v8::HandleScope handle_scope(isolate);
callback_scope = std::make_unique<node::CallbackScope>(
isolate, v8::Object::New(isolate), node::async_context{0, 0});
}

// Perform microtask checkpoint after running JavaScript.
gin_helper::MicrotasksScope microtasks_scope(
isolate, obj->GetCreationContextChecked()->GetMicrotaskQueue(), true);
Expand Down
8 changes: 3 additions & 5 deletions shell/renderer/electron_api_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,10 @@ void InvokeIpcCallback(v8::Local<v8::Context> context,

// Only set up the node::CallbackScope if there's a node environment.
// Sandboxed renderers don't have a node environment.
node::Environment* env = node::Environment::GetCurrent(context);
std::unique_ptr<node::CallbackScope> callback_scope;
if (env) {
node::async_context async_context = {};
callback_scope = std::make_unique<node::CallbackScope>(isolate, ipcNative,
async_context);
if (node::Environment::GetCurrent(context)) {
callback_scope = std::make_unique<node::CallbackScope>(
isolate, ipcNative, node::async_context{0, 0});
}

auto callback_key = gin::ConvertToV8(isolate, callback_name)
Expand Down
31 changes: 31 additions & 0 deletions spec/api-browser-window-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { closeWindow, closeAllWindows } from './lib/window-helpers';
import { areColorsSimilar, captureScreen, HexColors, getPixelColor } from './lib/screen-helpers';
import { once } from 'node:events';
import { setTimeout } from 'node:timers/promises';
import { setTimeout as syncSetTimeout } from 'node:timers';

const fixtures = path.resolve(__dirname, 'fixtures');
const mainFixtures = path.resolve(__dirname, 'fixtures');
Expand Down Expand Up @@ -1781,6 +1782,36 @@ describe('BrowserWindow module', () => {
expect(w.isFullScreen()).to.equal(true);
});

it('does not crash if maximized, minimized, then restored to maximized state', (done) => {
w.destroy();
w = new BrowserWindow({ show: false });

w.show();

let count = 0;

w.on('maximize', () => {
if (count === 0) syncSetTimeout(() => { w.minimize(); });
count++;
});

w.on('minimize', () => {
if (count === 1) syncSetTimeout(() => { w.restore(); });
count++;
});

w.on('restore', () => {
try {
throw new Error('hey!');
} catch (e: any) {
expect(e.message).to.equal('hey!');
done();
}
});

w.maximize();
});

it('checks normal bounds for maximized transparent window', async () => {
w.destroy();
w = new BrowserWindow({
Expand Down