Stop passing a JSContext to WorkerRunnable::Dispatch

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

Trunk
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(8 attachments)

1.57 KB, patch
khuey
: review+
Details | Diff | Splinter Review
11.13 KB, patch
khuey
: review+
Details | Diff | Splinter Review
25.34 KB, patch
khuey
: review+
Details | Diff | Splinter Review
54.36 KB, patch
khuey
: review+
Details | Diff | Splinter Review
5.42 KB, patch
khuey
: review+
Details | Diff | Splinter Review
5.26 KB, patch
khuey
: review+
Details | Diff | Splinter Review
17.85 KB, patch
khuey
: review+
Details | Diff | Splinter Review
6.57 KB, patch
khuey
: review+
Details | Diff | Splinter Review
And some resulting cleanup.
Note that I did a try run after just part 3, with the asserts in place.  That's at https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8f37e9e39d2 and looking pretty green.  The try run for all the patches is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5f8d61ffe0f&selectedJob=17192229 and also looking pretty green.
Attachment #8723244 - Flags: review?(khuey)
Attachment #8723245 - Flags: review?(khuey)
Attachment #8723246 - Flags: review?(khuey)
Attachment #8723247 - Flags: review?(khuey)
Attachment #8723248 - Flags: review?(khuey)
Attachment #8723249 - Flags: review?(khuey)
Attachment #8723250 - Flags: review?(khuey)
Attachment #8723251 - Flags: review?(khuey)
Whiteboard: btpp-active
Comment on attachment 8723246 [details] [diff] [review]
part 3.  Remove the JSContext argument of WorkerRunnable::PostDispatch and its overrides

Review of attachment 8723246 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/WorkerRunnable.cpp
@@ +171,5 @@
>        break;
>  
>      case WorkerThreadUnchangedBusyCount:
>        aWorkerPrivate->AssertIsOnParentThread();
>        break;

You could coalesce these two cases now.
Attachment #8723246 - Flags: review?(khuey) → review+
Comment on attachment 8723247 [details] [diff] [review]
part 4.  Remove the JSContext argument of WorkerRunnable::Dispatch

Review of attachment 8723247 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/ServiceWorkerRegistration.cpp
@@ +1200,1 @@
>      }

WTF?
Attachment #8723247 - Flags: review?(khuey) → review+
Comment on attachment 8723248 [details] [diff] [review]
part 5.  Remove the JSContext argument from WorkerPrivateParent::Freeze/Thaw

Review of attachment 8723248 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/WorkerPrivate.h
@@ +303,5 @@
>    // as these are only used for globals going in and out of the bfcache.
> +  //
> +  // XXXbz: This is a bald-faced lie given the uses in RegisterSharedWorker and
> +  // CloseSharedWorkersForWindow, which pass null for aWindow to Thaw and Freeze
> +  // respectively.

This is worth a followup, something isn't right here.
Attachment #8723248 - Flags: review?(khuey) → review+
> You could coalesce these two cases now.

Done.

> WTF?

Took out the weirdness.

> This is worth a followup, something isn't right here.

Filed bug 1251722.
Blocks: 1252091
You need to log in before you can comment on or make changes to this bug.