Closed Bug 1065186 Opened 5 years ago Closed 5 years ago

Turn off COWs for Arrays

Categories

(Core :: XPConnect, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: bholley, Assigned: bholley)

References

Details

(Keywords: addon-compat)

Attachments

(2 files, 1 obsolete file)

My spidey senses tell me that, if we can successfully do bug 1065185, this may also be straightforward.
Note - If the landing of this bug breaks your code, you need to find a different way to expose Arrays to content. For DOM APIs, you should use WebIDL bindings. For extensions etc, you should use |new contentWindow.Array()| or |Cu.cloneInto(yourArray, contentWindow)|.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=08d1df66f8a1
CCing Matteo just in case since we had to resolve a breakage related to bug 1065185 in jetpack. I don't expect this one to cause any more issues but we'll see.
linux64 is green. Let's see about the rest of CI:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c7e99220354e
Attachment #8504083 - Flags: review?(gkrizsanits)
Blocks: 1081985
(In reply to Bobby Holley (:bholley) from comment #4)
> Created attachment 8504083 [details] [diff] [review]
> Part 1 - Fix up test suite to not rely on COWed Arrays. v1

Some changes of this patch seem to belong to Bug 1081990 no? Is it intentional that you add functions related tests here?
Attachment #8504084 - Flags: review?(gkrizsanits) → review+
Depends on: 1082450
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #6)
> Some changes of this patch seem to belong to Bug 1081990 no? Is it
> intentional that you add functions related tests here?

I switched them over to Function to test a different kind of COW (aside from Object), before I started feeling ambitious about nuking Function COWs as well. ;-)

Anyway, I decided to do bug 1082450 before landing this bug, so I'm holding off on the patches here for the moment. Hopefully this should all land very soon.
Comment on attachment 8504083 [details] [diff] [review]
Part 1 - Fix up test suite to not rely on COWed Arrays. v1

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

Alright let me know if you want me to review in it's current form then, but I guess then it might change soon, so I just cancel the review for now.
Attachment #8504083 - Flags: review?(gkrizsanits)
This is now rebased on top of bug 1082450. And doesn't do any shortsighted
Array => Function switches.
Attachment #8504083 - Attachment is obsolete: true
Attachment #8505285 - Flags: review?(gkrizsanits)
Attachment #8505285 - Flags: review?(gkrizsanits) → review+
https://hg.mozilla.org/mozilla-central/rev/d6aadffaf239
https://hg.mozilla.org/mozilla-central/rev/ce796ac8901b
Assignee: nobody → bobbyholley
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Keywords: addon-compat
You need to log in before you can comment on or make changes to this bug.