If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Create a common RootLists structure to share the root list fields between PerThreadData and ContextFriendFields

RESOLVED FIXED in Firefox 42

Status

()

Core
JavaScript: GC
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: terrence, Assigned: terrence)

Tracking

(Blocks: 1 bug)

Trunk
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Created attachment 8630476 [details] [diff] [review]
split_out_root_list_structures-v0.diff

This seems slightly cleaner to me.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4db69179ff3e
Attachment #8630476 - Flags: review?(jcoppeard)
Comment on attachment 8630476 [details] [diff] [review]
split_out_root_list_structures-v0.diff

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

Looks good!

::: js/src/jspubtd.h
@@ +355,5 @@
>  template <> struct RootKind<jsid> : SpecificRootKind<jsid, THING_ROOT_ID> {};
>  template <> struct RootKind<JS::Value> : SpecificRootKind<JS::Value, THING_ROOT_VALUE> {};
>  
> +// Abstracts JS rooting mechanisms so that it can be mixed into and rooting
> +// mechanisms used with any of PerThreadData, JSRuntime, and JSContext

I'm not sure about the use of "mixed into" here.  It sounds like you're talking about mixins, but I don't think that's the case.

Also the comment needs full stop at the end.

@@ +363,5 @@
> +    JS::Rooted<void*>* stackRoots_[THING_ROOT_LIMIT];
> +    template <typename T> friend class JS::Rooted;
> +
> +    // Stack GC roots for stack-allocated AutoFooRooter classes.
> +    JS::AutoGCRooter*  autoGCRooters_;

Nit: Remove extra space between type and field name.
Attachment #8630476 - Flags: review?(jcoppeard) → review+

Comment 2

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff297c4e78a1
https://hg.mozilla.org/mozilla-central/rev/ff297c4e78a1
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.