983202 - Scrolling on CNN ceases to work, content does not repaint; URL bar scrolls off-screen

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Flaviu Cos]

Environment: 
Device: Google Nexus 10 (Android 4.4.2);
Build: Nightly 30.0a1 (2014-03-13);

Steps to reproduce:
1. Load Firefox with a clean profile;
2. Go to cnn.com;
3. Wait until the page is loaded and scroll the page up and down. 

Expected result:
The page successfully scrolls up and down.

Actual result:
The page hangs and only the url bar scrolls up and down.

Notes:
Please check the video: https://www.youtube.com/watch?v=3vzK5LQdSns

Comment 1

[Aaron Train [:aaronmt]]

I see this too on my Galaxy SIV Nightly (03/13). This is a major regression that needs a window.

Comment 2

[Aaron Train [:aaronmt]]

First round m-c

Last good revision: a62bde1d6efe (2014-02-13)
First bad revision: d275eebfae04 (2014-02-14)

Pushlog:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a62bde1d6efe&tochange=d275eebfae04

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Cwiiis, sounds like could be related to bug 982651

Comment 4

[Aaron Train [:aaronmt]]

Last good revision: 5d4ffa16f845
First bad revision: 2812fd3a3213
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5d4ffa16f845&tochange=2812fd3a3213

[e-mail]
Tue Feb 11 20:54:22 2014 +0000	2812fd3a3213	Bas Schouten — Bug 805406: Never draw directly to a window with Direct2D. r=jrmuizel

Beats me how Android is affected by that? Bas?

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to Aaron Train [:aaronmt] from comment #4)
> [e-mail]
> Tue Feb 11 20:54:22 2014 +0000	2812fd3a3213	Bas Schouten — Bug 805406: Never
> draw directly to a window with Direct2D. r=jrmuizel
> 
> Beats me how Android is affected by that? Bas?

That code isn't even compiled on android. Can you double check your regression window?

Comment 6

[Aaron Train [:aaronmt]]

Sorry was a mistake, in a new range I have

Last good revision: 8c7d4dddd104
First bad revision: 9ce7c3a70c13
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8c7d4dddd104&tochange=9ce7c3a70c13

[e-mail]
Tue Feb 11 04:58:44 2014 +0000	9ce7c3a70c13	Matt Woodrow — Bug 966679 - Follow-up to fix bustage on gcc 4.4, split Compose() into separate functions. CLOSED TREE

Comment 7

[Chris Lord [:cwiiis]]

n?mattwoodrow, going on comment #6.

Comment 8

[Aaron Train [:aaronmt]]

I am double checking the above too.

Comment 9

[Matt Woodrow (:mattwoodrow)]

That part is a fairly trivial rename to avoid a compiler error on b2g. I'd be surprised if it caused a regression :)

Comment 10

[Aaron Train [:aaronmt]]

Issue above was in attempting to reproduce before entire page-load which was throwing me off from 'good' and 'bad' builds.

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=74cbeb541d03&tochange=a523d5b55989

Will need bi-sect, Kevin or better yet, Graphics team take a look?

Comment 11

[Kartikaya Gupta (email:[e-mail])]

None of the changes in that range touch any of the files in gfx/

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Even the pref changes look benign. There's nothing in that range that I think would cause this problem.

Comment 13

[Chris Lord [:cwiiis]]

I'm with kats, none of those changes look like they have any relevance to something like this... I guess a profile would tell us what's going on, the compositor must be getting stuck if the url bar can still scroll but the page can't.

I can't promise I'll get the time to look at this by the end of tomorrow, but I'll try. If anyone has spare cycles, feel free to investigate.

Comment 15

[Aaron Train [:aaronmt]]

Noticed that a mere device rotation will aleve the symptoms when one is in this state on CNN

Comment 16

[Chris Lord [:cwiiis]]

We've been profiling, and there seems to be some weirdness with event/transaction handling when progressive rendering is enabled. Still looking into it. Explains somewhat why a rotation alleviates it though.

Comment 17

[Aaron Train [:aaronmt]]

I also noticed that this happens when one attempts to scroll when the top 'Get CNN on Android' related banner appears but is not fully loaded. At least for me. If I wait until *everything* is done loading, everything appears to work fine.

Comment 18

[Aaron Train [:aaronmt]]

Screen-record from my Nexus 5, today's Nightly build https://www.youtube.com/watch?v=t6z6jEI5JY8

Comment 19

[Brian Nicholson (:bnicholson)]

Trying the range from comment 10, I found that even the earliest cset still reproduces the bug. I tried again using TBPL builds, and here's the range I came up with: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8bb30e0e4ddd&tochange=edd724161ab4

Comment 20

[Brian Nicholson (:bnicholson)]

Looks like Aaron was only one changeset off back in comment 6. After a manual bisection, I can confirm that this was due to bug 966679.

The offending changeset: https://hg.mozilla.org/mozilla-central/rev/8c7d4dddd104

Comment 21

[Matt Woodrow (:mattwoodrow)]

(In reply to Chris Lord [:cwiiis] from comment #16)
> We've been profiling, and there seems to be some weirdness with
> event/transaction handling when progressive rendering is enabled. Still
> looking into it. Explains somewhat why a rotation alleviates it though.

Can you keep looking at this Chris?

The changeset doesn't sound like it should cause any issues, we're just marking a layer as mutated on the client side. I can't see how that could possibly affect APZC, unless it's somehow tripping over an existing bug.

Comment 22

[Chris Lord [:cwiiis]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #21)
> (In reply to Chris Lord [:cwiiis] from comment #16)
> > We've been profiling, and there seems to be some weirdness with
> > event/transaction handling when progressive rendering is enabled. Still
> > looking into it. Explains somewhat why a rotation alleviates it though.
> 
> Can you keep looking at this Chris?
> 
> The changeset doesn't sound like it should cause any issues, we're just
> marking a layer as mutated on the client side. I can't see how that could
> possibly affect APZC, unless it's somehow tripping over an existing bug.

I can confirm that rolling back all the patches in bug 966679 does fix this, and looking at them together, it does appear like they could affect repeated transactions. Seeing as you probably have a lot on your plate, I'll look at this, but it's worth noting that a backout does fix this.

Next I'll see if the identified hunk does indeed fix it alone, and why.

Comment 23

[Chris Lord [:cwiiis]]

Attachment: Don't call Mutated more than necessary

This fixes the issue for me, and I think this is why: The excess calls to Mutated were flooding IPC, especially when there are complex regions involved (which there always are with progressive updates) and the compositor was getting held up dealing with them (this is supported somewhat by the profiles we took and gdb).

I think this highlights a possible issue here though that progressive updates makes layer transactions quite a bit more expensive. But that's another issue for another day...

Comment 24

[Chris Lord [:cwiiis]]

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/46c389bf27d9

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/46c389bf27d9