[@ nsIPresShell::RemoveRefreshObserverInternal(nsARefreshObserver*, mozFlushType)]

RESOLVED FIXED in mozilla2.0b11

Status

()

Core
Layout
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: smaug, Assigned: surkov)

Tracking

({regression})

unspecified
mozilla2.0b11
x86
Windows 7
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Comment 1

8 years ago
Boris, is it unsafe to remove refresh observer when pagehide event is handled?
All those crashes are at offset 0x20 == 32 and in 32-bit builds.  And the code it crashes on is:

  GetPresContext()->RefreshDriver()->
    RemoveRefreshObserver(aObserver, aFlushType); 

32 is the offset of mRefreshDriver inside nsPresContext on 32-bit systems.  So GetPresContext() is returning null.  And yes, that could happen during pagehide...

nsIPresShell::RemoveRefreshObserverInternal should  null-check the prescontext.  Probably the add method should too.

Alexander, want to write the the patch?
blocking2.0: --- → ?
Keywords: regression
(Assignee)

Comment 3

8 years ago
Created attachment 508677 [details] [diff] [review]
patch

sure.

asking approval until bug is marked as blocking
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #508677 - Flags: review?(bzbarsky)
Attachment #508677 - Flags: approval2.0?
Comment on attachment 508677 [details] [diff] [review]
patch

I'd prefer a local for the prescontext.  With that change, r+a=me.
Attachment #508677 - Flags: review?(bzbarsky)
Attachment #508677 - Flags: review+
Attachment #508677 - Flags: approval2.0?
Attachment #508677 - Flags: approval2.0+
(Assignee)

Comment 5

8 years ago
Created attachment 508683 [details] [diff] [review]
patch3 [for landing]

with bz's comment addressed
(Assignee)

Comment 6

8 years ago
landed on 2.0 beta 11 - http://hg.mozilla.org/mozilla-central/rev/8b5cb26bbb10
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b11
(Assignee)

Updated

8 years ago
blocking2.0: ? → ---
You need to log in before you can comment on or make changes to this bug.