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

See attached test - loading the page should result in the inner frame scrolling to 500px. That's what happens in Chrome and Safari. In Firefox it doesn't. I think the reason is the same as what's happening in bug 1235899 - the smooth-scroll gets interrupted because the nsGfxScrollFrame is destroyed and re-created.

That being said, I would have expected my patch in bug 1235899 to fix this (with APZ enabled) but it doesn't seem to. It needs some more investigation.

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

This is not really perf, more correctness. Also I guess technically it's a regression from when the scroll-behavior stuff was turned on.

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

Bumping out of 48

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

This one turned out to be pretty straightforward: the frame reconstruction basically loses the destination of the smooth-scroll animation. The restored scroll position is whatever the scroll position was at the time the frame was destroyed. In the non-APZ case the AsyncScroll or AsyncSmoothMSDScroll scroll is destroyed during frame reconstruction, halting the animation. In the APZ case a new scroll position update is sent to the APZ which aborts the animation in the compositor and resets the position back to what the main thread thinks it was.

As per discussion with tnikkel last week, it's better to save/restore the destination of the animation - that way we'll end up scrolled to the right position, even if we don't do a smooth animation to it. The attached patch seems to work, and includes the test case converted to a mochitest.

Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea5795a031d1

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

The try push showed an intermittent failure in the test I wrote. I have to put this on the back burner for now to go fix other intermittent failures but I'll get back to it later.

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

Cleaner try push with modified test at https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c3c2719b3dc&group_state=expanded

	Timothy Nikkel (:tnikkel)
	Comment 7 • 8 years ago
mozreview-review

Comment on attachment 8786149 [details]
Bug 1247074 - When a compositor-based smooth scroll animation is in progress and the scrollframe is reconstructed, restore to the animation destination.

https://reviewboard.mozilla.org/r/75116/#review73870

	Pulsebot
	Comment 8 • 8 years ago

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f478daa490df
When a compositor-based smooth scroll animation is in progress and the scrollframe is reconstructed, restore to the animation destination. r=tnikkel

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 9 • 8 years ago
bugherder

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

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

Since this didn't appear to fix bug 1297419 (the reporter can still repro that one) it's probably not worth uplifting.