1031067 - [B2G][Browser]Scrolling into overscroll with two fingers causes the browser to freeze

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Andrew Stole [:astole]]

Attachment: logcat

Scrolling with two fingers in the browser window makes it to zoom in and out while scrolling which causes the browser to freeze. Pressing the home button and relaunching the browser causes it to no longer be frozen. On some occasions, pressing refresh unfroze the Browser as well as locking and unlocking the device but these did not work every time.

Repro Steps:
1) Update a Flame to BuildID: 20140626040205
2) Launch the Browser
3) Proceed to a webpage
4) Scroll using two fingers

Actual:
The Browser freezes

Expected:
The Browser does not freeze

2.1 Environmental Variables:
Device: Flame 2.1
BuildID: 20140626040205
Gaia: 87a7746568ac5708e828026160c0732ba252300f
Gecko: c43be7e4ec49
Version: 33.0a1
Firmware Version: v122

Repro frequency: 2/2, 100%
See attached: Video, logcat

Comment 1

[Andrew Stole [:astole]]

Attaching video and adding qawanted to test on other devices and builds.

User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Comment 2

[Joshua Mitchell (Inactive)]

Nomming as a blocker - freezing in a major feature (browser) sounds bad.

Comment 3

[Cody Roesch [:croesch]]

This bug repro's on: Flame 2.1 Master, Flame 2.0, OpenC 2.1 Master, OpenC 2.0

Actual Results: Website scrolling freezes when scrolling with 2 fingers for a short time.

Environmental Variables:
Device: Flame Master
Build ID: 20140626063403
Gaia: 87a7746568ac5708e828026160c0732ba252300f
Gecko: 4c9d8c885791
Version: 33.0a1 (Master)
Firmware Version: v122
------------------------------------------
Environmental Variables:
Device: Flame 2.0
Build ID: 20140626060203
Gaia: d815bfbb70d0f4a3757bb9eb93956a59a103d181
Gecko: cfa9446d0960
Version: 32.0a2 (2.0)
Firmware Version: v122
------------------------------------------
Environmental Variables:
Device: Open_C Master
Build ID: 20140626063403
Gaia: 87a7746568ac5708e828026160c0732ba252300f
Gecko: 4c9d8c885791
Version: 33.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
------------------------------------------
Environmental Variables:
Device: Open_C 2.0
Build ID: 20140626095753
Gaia: 34f4b94233a0e03ffbb3b84dc0ed1cc52df1df1a
Gecko: 0356c34ec908
Version: 32.0a2 (2.0)
Firmware Version: P821A10V1.0.0B06_LOG_DL

---------------------------------------------
---------------------------------------------

This bug does NOT repro on: Flame 1.4, OpenC 1.4, Buri 2.1 Master, Buri 2.0, Buri 1.4

Actual Result: Scrolling with two fingers on a webpage does not cause a freeze for the browser.

Environmental Variables:
Device: Flame 1.4
Build ID: 20140625000201
Gaia: c9416de14acf9e94ab006619cd2418c768422fcb
Gecko: cddf88f78632
Version: 30.0 (1.4)
Firmware Version: v122
-------------------------------------------
Environmental Variables:
Device: Buri Master
Build ID: 20140626063403
Gaia: 87a7746568ac5708e828026160c0732ba252300f
Gecko: 4c9d8c885791
Version: 33.0a1 (Master)
Firmware Version: v1.2device.cfg
------------------------------------------
Environmental Variables:
Device: Buri 2.0
Build ID: 20140626113947
Gaia: 34f4b94233a0e03ffbb3b84dc0ed1cc52df1df1a
Gecko: 8eff2a44f2b2
Version: 32.0a2 (2.0)
Firmware Version: v1.2device.cfg
-------------------------------------------
Environmental Variables:
Device: Buri 1.4
Build ID: 20140626114544
Gaia: 2f4acc16a99d40cf18d78fe51e14f1ea63b837c2
Gecko: 236ef7ae3260
Version: 30.0 (1.4)
Firmware Version: v1.2device.cfg
-------------------------------------------
Environmental Variables:
Device: Open_C 1.4
Build ID: 20140625000201
Gaia: c9416de14acf9e94ab006619cd2418c768422fcb
Gecko: cddf88f78632
Version: 30.0 (1.4)
Firmware Version: P821A10V1.0.0B06_LOG_DL

