Make sure nsScreen don't call RemoveObserver when it's actually not observing

RESOLVED FIXED in mozilla14

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

1.73 KB, patch
Justin Lebar (not reading bugmail)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Follow-up from bug 725951: nsScreen was causing the crash because it can call twice RemoveObserver().
(Assignee)

Comment 1

5 years ago
Created attachment 605975 [details] [diff] [review]
Patch v1

As promised.
Assignee: nobody → mounir
Status: NEW → ASSIGNED
Attachment #605975 - Flags: review?(justin.lebar+bug)
Did you forget to qref?  I don't see how this changes anything -- we'll still always call UnregisterScreenOrientationObserver.
(Assignee)

Comment 3

5 years ago
The issue was that Invalidate() was called then mScreen was set to nsnull so mScreen was finally deleted and the dtor called so Invalidate() was called again. This should fix the issue.
Comment on attachment 605975 [details] [diff] [review]
Patch v1

Oh, duh.
Attachment #605975 - Flags: review?(justin.lebar+bug) → review+
https://hg.mozilla.org/mozilla-central/rev/329c07609ca8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
I backed this out along with all the other patches in the initial push. Something in the push regressed Ts on Android by 20% and I don't know which patch is to blame:

https://hg.mozilla.org/mozilla-central/rev/e94edfdb1f5b

Regression Ts increase 20.6% on Android 2.2 Mozilla-Inbound
--------------------------------------------------------------
    Previous: avg 2653.656 stddev 87.856 of 30 runs up to revision 1239bd0779a6
    New     : avg 3201.178 stddev 110.202 of 5 runs since revision 0d61a0d8dba4
    Change  : +547.522 (20.6% / z=6.232)
    Graph   : http://mzl.la/zD3EWy
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/d5713db68749
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.