###!!! ABORT: should already have refreshed style rule: 'ea->mStyleRuleRefreshTime == mPresContext->RefreshDriver()->MostRecentRefresh()', file layout/style/nsAnimationManager.cpp, line 801 This assertion is part of the new code from bug 435442, and the testcase uses MozAnimationName, so I guess that makes sense.

The underlying problem here is that the refresh driver was lying about it's most recent refresh time. This fixes it so that it only updates mMostRecentRefresh either (a) when it goes from not having observers (or callers of MostRecentRefresh()) to having some or (b) when it actually notifies its observers. Note that this inserts a notification in Thaw().

I'll land Jesse's crashtest in a separate cset (queued up in my tree already).

Comment on attachment 527620 [details] [diff] [review] patch The Notify() call in Thaw() worries me. Can consumers there deal with script running? Why is that call needed?

Well, Thaw() advances the most recent refresh time up to the present... which I tend to think it should do. Waiting until 15ms after Thaw() to update animation state seems a little silly. But I suppose running things inside Thaw() probably isn't the best idea. Maybe we should defer everything until after Thaw()? I would like to maintain the invariant that any observer that's been registered can call MostRecentRefresh() to get the time their WillRefresh() method was called; the current Thaw() code violates that by updating the result of MostRecentRefresh() without notifying observers. I also think just failing to update MostRecentRefresh() is also a bad idea, since if an animation starts between then and when the timer fires, it'll have its timing way off.

> Waiting until 15ms after Thaw() to update animation state seems a little silly. We don't wait 15ms. In Thaw() we immediately post an event to the event loop to call DoRefresh(), so we'll Notify(nsnull) as soon as that event is processed. I agree that the problems you describe in the last part of comment 5 need to be fixed. Maybe we should put a script blocker around the entire thawing process in docshell and run the Notify() call off a script runner? If so, we can take out the DoRefresh event.

For some reason I didn't notice that DoRefresh call right there. :-) In any case, I think the Thaw issue is mostly less serious -- how about fixing most of the problems here and worrying about that later, or at least in a distinct patch.

Jesse's test doesn't seem to work in the crashtest harness. (Is it the popup blocker?)

Sort of ... last I checked crash|reftests couldn't open windows at all.

Comment on attachment 527661 [details] [diff] [review] patch r=me; let's get a followup bug filed on Thaw(). I just looked at the callers, and PresShell:Thaw() can already trigger frame flushes (to ensure plugin instantiation), so we need to make sure it and its callers can handle arbitrary script running anyway. But I agree that a separate bug for it is good.

Filed bug 652030.