Last Comment Bug 735781 - Make sure nsScreen don't call RemoveObserver when it's actually not observing
: Make sure nsScreen don't call RemoveObserver when it's actually not observing
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Mounir Lamouri (:mounir)
:
Mentors:
Depends on:
Blocks: 725951
  Show dependency treegraph
 
Reported: 2012-03-14 11:21 PDT by Mounir Lamouri (:mounir)
Modified: 2012-03-20 03:48 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (1.73 KB, patch)
2012-03-14 15:05 PDT, Mounir Lamouri (:mounir)
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2012-03-14 11:21:13 PDT
Follow-up from bug 725951: nsScreen was causing the crash because it can call twice RemoveObserver().
Comment 1 Mounir Lamouri (:mounir) 2012-03-14 15:05:26 PDT
Created attachment 605975 [details] [diff] [review]
Patch v1

As promised.
Comment 2 Justin Lebar (not reading bugmail) 2012-03-14 15:53:03 PDT
Did you forget to qref?  I don't see how this changes anything -- we'll still always call UnregisterScreenOrientationObserver.
Comment 3 Mounir Lamouri (:mounir) 2012-03-14 16:20:02 PDT
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 4 Justin Lebar (not reading bugmail) 2012-03-14 16:23:38 PDT
Comment on attachment 605975 [details] [diff] [review]
Patch v1

Oh, duh.
Comment 5 Phil Ringnalda (:philor, back in August) 2012-03-17 17:11:08 PDT
https://hg.mozilla.org/mozilla-central/rev/329c07609ca8
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2012-03-17 19:00:04 PDT
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
Comment 7 Marco Bonardo [::mak] (Away 6-20 Aug) 2012-03-20 03:48:54 PDT
https://hg.mozilla.org/mozilla-central/rev/d5713db68749

Note You need to log in before you can comment on or make changes to this bug.