Closed
Bug 1282366
Opened 8 years ago
Closed 8 years ago
Improve WorkerHolder use in Runnables
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: baku, Assigned: baku)
References
Details
Attachments
(1 file)
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•8 years ago
|
||
There are a couple of runnables (in Console and in BindingUtils) that use WorkerHolder just to keep the worker alive.
With this patch I want to unify these runnables implementing a common base class and use ModifyBusyCount instead using WorkerHolder.
This is a good first step to make WorkerHolder a thread-only class so that we can add assertions in the CTOR and in the DTOR.
Attachment #8765362 -
Flags: review?(khuey)
Comment on attachment 8765362 [details] [diff] [review]
holder.patch
Review of attachment 8765362 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/indexedDB/ActorsChild.cpp
@@ +927,5 @@
> + if (NS_WARN_IF(!mWorkerPrivate->ModifyBusyCountFromWorker(true))) {
> + return false;
> + }
> +
> + if (NS_WARN_IF(NS_FAILED(NS_DispatchToMainThread(this)))) {
here too
::: dom/workers/WorkerRunnable.cpp
@@ +721,5 @@
> + RunBackOnWorkerThread();
> + return false;
> + }
> +
> + if (NS_WARN_IF(NS_FAILED(NS_DispatchToMainThread(this)))) {
NS_DispatchToMainThread can't fail until after xpcom-shutdown-threads, at which point the worker has to be gone. So you can MOZ_ALWAYS_SUCCEEDS this.
::: dom/workers/WorkerRunnable.h
@@ +410,5 @@
>
> +// This runnable is an helper class for dispatching something from a worker
> +// thread to the main-thread and back to the worker-thread. During this
> +// operation, this class will keep the worker alive.
> +class WorkerToMainThreadRunnable : public Runnable
I have to admit that I don't care for this name, but I can't think of anything much better. WorkerProxyToMainThreadRunnable? WorkerPingPongToMainThreadRunnable? :P
I think the name needs to indicate that it comes back ...
Attachment #8765362 -
Flags: review?(khuey) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc14db264521
Improve WorkerHolder use in Runnables, r=khuey
Comment 4•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•