799242 - Crash [@ nsRefreshDriver::Tick] with resizing and opening new tabs

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase (see comment 0)

Using a build that includes the patch for bug 731974,

1. Create a new profile 
2. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi
3. Replace prefs.js with the following:
  user_pref("browser.sessionstore.resume_from_crash", false);
  user_pref("toolkit.startup.max_resumed_crashes", -1);
  user_pref("dom.min_background_timeout_value", 4);
  user_pref("nglayout.debug.disable_xul_cache", true);
4. Run with the testcase on the command line

Result: Crash [@ nsRefreshDriver::Tick], usually on the second time the testcase opens a new tab.

At least on my laptop running Mac OS X 10.7, and a debug build from Tinderbox that was built from http://hg.mozilla.org/mozilla-central/rev/d9e032542831.  It seems to be horribly dependent on timing.

This might hold clues to topcrash bug 798872.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

So what's happening here is that we're poking all our drivers in the timer, and when one of them runs, it causes a later one to be removed:

0[acd5f0]: [fc54401] ticking drivers...
0[acd5f0]: >> TickDriver: 3ebdeb8 (jsnow: 1349798419095175)
0[acd5f0]: >> TickDriver: 82e78c8 (jsnow: 1349798419095175)
0[acd5f0]: [7f570c8] RemoveRefreshDriver 82e7730
0[acd5f0]: >> TickDriver: 82e7730 (jsnow: 1349798419095175)

but we try to poke it anyway since we took a copy of our list before we started.  We hold refs to everything, so I can only guess that the Tick itself is doing something busted.  I have an idea on how to fix it, though.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: don't Tick if we were disconnected

Simple fix -- just check if we were disconnected in Tick().  We have some helpers in DoTick/DoRefresh that are used for some internal test usages.  I also added the assertions to the main Tick call as well.  (This is on top of the patch in bug 731974; I'll fold them together when relanding, once I fix the winxp orange)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 669628 [details] [diff] [review]
don't Tick if we were disconnected

I don't think you can assert !mFrozen.  Another driver could have done history.back() on us and then spun the event loop until we are in fact frozen, no?  So I suspect we need to just test for that just like we test for mPresContext.

r=me with that

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Landed as part of bug 731974 relanding.  https://hg.mozilla.org/integration/mozilla-inbound/rev/5b727a94eebd

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Hm, forgot to resolve this; this should have been fixed when 731974 finally landed.  Is it still reproducible?

Comment 7

[Boris Zbarsky [:bzbarsky]]

I haven't tested; just wanted to make sure it didn't get lost...

Comment 8

[Jesse Ruderman]

Seems ok with the testcase in comment 0, and with the other testcases I had lying around.

Comment 9

[Alex Keybl [:akeybl]]

Tracking as fixed, QA please verify the testcase in FF20.

Comment 10

[Paul Silaghi, QA [:pauly]]

The Nightly debug builds from 2012-10-07 and 2012-10-08 seem to have some problems, they won't start on Mac. On Win, I couldn't reproduce the problem too.

Anyway, I don't see any crash on FF 20 2013-01-22-mozilla-aurora-debug using the STR in comment 0.

Comment 11

[Paul Silaghi, QA [:pauly]]

I see 16 crashes on FF 20b1 in the last week:
https://crash-stats.mozilla.com/report/list?signature=nsRefreshDriver%3A%3ATick%28__int64%2C+mozilla%3A%3ATimeStamp%29
Any thoughts?

Comment 12

[Paul Silaghi, QA [:pauly]]

More crashes appeared on 20 betas. Can anyone please take a look ?
https://crash-stats.mozilla.com/report/list?signature=nsRefreshDriver%3A%3ATick%28__int64%2C+mozilla%3A%3ATimeStamp%29