Comment 4

[Duane Dixon [:ddixon]]

B2G Inbound Regression Window

Last Working

Device: Flame Master
Build ID: 20140609154428
Gaia: d64172b2df8b406183e4d6de0cab2494c6dcf211
Gecko: 7fb68b04c53c
Version: 33.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

First Broken

Device: Flame Master
Build ID: 20140609155654
Gaia: 901646a3279c66daa7621bebb62641779d8ae22c
Gecko: 58cf55af0ec1
Version: 33.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Last Working Gaia and First Broken Gecko
Issue DOES NOT occur here.
Gaia: d64172b2df8b406183e4d6de0cab2494c6dcf211
Gecko: 58cf55af0ec1

Last Working Gecko and First Broken Gaia
Issue DOES occur here.
Gaia: 901646a3279c66daa7621bebb62641779d8ae22c
Gecko: 7fb68b04c53c

Gaia Pushlog:
https://github.com/mozilla-b2g/gaia/compare/d64172b2df8b406183e4d6de0cab2494c6dcf211...901646a3279c66daa7621bebb62641779d8ae22c

Possible Causes:

bug 982295 - Use new Browser API download method. r=benfrancis

bug 1020045 - Turn on APZ overscrolling by default. r=kats

Comment 5

[Joshua Mitchell (Inactive)]

Aus - one of the suspected causes from the push-log is yours, could I get you to take a look please?

Comment 6

[Jason Smith [:jsmith]]

