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

feat: add webContents.getPrintersAsync() #31023

Merged

Conversation

RaisinTen
Copy link
Contributor

@RaisinTen RaisinTen commented Sep 20, 2021

Description of Change

This deprecates the synchronous and blocking webContents.getPrinters()
function and introduces webContents.getPrintersAsync(), which is
asynchronous and non-blocking.

Signed-off-by: Darshan Sen darshan.sen@postman.com

cc @deepak1556 @nornagon

Checklist

Release Notes

Notes: Deprecates webContents.getPrinters() and introduces webContents.getPrintersAsync()

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 20, 2021
@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch from 77dd7ce to b3e2ea2 Compare September 21, 2021 15:31
@RaisinTen RaisinTen changed the title feat: add webContents.getPrintersAsync() feat: add async non-blocking variation of webContents.getPrinters() Sep 21, 2021
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 27, 2021
@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch 2 times, most recently from 3177381 to 4d785ae Compare October 9, 2021 08:31
@RaisinTen RaisinTen changed the title feat: add async non-blocking variation of webContents.getPrinters() feat: add webContents.getPrintersAsync() Oct 9, 2021
@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch from 4d785ae to deaf392 Compare October 9, 2021 08:34
@RaisinTen RaisinTen requested a review from zcbenz October 9, 2021 08:40
@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch from deaf392 to 297d8a9 Compare October 11, 2021 12:26
@RaisinTen RaisinTen requested a review from zcbenz October 11, 2021 12:27
@zcbenz zcbenz added no-backport semver/minor backwards-compatible functionality labels Oct 12, 2021
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, left some additional reviews.

shell/browser/api/electron_api_printing.cc Outdated Show resolved Hide resolved
shell/browser/api/electron_api_printing.cc Outdated Show resolved Hide resolved
@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch 2 times, most recently from eabe270 to e08e1b6 Compare October 13, 2021 05:23
@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch 2 times, most recently from 9877715 to d2b04e4 Compare October 13, 2021 05:44
@zcbenz
Copy link
Member

zcbenz commented Oct 15, 2021

It is crashing on Windows:

