909877 - (gaia-apzc) [meta] Turn on APZC for all of gaia

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

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

Currently subframe scrolling in Gaia apps happens using BrowserElementPanning.js and occurs synchronously. This bug tracks all the work needed to switch that over to AsyncPanZoomController's async panning.

Comment 1

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

Attachment: WIP - turn on APZC on everything in gaia

For the record this is what I'm using as my current hack to "turn on APZC everywhere". It may not be correct but it's good enough to find out what bugs need fixing.

Comment 2

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

We should keep an eye on the FPS (e.g., bug 912134) as we convert the apps, as a sanity check that the performance is not regressing with these changes.

Comment 3

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

To turn this on by default, check 929728 which introduced the preference.  Not ready for it yet.

Comment 4

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

See: https://wiki.mozilla.org/B2G/Roadmap#Committed_1.3_Features

Comment 5

[Jason Smith [:jsmith]]

Spoke with nhirata in person about this - he's going to drive the QA testing on this.

Comment 6

[Rob MacDonald [:robmac]]

Is there any UX design required for this? Or is APZC mainly related to performance?

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Rob MacDonald [:robmac] from comment #6)
> Is there any UX design required for this? Or is APZC mainly related to
> performance?

Afaict APZC in this bug is mostly related to performance. 

I believe that there are some UX questions that can be discussed into relevant bugs. For example about overscroll indicators in bug 775469. Or about the panning heuristic, or event about when to display scrollbars and when not, etc...

IMHO the main consideration you should have here is that APZC will unify the scrolling behavior between Gaia Web Browser and Apps running in Firefox OS. The behavior will be derived from the current behavior of the browser app, so if there are things that UX does not like in it, feel free to open bugs to that blocks this one and let's discuss the details there.

Comment 9

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

(In reply to Rob MacDonald [:robmac] from comment #6)
> Is there any UX design required for this? Or is APZC mainly related to
> performance?

What Vivien said.

Comment 10

[Jan Jongboom [:janjongboom] (inactive)]

Is this gonna be in 1.3 after all?

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Jan Jongboom [:janjongboom] from comment #10)
> Is this gonna be in 1.3 after all?

The target is 1.3.

Comment 12

[Hubert Figuiere [:hub]]

(From FxOS Perf triage) I thought we didn't 1.3+ meta bug?

Comment 13

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

(In reply to Hubert Figuiere [:hub] from comment #12)
> (From FxOS Perf triage) I thought we didn't 1.3+ meta bug?

It doesn't hurt, and it makes it easier to make sure dependent bugs are triaged.  Sometimes we forget to put a 1.3? or 1.3+ on the blockers.

Comment 14

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

The patch doesn't apply to the current 1.3 build.  Need to talk to kats to figure out where to apply the patch.

Comment 15

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

Comment on attachment 796178 [details] [diff] [review]
WIP - turn on APZC on everything in gaia

The patch is obsolete now. There is an option in the gaia settings to enable APZ

Comment 16

[Preeti Raghunath(:Preeti)]

Vivien,

What are the next steps here?

Comment 17

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

Pull request pending merge at https://github.com/mozilla-b2g/gaia/pull/15122 to enable by default.

Comment 18

[Jason Smith [:jsmith]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #17)
> Pull request pending merge at https://github.com/mozilla-b2g/gaia/pull/15122
> to enable by default.

Are you looking for someone to review that patch?

Comment 19

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

There's not much to review. Milan asked me to make the change and fabrice said he would merge it once the travis run is green.

Comment 20

[[:fabrice] Fabrice Desré]

https://github.com/mozilla-b2g/gaia/commit/236e4f96cbfdaf2c90c12ebf61107dd274e7fd73

Comment 21

[Jason Smith [:jsmith]]

(In reply to Fabrice Desré [:fabrice] from comment #20)
> https://github.com/mozilla-b2g/gaia/commit/
> 236e4f96cbfdaf2c90c12ebf61107dd274e7fd73

Should we mark this fixed with this landing then?

Comment 23

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

I'm unsure of the gaia branching model but I believe this will need to be merged to 1.3 as well.

Comment 24

[Natalya Kot [:nkot]]

cannot scroll in FTU, see bug 958208

Comment 25

[John Ford [:jhford] CET/CEST Berlin Time]

This does not uplift cleanly:

$ git diff
* Unmerged path build/config/common-settings.json


$ git status
<snip>
Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)

	deleted by us:      build/config/common-settings.json

Comment 26

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

I made a rebased version on the latest mozillaorg/v1.3 code:

https://github.com/staktrace/gaia/commit/969448b53a9148e58880f64c7a12425ddef99dda

Please let me know if you require it in some other format.

Comment 27

[John Ford [:jhford] CET/CEST Berlin Time]

[v1.3 226aa05] Bug 909877 - Enable APZ for all of gaia. rs=milan

Comment 28

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Next step has already been done. Clearing.

Comment 29

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

generic apz test created to cover a lot of things to check.

Comment 31

[mac]

Depends on: 968145