feat: promisify webContents.printToPDF() #16795
Conversation
9065704 to
8d8fac3
Compare
| v8::Exception::Error(error_message), v8::Null(isolate)); | ||
| } | ||
| print_to_pdf_callback_map_.erase(request_id); | ||
| v8::MicrotasksScope script_scope(isolate, |
There was a problem hiding this comment.
the util file deals with scoping prior to resolution; this can be removed
There was a problem hiding this comment.
will remove the script_scope, should I keep the context_scope though?
There was a problem hiding this comment.
we handle it in the util, so i think you can remove that too!
v8::Maybe<bool> Resolve() {
v8::HandleScope handle_scope(isolate());
v8::MicrotasksScope script_scope(isolate(),
v8::MicrotasksScope::kRunMicrotasks);
v8::Context::Scope context_scope(
v8::Local<v8::Context>::New(isolate(), GetContext()));
return GetInner()->Resolve(GetContext(), v8::Undefined(isolate()));
}
There was a problem hiding this comment.
I know, but don't we need that because we are creating the v8::Local<v8::Value> buffer before calling the util?
8d8fac3 to
a9ac096
Compare
3490bc6 to
398faec
Compare
|
Lets rerun the osx test suite, to rule out flakiness before merging. Thanks! |
398faec to
0ff629c
Compare
|
I re-ran the macOS CI and the same test failed 🤔 |
0ff629c to
a9fa543
Compare
| } else { | ||
| console.error('Error: Printing feature is disabled.') | ||
| return Promise.reject(new Error('Printing feature is disabled')) |
|
Release Notes Persisted
|
|
/trop run backport-to 5-0-x |
|
The backport process for this PR has been manually initiated, |
|
I was unable to backport this PR to "5-0-x" cleanly; |
Description of Change
This PR promisifies
webContents.printToPDF()/cc @codebytere
Checklist
npm testpassesRelease Notes
Notes: Converted
webContents.printToPDF()to return a Promise instead of taking a callback.