[7448:1013/065958.767:FATAL:thread_restrictions.cc(104)] Check failed: !*GetBlockingDisallowedTls(). Function marked as blocking was called from a scope that disallows blocking! If this task is running inside the ThreadPool, it needs to have MayBlock() in its TaskTraits. Otherwise, consider making this blocking work asynchronous or, as a last resort, you may use ScopedAllowBlocking (see its documentation for best practices).
46771g_blocking_disallowed 1 set by
46772Backtrace:
46773	base::debug::CollectStackTrace [0x00007FF670C56DE2+18] (o:\base\debug\stack_trace_win.cc:303)
46774	base::debug::StackTrace::StackTrace [0x00007FF670BAE152+18] (o:\base\debug\stack_trace.cc:197)
46775	base::ThreadRestrictions::SetIOAllowed [0x00007FF670C44402+82] (o:\base\threading\thread_restrictions.cc:224)
46776	base::internal::TaskTracker::RunTask [0x00007FF6726036C7+167] (o:\base\task\thread_pool\task_tracker.cc:462)
46777	base::internal::TaskTracker::RunAndPopNextTask [0x00007FF672603401+545] (o:\base\task\thread_pool\task_tracker.cc:433)
46778	base::internal::WorkerThread::RunWorker [0x00007FF6735DF084+964] (o:\base\task\thread_pool\worker_thread.cc:377)
46779	base::internal::WorkerThread::RunPooledWorker [0x00007FF6735DEB98+24] (o:\base\task\thread_pool\worker_thread.cc:267)
46780	base::`anonymous namespace'::ThreadFunc [0x00007FF670C6C88A+266] (o:\base\threading\platform_thread_win.cc:114)
46781	BaseThreadInitThunk [0x00007FF8C9EF7974+20]
46782	RtlUserThreadStart [0x00007FF8CA88A0B1+33]
46783
46784Backtrace:
46785	base::debug::CollectStackTrace [0x00007FF670C56DE2+18] (o:\base\debug\stack_trace_win.cc:303)
46786	base::debug::StackTrace::StackTrace [0x00007FF670BAE152+18] (o:\base\debug\stack_trace.cc:197)
46787	logging::LogMessage::~LogMessage [0x00007FF670BC42EE+190] (o:\base\logging.cc:591)
46788	logging::LogMessage::~LogMessage [0x00007FF670BC5470+16] (o:\base\logging.cc:584)
46789	base::internal::AssertBlockingAllowed [0x00007FF670C4363B+267] (o:\base\threading\thread_restrictions.cc:111)
46790	base::ScopedBlockingCall::ScopedBlockingCall [0x00007FF670C3ED2A+138] (o:\base\threading\scoped_blocking_call.cc:45)
46791	printing::PrintBackendWin::GetDefaultPrinterName [0x00007FF67228A119+169] (o:\printing\backend\print_backend_win.cc:260)
46792	printing::PrintBackendWin::EnumeratePrinters [0x00007FF672289CDE+270] (o:\printing\backend\print_backend_win.cc:242)
46793	base::internal::Invoker<base::internal::BindState<`lambda at ../../electron/shell/browser/api/electron_api_printing.cc:69:33'>,std::__1::vector<printing::PrinterBasicInfo,std::__1::allocator<printing::PrinterBasicInfo> > ()>::RunOnce [0x00007FF66D60056F+111] (o:\base\bind_internal.h:690)
46794	base::OnceCallback<absl::optional<base::FilePath> ()>::Run [0x00007FF66D5D5C51+59] (o:\base\callback.h:100)
46795	base::internal::ReturnAsParamAdapter<std::__1::vector<printing::PrinterBasicInfo,std::__1::allocator<printing::PrinterBasicInfo> > > [0x00007FF66D600975+37] (o:\base\post_task_and_reply_with_result_internal.h:22)
46796	base::internal::Invoker<base::internal::BindState<void (*)(base::OnceCallback<absl::optional<base::FilePath> ()>, std::__1::unique_ptr<absl::optional<base::FilePath>,std::__1::default_delete<absl::optional<base::FilePath> > > *),base::OnceCallback<absl::o [0x00007FF66D5D59A5+53] (o:\base\bind_internal.h:690)
46797	base::`anonymous namespace'::PostTaskAndReplyRelay::RunTaskAndPostReply [0x00007FF67177B2E2+178] (o:\base\threading\post_task_and_reply_impl.cc:100)
46798	base::internal::Invoker<base::internal::BindState<void (*)(base::(anonymous namespace)::PostTaskAndReplyRelay),base::(anonymous namespace)::PostTaskAndReplyRelay>,void ()>::RunOnce [0x00007FF67177B784+84] (o:\base\bind_internal.h:690)
46799	base::TaskAnnotator::RunTask [0x00007FF670C1E0B9+457] (o:\base\task\common\task_annotator.cc:178)
46800	base::internal::TaskTracker::RunSkipOnShutdown [0x00007FF672604287+23] (o:\base\task\thread_pool\task_tracker.cc:664)
46801	base::internal::TaskTracker::RunTask [0x00007FF672603A89+1129] (o:\base\task\thread_pool\task_tracker.cc:528)
46802	base::internal::TaskTracker::RunAndPopNextTask [0x00007FF672603401+545] (o:\base\task\thread_pool\task_tracker.cc:433)
46803	base::internal::WorkerThread::RunWorker [0x00007FF6735DF084+964] (o:\base\task\thread_pool\worker_thread.cc:377)
46804	base::internal::WorkerThread::RunPooledWorker [0x00007FF6735DEB98+24] (o:\base\task\thread_pool\worker_thread.cc:267)
46805	base::`anonymous namespace'::ThreadFunc [0x00007FF670C6C88A+266] (o:\base\threading\platform_thread_win.cc:114)
46806	BaseThreadInitThunk [0x00007FF8C9EF7974+20]
46807	RtlUserThreadStart [0x00007FF8CA88A0B1+33]
46808Task trace:
46809Backtrace:
46810	electron::api::GetPrinterListAsync [0x00007FF66D5FF9F3+227] (o:\electron\shell\browser\api\electron_api_printing.cc:68)
46811	IPC::`anonymous namespace'::ChannelAssociatedGroupController::Accept [0x00007FF670F109AA+1546] (o:\ipc\ipc_mojo_bootstrap.cc:937)
46812	mojo::SimpleWatcher::Context::Notify [0x00007FF670E2A551+347] (o:\mojo\public\cpp\system\simple_watcher.cc:99)
46813IPC message handler context: 0xCA602648
46814Crash keys:
46815  "total-discardable-memory-allocated" = "0"
46816  "osarch" = "x86_64"
46817  "pid" = "7448"
46818  "ptype" = "browser"
46819  "chrome-trace-id" = "17021947898329446379"
46820  "ui_scheduler_async_stack" = "0x7FF66D70E8DF 0x0"
46821  "io_scheduler_async_stack" = "0x7FF66FD9394D 0x7FF66D630CF2"
46822  "platform" = "win32"
46823  "process_type" = "browser"
46824
46825✗ Electron tests failed with code 0x80000003.
46826error Command failed with exit code 1.
46827

@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch 2 times, most recently from 9ebf849 to 1861134 Compare October 15, 2021 11:12
@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch from 1861134 to 5063ba8 Compare October 15, 2021 11:34
This deprecates the synchronous and blocking `webContents.getPrinters()`
function and introduces `webContents.getPrintersAsync()`, which is
asynchronous and non-blocking.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
@RaisinTen RaisinTen force-pushed the feat/add-webContents.getPrintersAsync() branch from 5063ba8 to 1fe9f19 Compare October 15, 2021 12:55
@RaisinTen
Copy link
Contributor Author

@zcbenz the crash has been fixed.

Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API looks good to me.

@zcbenz zcbenz changed the title feat: add webContents.getPrintersAsync() feat: add webContents.getPrintersAsync() fix ci Oct 18, 2021
@zcbenz zcbenz changed the title feat: add webContents.getPrintersAsync() fix ci feat: add webContents.getPrintersAsync() Oct 18, 2021
@jkleinsc
Copy link
Contributor

API LGTM

@jkleinsc
Copy link
Contributor

Merging as CI failure is known flake already resolved.

@jkleinsc jkleinsc merged commit 8f51d3e into electron:main Oct 25, 2021
@release-clerk
Copy link

release-clerk bot commented Oct 25, 2021

Release Notes Persisted

Deprecates webContents.getPrinters() and introduces webContents.getPrintersAsync()

@RaisinTen RaisinTen deleted the feat/add-webContents.getPrintersAsync() branch October 26, 2021 05:08
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 27, 2021
This deprecates the synchronous and blocking `webContents.getPrinters()`
function and introduces `webContents.getPrintersAsync()`, which is
asynchronous and non-blocking.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 27, 2021
This deprecates the synchronous and blocking `webContents.getPrinters()`
function and introduces `webContents.getPrintersAsync()`, which is
asynchronous and non-blocking.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
This deprecates the synchronous and blocking `webContents.getPrinters()`
function and introduces `webContents.getPrintersAsync()`, which is
asynchronous and non-blocking.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
This deprecates the synchronous and blocking `webContents.getPrinters()`
function and introduces `webContents.getPrintersAsync()`, which is
asynchronous and non-blocking.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
This deprecates the synchronous and blocking `webContents.getPrinters()`
function and introduces `webContents.getPrintersAsync()`, which is
asynchronous and non-blocking.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
This deprecates the synchronous and blocking `webContents.getPrinters()`
function and introduces `webContents.getPrintersAsync()`, which is
asynchronous and non-blocking.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants