Closed Bug 489812 Opened 11 years ago Closed 11 years ago

[FIX] Suspending timeouts doesn't clear mFiringDepth

Categories

(Core :: DOM: Core & HTML, defect, P2)

x86
All
defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1

People

(Reporter: smaug, Assigned: smaug)

References

Details

(Keywords: fixed1.9.1, regression)

Attachments

(3 files, 1 obsolete file)

Attached patch proposed patch (obsolete) — Splinter Review
Passes chrome/browser-chrome/mochitest

Ginn, could you test this with your testcase?
Flags: blocking1.9.1?
Attachment #374273 - Flags: superreview?(jst)
Attachment #374273 - Flags: review?(bent.mozilla)
Priority: -- → P2
Blocks: 315286
Attachment #374273 - Attachment is obsolete: true
Attachment #374283 - Flags: superreview?(jst)
Attachment #374283 - Flags: review?(bent.mozilla)
Attachment #374273 - Flags: superreview?(jst)
Attachment #374273 - Flags: review?(bent.mozilla)
Comment on attachment 374283 [details] [diff] [review]
Remove some debug leftovers from the test.

Are you sure you want to move the IsFrozen check too? That was in place before we ever started messing with the sync xhr stuff so I'm worried it might cause some other hard-to-find regression. Having said that, though, I think that your change is correct. jst should weigh in.
(In reply to comment #3)
> (From update of attachment 374283 [details] [diff] [review])
> Are you sure you want to move the IsFrozen check too? 
As far as I see, that is the right thing to do... but sure, for 1.9.1 we can
make only the minimal change.
Attachment #374304 - Flags: superreview?(jst)
Attachment #374304 - Flags: review?(bent.mozilla)
Attachment #374304 - Flags: review?(bent.mozilla) → review+
It solved the problem of the website.

Thanks!
If I understand this right this can cause random timers to never fire, and that could break random sites. Blocking.
Flags: blocking1.9.1? → blocking1.9.1+
(In reply to comment #7)
> If I understand this right this can cause random timers to never fire, and that
> could break random sites.
Right.

Any chance for a sreview ;) The latter patch changes only sync-XHR case. The
first patch should fix also the case when page goes to bfcache and then back, but that is an *old* regression.
Attachment #374283 - Flags: superreview?(jst) → superreview+
Comment on attachment 374283 [details] [diff] [review]
Remove some debug leftovers from the test.

I think we do want this, and I'd even say we should take this for the branch as well.
Attachment #374304 - Flags: superreview?(jst) → superreview+
Comment on attachment 374304 [details] [diff] [review]
Moving only mTimeoutsSuspendDepth check

sr=jst for this one as well, but I still think we should take the other patch on the branch.
Comment on attachment 374304 [details] [diff] [review]
Moving only mTimeoutsSuspendDepth check

http://hg.mozilla.org/mozilla-central/rev/531f60d6cf88
I pushed this one. I think I need to write some tests before pushing the other one.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Ginn, could you verify (using a trunk build) that the bug is fixed.
I've confirmed it is fixed in this build.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090426 Minefield/3.6a1pre
Built from http://hg.mozilla.org/mozilla-central/rev/b552b1ef8aa0
Status: RESOLVED → VERIFIED
Attached patch 191 patchSplinter Review
Flags: in-testsuite+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.