Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Description • 8 years ago

I can hit this assertion in a testcase that I wrote for bug 1235899. It happens because the ScrollbarActivity code doesn't deal with a frame reconstruction while the mNestedActivityCounter > 0. In other words, if the following sequence of events happens:

1) start scrolling, call ScrollbarActivity::ActivityStarted()
2) frame reconstruction of the nsGfxScrollFrame
3) finish scrolling, call ScrollbarActivity::ActivityStopped()

Step 1 sets the mNestedActivityCounter to 1. Step 2 resets it back to 0 because the ScrollbarActivity instance is destroyed and a new one is created. Step 3 tries to decrement mNestedActivityCounter but triggers this assertion first.

In bug 1235899 I plan to land a test that consistently reproduces this. I'm not entirely sure what the right fix is so I'm filing this bug to track the issue.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 1 • 8 years ago

oops, summary was wrong, left over from the bug I cloned.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 2 • 8 years ago

This is kind of annoying because it hits consistently when running test_frame_reconstruction.html on a debug build, I'm going to try to fix it.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 3 • 8 years ago

I keep running into this but every time I look into it I can't come up with a good solution - the right thing seems to be persist the state in the ScrollbarActivity to the nsPresState and restore it after the frame reconstruction but honestly that seems like overkill for a problem nobody has ever noticed in production code, as far as I can tell. So I think just gracefully handling this codepath is probably best.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 4 • 8 years ago

Review commit: https://reviewboard.mozilla.org/r/59266/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59266/

	Markus Stange [:mstange]
	Comment 5 • 8 years ago

Comment on attachment 8762772 [details]
Bug 1247050 - Avoid tripping an NS_ASSERTION that can legitimately fire.

https://reviewboard.mozilla.org/r/59266/#review56290

yeah ok, not particularly important to fix properly. We could do something like we do for LayerActivity where we briefly transfer the instance to a property on the frame's nsIContent, and then restore iti from there, but I don't think it's worth the effort.

	Pulsebot
	Comment 6 • 8 years ago

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae0c2fe7d0c3
Avoid tripping an NS_ASSERTION that can legitimately fire. r=mstange

	Carsten Book [:Tomcat]
	Comment 7 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/ae0c2fe7d0c3