Closed Bug 1185752 Opened 5 years ago Closed 4 years ago

Use TraceableHashMap to simplify CloneMemory

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(2 files)

The first patch is basically just adding a new typedef. The second patch removes the orphaned AutoObjectUnsignedHashMap.
Attachment #8636270 - Flags: review?(sphink)
Comment on attachment 8636270 [details] [diff] [review]
replace_CloneMemory_with_rooted-v0.diff

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

::: js/src/vm/StructuredClone.cpp
@@ +267,5 @@
>                                       void* cbClosure,
>                                       Value tVal)
>          : out(cx), objs(out.context()),
>            counts(out.context()), entries(out.context()),
> +          memory(out.context(), mozilla::Move(CloneMemory(out.context()))), callbacks(cb),

Admittedly, I'm pretty shaky on this stuff, but isn't that mozilla::Move redundant here?
Attachment #8636270 - Flags: review?(sphink) → review+
Comment on attachment 8636271 [details] [diff] [review]
remove_AutoObjectUnsigned32HashMap-v0.diff

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

\o/
Attachment #8636271 - Flags: review?(sphink) → review+
(In reply to Steve Fink [:sfink, :s:] from comment #2)
> 
> Admittedly, I'm pretty shaky on this stuff, but isn't that mozilla::Move
> redundant here?

For some reason both clang and gcc were insisting on selecting the deleted copy constructor without it. It doesn't reproduce when I retried now though, so I guess I there was something else wrong that I've fixed in the meantime.
https://hg.mozilla.org/mozilla-central/rev/7b01ff214a2d
https://hg.mozilla.org/mozilla-central/rev/09dfd750825a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.