Closed Bug 1026860 Opened 7 years ago Closed 7 years ago

Remove mThisObjects map from mozJSComponentLoader

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla33
Tracking Flags:
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
b2g-v1.4 --- wontfix
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(1 file)

Remove mThisObjects and use the function parent instead. v1
13.90 KB, patch
khuey
: review+
luke
: review+
lmandel
: approval-mozilla-aurora+
lmandel
: approval-mozilla-b2g30-
Details | Diff | Splinter Review 
Kyle added this in bug 810987, and it has caused (bug 980537) us (bug 950653) endless (bug 1023899) pain. It also makes it impossible/unsafe to unload modules in b2g (see bug 1004295). And it isn't GGC-safe (bug 983399).

I described the basic setup in bug 980537 comment 28. Digging in a bit more, it seems like we could instead just check if the function is parented to a FakeBackstagePass, and if so, use that as the target.

I'm going to prototype this and see what breaks.
Attachment #8442109 - Flags: review?(luke)
Attachment #8442109 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/1ffd2122f120
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Duplicate of this bug: 983399
Comment on attachment 8442109 [details] [diff] [review]
Remove mThisObjects and use the function parent instead. v1

This bug seems to have fixed the very serious b2g stability issues in bug 950653. It also gives us a _much_ simpler and safer setup. I recommend we take it for b2g 1.4 and 2.0.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Unknown
User impact if declined: Crashes
Testing completed: Landed on m-c last week, appears to fix the crashes.
Risk to taking this patch (and alternatives if risky): Surprisingly, very low-risk.
String or UUID changes made by this patch: None
Attachment #8442109 - Flags: approval-mozilla-b2g30?
Attachment #8442109 - Flags: approval-mozilla-aurora?
Comment on attachment 8442109 [details] [diff] [review]
Remove mThisObjects and use the function parent instead. v1

Aurora+

b2g30- Given that the branch stability is currently meeting our partner's expectations, we are not taking any additional speculative patches at this point. (I use the term speculative based on the comments in bug 950653.)
Attachment #8442109 - Flags: approval-mozilla-b2g30?
Attachment #8442109 - Flags: approval-mozilla-b2g30-
Attachment #8442109 - Flags: approval-mozilla-aurora?
Attachment #8442109 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.