(In reply to Joshua Mitchell from comment #5)
> Aus - one of the suspected causes from the push-log is yours, could I get
> you to take a look please?

bug 1020045 is more likely the cause of the regression, so I'm switching the needinfo here to Milan to find someone to look into this.

Comment 7

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

The problem here is that we can scroll initially with two fingers, before the finger span changes enough for it to be detected as a pinch. During this time we can go into an overscroll state. If the finger span then changes, it becomes a pinch, and we go into the pinch handling code. When we end the pinch handling code we never clear the overscroll.

Considering that right now you can't go into overscroll *after* you enter a pinch state, I think we should also disallow going into a pinch after entering an overscroll state. That is, the invariant "you cannot simultaneously be in a pinch state and overscrolled" should be enforced. We might be able to remove this invariant and provide a better overall solution in bug 1031443 but that would be too risky for uplifting to 2.0, I think.

Comment 8

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

Attachment: [Approach 1] Prevent going into a pinch when in overscroll

Comment 9

[Doug Sherk (:drs) (inactive)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #7)
> Considering that right now you can't go into overscroll *after* you enter a
> pinch state, I think we should also disallow going into a pinch after
> entering an overscroll state. That is, the invariant "you cannot
> simultaneously be in a pinch state and overscrolled" should be enforced. We
> might be able to remove this invariant and provide a better overall solution
> in bug 1031443 but that would be too risky for uplifting to 2.0, I think.

This seems really bad from a UX perspective. I could easily see users accidentally overscrolling slightly while setting up a pinch, and it's unexpected to not be able to pinch while overscrolled even if the user knows that they're overscrolled. I think ideally we would set up a fast, uninterruptible overscroll snapback animation which, when completed, enters the pinch state. But this is a UX decision, so needinfoing them. I'm not sure who owns this.

Comment 10

[Doug Sherk (:drs) (inactive)]

Also, note that for the purpose of comment 9, this isn't specific to the browser. This is a decision that will affect any APZ'd apps/content.

Comment 11

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

Gordon has been the UX person we've been working with for all the overscroll UX.

Comment 12

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

Comment on attachment 8449480 [details] [diff] [review]
[Approach 1] Prevent going into a pinch when in overscroll

Review of attachment 8449480 [details] [diff] [review]:
-----------------------------------------------------------------

Botond, not sure this scenario came up in your discussions with Gordon and if you know what the right answer is. If you know of a UX-acceptable solution (that's different from this patch) feel free to steal this bug.

Comment 13

[Botond Ballo [:botond]]

Comment on attachment 8449480 [details] [diff] [review]
[Approach 1] Prevent going into a pinch when in overscroll

Review of attachment 8449480 [details] [diff] [review]:
-----------------------------------------------------------------

This specific situation did not come up in my discussions with Gordon.

I wonder if we could simply clear overscroll at the end of a pinch instead.

Comment 14

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

(In reply to Botond Ballo [:botond] from comment #13)
> I wonder if we could simply clear overscroll at the end of a pinch instead.

Can you define "end of a pinch"?

Comment 15

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

(In reply to Doug Sherk (:drs) from comment #9)
> This seems really bad from a UX perspective. I could easily see users
> accidentally overscrolling slightly while setting up a pinch, and it's
> unexpected to not be able to pinch while overscrolled even if the user knows
> that they're overscrolled.

So this is a problem even now, if while "setting up a pinch" you put one finger down, scroll slightly into overscroll, and then put down the second finger to complete the setup. I don't think that there's a good way to get away from this in the current implementation.

Comment 16

[Botond Ballo [:botond]]

Attachment: [Approach 2] Clear overscroll at the end of a pinch

The attached patch clears overscroll at the end of a pinch, i.e. in AsyncPanZoomController::OnScaleEnd(). This avoids the bad UX Doug cautioned against in comment 9 while still solving the original problem of getting stuck in the overscrolled state.

Clearing the overscroll causes the scroll position to jump immediately, but I think this is fine as this scenario is an edge case anyways; if we think this is a problem, we could launch a snap-back animation instead.

Comment 17

[Botond Ballo [:botond]]

Comment on attachment 8449480 [details] [diff] [review]
[Approach 1] Prevent going into a pinch when in overscroll

Dropping review flag while we evaluate the approach in the other patch.

Comment 18

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

Comment on attachment 8452627 [details] [diff] [review]
[Approach 2] Clear overscroll at the end of a pinch

Review of attachment 8452627 [details] [diff] [review]:
-----------------------------------------------------------------

The only concern I could think of is that the overscroll might actually be somewhere else up the chain. I don't think that can happen at present because only a root APZC for a subtree will execute this code, and I don't think there's any case where we can hand off the overscroll across the layer subtree boundary. If that changes in the future then this might be a problem again.

Comment 19

[Botond Ballo [:botond]]

Try push: https://tbpl.mozilla.org/?tree=Try&rev=a1feca871f26

Comment 20

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #18)
> Comment on attachment 8452627 [details] [diff] [review]
> [Approach 2] Clear overscroll at the end of a pinch
> 
> Review of attachment 8452627 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The only concern I could think of is that the overscroll might actually be
> somewhere else up the chain. I don't think that can happen at present
> because only a root APZC for a subtree will execute this code, and I don't
> think there's any case where we can hand off the overscroll across the layer
> subtree boundary. If that changes in the future then this might be a problem
> again.

Can we assert/check/log when this assumption fails, maybe even with this bug number mentioned.  It would make it easier to resolve if we ever hit it.

Comment 21

[Botond Ballo [:botond]]

Attachment: [Approach 2] Clear overscroll at the end of a pinch

Added warning about handing off scroll across a layer tree boundary as per Milan's suggestion.

Comment 22

[Botond Ballo [:botond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/764a33b3eca8

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/764a33b3eca8

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/06dd0707ddbf

Comment 25

[Shally Li（leave from Mozilla）]

This issue has been successfully verified on Flame 2.1&2.0.
See attachment: verified_v2.1.mp4
Reproduce rate: 0/5

Actual Result: Scrolling with two fingers on a webpage does not cause a freeze for the browser.

Flame 2.1 build:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141201001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141201.034405
FW-Date         Mon Dec  1 03:44:15 EST 2014
Bootloader      L1TC00011880

Flame 2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141201000201
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141201.034308
FW-Date         Mon Dec  1 03:43:18 EST 2014
Bootloader      L1TC00011880

Comment 26

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1.MP4