Closed Bug 1252091 Opened 6 years ago Closed 6 years ago

Add/RemoveFeature don't need a JSContext argument

Categories

(Core :: DOM: Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

(Whiteboard: btpp-active)

Attachments

(1 file)

Now that ModifyBusyCountFromWorker doesn't need one...
Blocks: 1252123
Blocks: 1252127
Blocks: 1252130
Comment on attachment 8724781 [details] [diff] [review]
Add/RemoveFeature don't need a JSContext argument

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

::: dom/notification/Notification.cpp
@@ +2509,5 @@
>    MOZ_ASSERT(mWorkerPrivate);
>    mWorkerPrivate->AssertIsOnWorkerThread();
>    MOZ_ASSERT(!mFeature);
>    mFeature = MakeUnique<NotificationFeature>(this);
> +  bool added = mWorkerPrivate->AddFeature(mFeature.get());

is the .get() really needed here?  Seems odd.
Attachment #8724781 - Flags: review?(khuey) → review+
Whiteboard: btpp-active
> is the .get() really needed here?

Yes, because mFeature is UniquePtr.
https://hg.mozilla.org/mozilla-central/rev/954c9e9408fd
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.