Mayank Bansal Reporter
	Description • 9 years ago

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150512030215

Steps to reproduce:

1. Create a new profile. (enable e10s)
2. set layers.async-pan-zoom.enabled  = true
3. Go to http://www.snapdeal.com/product/hp-15r205tu-notebook-k8u05pa-5th/683139390240

4. Scroll down.  May have to let the comments also load.


Actual results:

the page elements on the bottom of the screen shift from right-left.


Expected results:

not so.

	Markus Stange [:mstange]
	Comment 2 • 9 years ago

It looks like all position:fixed elements on the page briefly forget about the vertical scrollbar and jump to the right by the scrollbar width.

	Alice0775 White
	Comment 3 • 9 years ago

I can repro on Windows7

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

I can repro on Linux also. Increasing the apz repaint interval prefs makes it easier to repro.

	Botond Ballo [:botond] Assignee
	Comment 5 • 9 years ago

I'll note that the following message appears repeatedly on the console while browsing this page:

###!!! ASSERTION: 3D transform found between fixedpos content and its viewport (should never happen): 'Error', file /home/botond/dev/mozilla/central/layout/base/nsLayoutUtils.cpp, line 1726

Not sure yet if it's relevant.

	Botond Ballo [:botond] Assignee
	Comment 6 • 9 years ago

(In reply to Botond Ballo [:botond] from comment #5)
> I'll note that the following message appears repeatedly on the console while
> browsing this page:
> 
> ###!!! ASSERTION: 3D transform found between fixedpos content and its
> viewport (should never happen): 'Error', file
> /home/botond/dev/mozilla/central/layout/base/nsLayoutUtils.cpp, line 1726
> 
> Not sure yet if it's relevant.

I think this is unrelated, because the layer which has the 3D transform is not visible, and the layers which are jumping around do not have 3D transforms.

	Botond Ballo [:botond] Assignee
	Comment 9 • 9 years ago

The fixed-position items appear to be incorrectly positioned in the stable state, and then temporarily jump to their correct position during scrolling.

However, the adjustment is not being done in the compositor - the transform Layout sets on the layers is changing.

I have attached display list and layer dumps for both the correct and incorrect positioning. One of the layers which is jumping is 0x5cfda30 (client-side address) - its transform has x=1866 in the correct case and x=1879 in the incorrect case.

	Botond Ballo [:botond] Assignee
	Comment 10 • 9 years ago

I debugged this further with Timothy's help, and we determined that Layout is putting the position:fixed frame in the wrong spot. I'm continuing to investigate.

	Botond Ballo [:botond] Assignee
	Comment 11 • 9 years ago

The position of the fixed frame is in turn determined by the SPC-SPS, whose width is changing between 1525 (good frames) and 1536 (bad frames).

	Botond Ballo [:botond] Assignee
	Comment 12 • 9 years ago
diagnosis

The bad value of the SPC-SPS, which fails to exclude the size of the scrollbars, is coming from TabChild::HandlePossibleViewportSize()'s call to nsLayoutUtils::SetScrollPositionClampingScrollPortSize(). We pass in the composition bounds, which come from TabChild::mInnerSize, without anyone subtracting the scrollbar sizes. This is in contrast to other places where we calculate the composition bounds, where we do exclude the scrollbars.

	Botond Ballo [:botond] Assignee
	Comment 13 • 9 years ago

/r/9269 - Bug 1164406 - Exclude the scrollbars from the composition bounds in TabChild::HandleCalculateViewportChange() when appropriate. r=kats

Pull down this commit:

hg pull -r fda8530860ddc4b5ed9214b93deaf3ef1d2bf5e4 https://reviewboard-hg.mozilla.org/gecko/

	Botond Ballo [:botond] Assignee
	Comment 14 • 9 years ago

The attached patch fixes the problem for me.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 15 • 9 years ago

https://reviewboard.mozilla.org/r/9269/#review7953

::: dom/ipc/TabChild.cpp:346
(Diff revision 1)
> +    if (nsIScrollableFrame* rootScrollableFrame = shell->GetRootScrollFrameAsScrollable()) {

Can we reuse nsLayoutUtils::DeflateScrollbarAreaFromCompositionBoundsFor here? We can modify that function a little to return the nsMargin instead of a bool to encapsulate more of the code and reduce the need to get the nsIScrollableFrame at the callsite.

	Botond Ballo [:botond] Assignee
	Comment 16 • 9 years ago

Comment on attachment 8609497 [details]
MozReview Request: bz://1164406/botond

/r/9275 - Bug 1164406 - Expose scrollbar-area deflation in nsLayoutUtils. r=kats
/r/9269 - Bug 1164406 - Exclude the scrollbars from the composition bounds in TabChild::HandleCalculateViewportChange() when appropriate. r=kats

Pull down these commits:

hg pull -r 5e63b044025d339a6899f0ecb6b64d79eec5641d https://reviewboard-hg.mozilla.org/gecko/

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 17 • 9 years ago

https://reviewboard.mozilla.org/r/9269/#review8001

::: dom/ipc/TabChild.cpp:347
(Diff revision 2)
> +      nsMargin scrollbarsAppUnits = nsLayoutUtils::ScrollbarAreaToExcludeFromCompositionBoundsFor(rootScrollFrame);

You can reduce one level of indentation by just passing shell->GetRootScrollFrame() directly to the ScrollbarArea function. It will return 0 if passed a nullptr

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 18 • 9 years ago

Comment on attachment 8609497 [details]
MozReview Request: bz://1164406/botond

https://reviewboard.mozilla.org/r/9267/#review8003

Ship It!

	Botond Ballo [:botond] Assignee
	Comment 19 • 9 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #17)
> You can reduce one level of indentation by just passing
> shell->GetRootScrollFrame() directly to the ScrollbarArea function. It will
> return 0 if passed a nullptr

Updated locally. 

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

	Pulsebot
	Comment 20 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/d37b2f90f5af
https://hg.mozilla.org/integration/mozilla-inbound/rev/256f3225ca4e

	Phil Ringnalda (:philor)
	Comment 21 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/d37b2f90f5af
https://hg.mozilla.org/mozilla-central/rev/256f3225ca4e