Bug 1178298 (all-aboard-apz)

Let APZ on desktop ride the train

RESOLVED FIXED in Firefox 46

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

({feature})

Firefox Tracking Flags

(firefox46 fixed, relnote-firefox 46+)

Details

(Whiteboard: [gfx-noted][e10s-45-uplift])

Attachments

(1 attachment)

I plan on enabling APZ on nightly only soon. Once that happens this bug will track the things that need to be fixed before we will let it ride the train to Aurora.
Alias: all-aboard-apz
Depends on: 990974
Depends on: 1157708
Depends on: 1166301
Depends on: 1157834
Depends on: 1141884
Depends on: 1167773
Depends on: 1168487
Depends on: 1012752
Depends on: 1122916
Depends on: 1148581
Depends on: 1169957
Whiteboard: [gfx-noted]
Depends on: 1178745
Depends on: 1178382
Depends on: 1181135
Depends on: 1184343
Depends on: 1150941
Depends on: 1187373
Depends on: 1187616
Depends on: 1187804

Updated

4 years ago
Depends on: 1187990
Depends on: 1187792
No longer depends on: 1167773
Depends on: 1188064
No longer depends on: 1168487
Depends on: 1188602
Depends on: 1189159
Depends on: 1189746
Depends on: 1188430
Depends on: 1188624
Depends on: 1160540

Updated

4 years ago
Depends on: 1189972
Depends on: 1190105

Updated

4 years ago
Depends on: 1190112
Depends on: 1190936
Depends on: 1189837
Depends on: 1189565
No longer depends on: 1157834
Depends on: 1189443
No longer depends on: 1012752
Depends on: 1192419
Depends on: 1193781
Depends on: 1192919
Depends on: 1194851
Depends on: 1195400
Depends on: 1195526
No longer depends on: 1195526
Depends on: 1197312
Depends on: 1197654
Depends on: 1200158
Depends on: 1200778
Depends on: 1201192
Depends on: apz-parallax
Depends on: 1201889
Depends on: 1201508
Depends on: 1193969
No longer depends on: apz-parallax
Depends on: 1201996
Depends on: 1203109
Depends on: 1203190
Depends on: 1205459
Bug 1205459 causes major problems for developer tools, I know this bug is specifically about launching apz in release - should we log a separate bug for enabling apz in aurora/dev edition?
Flags: needinfo?(bugmail.mozilla)
My understanding (based on comment 0) is that APZ won't even go to Aurora until the dependencies of this bug are fixed.
What Botond said :)
Flags: needinfo?(bugmail.mozilla)
Depends on: 1200580
Summary: Let APZ ride the train → Let APZ on desktop ride the train
Depends on: 1198021
Depends on: 1208382
Depends on: 1207412
Depends on: 1208829
Depends on: 1209100
No longer depends on: 1209100
Depends on: 1209058
Depends on: 1209964
Depends on: 1210418
Depends on: 1210496
Depends on: 1210377
Depends on: 1211280
Depends on: 1211777

Updated

4 years ago
Depends on: 1213324
Depends on: 1211506
Depends on: 1214170

Updated

4 years ago
Depends on: 1216488
No longer depends on: 1217627
No longer depends on: 1201192
No longer depends on: 1212020
Depends on: 1213073
Depends on: 1214261
No longer depends on: 1211777
Depends on: 1220091
Depends on: 1216924
Depends on: 1152049
Depends on: 1193055
Depends on: 1222190
No longer depends on: 1188430
Depends on: 1225868
No longer depends on: 1188624
Depends on: 1160601
Depends on: 1176395
Depends on: 1229039
Depends on: 1229052
Depends on: 1229451
Depends on: 1228407
Depends on: 1230228
No longer depends on: 1193969
No longer depends on: 1197654
No longer depends on: 1201996
No longer depends on: 1209058
No longer depends on: 1230228
No longer depends on: 1160601
Depends on: apz-talos
No longer depends on: 1216924

Updated

3 years ago
No longer depends on: 1193781
Depends on: 1193781
No longer depends on: 1213073

Updated

3 years ago
Depends on: 1232184
No longer depends on: 1150941
No longer depends on: apz-talos
Posted patch PatchSplinter Review
Considering we're going to let APZ ride on 46 we might as well get this landed. Any outstanding dependencies can be uplifted to 46 if they don't land in this cycle.
Assignee: nobody → bugmail.mozilla
Attachment #8702668 - Flags: review?(botond)
Comment on attachment 8702668 [details] [diff] [review]
Patch

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

Exciting times!
Attachment #8702668 - Flags: review?(botond) → review+
Release Note Request (optional, but appreciated)
[Why is this notable]: Big improvement for users
[Suggested wording]: Async Pan/Zoom
[Links (documentation, blog post, etc)]: do we have a blog post?
relnote-firefox: --- → ?

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/84479159751f
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Depends on: 1235994
All dependencies fixed too \o/
Can we uplift this to current Aurora, so that beta45 gets it?
Flags: needinfo?(bugmail.mozilla)
(In reply to :Felipe Gomes (needinfo me!) from comment #10)
> Can we uplift this to current Aurora, so that beta45 gets it?

So I believe we want to this because of the experiment that we intend to run for e10s in Beta 45. However, do we have a sense of the quality of the feature at this point? Is it good enough for Beta?

Adding Vasilica to provide some input on this.
Flags: needinfo?(vasilica.mihasca)
(In reply to :Felipe Gomes (needinfo me!) from comment #10)
> Can we uplift this to current Aurora, so that beta45 gets it?

I just responded to your email. I'll post it here as well for posterity:

We fixed a whole slew of APZ-related things in 46; those fixes
are not in 45. So if you enable APZ on 45 (either aurora or for the
beta experiment, or both) you will get a "bad" APZ experience, lacking
fixes for a number of things. So:

- If you're planning on letting e10s ride to release on 45, then I
don't think it makes sense to turn on APZ on 45 at all, because the
experiment will give you results for "bad" APZ whereas the release
population will see no APZ at all.
- If the experiment is just a short-term thing (1 or 2 betas), then
I'm ok with turning on APZ for the 45 beta experiment to gather some
data, but keep in mind that the APZ experience will be better in 46.
- If the experiment is for the entire length of the 45 beta, and you
really want to have the "bad" APZ experience enabled, then I won't
really object but I'm not sure there's much value in doing that.
Instead, I would suggest that you leave it off for the 45 experiment
on beta. That will give you data on how e10s works without APZ. The 46
beta will have APZ enabled anyway, and that will give you data on how
e10s works *with* APZ. If it turns out the 46 beta has a bunch of
critical APZ bugs, then we can turn off APZ on 46 with more confidence
since we tested the e10s without APZ combo on 45.

Regardless of what we go with, I'd rather not remove the #ifdef on 45
aurora until very late in the cycle, because I don't want to subject
our aurora population to the "bad" APZ - it will probably just result
in a bunch of bugs being filed which are already fixed in 46. If you
want APZ in the 45 beta experiment I'd wait to remove the ifdef until
just before or just after the next merge.
Flags: needinfo?(bugmail.mozilla)
Removing ni from Vasilica since kats has pretty much clarified things here.
Flags: needinfo?(vasilica.mihasca)
Whiteboard: [gfx-noted] → [gfx-noted][e10s-45-uplift]
Comment on attachment 8702668 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: This is necessary to allow apz to be enabled during the e10s beta45 experiment. APZ is only active for users of e10s, so this change has no effect for non-e10s users on beta or when this gets to release.
[User impact if declined]: The testing done during the experiment wouldn't be useful as everyone would be testing something far from what we plan on shipping.
[Describe test coverage new/current, TreeHerder]: I imagine apz has tests..
[Risks and why]: There are some known apz bugs on 45 that have been fixed on 46 but are too risky to be uplifted, so the users will be exposed to them. The consensus is that this is better than not testing apz at all with e10s.
[String/UUID change made/needed]: none
Attachment #8702668 - Flags: approval-mozilla-aurora?
Comment on attachment 8702668 [details] [diff] [review]
Patch

I am sorry but I cannot accept this uplift. Comment #12 suggests that we already know that the experience will be bad for users. We have a few users on the beta channel: I don't want to give them a bad experience. And we are planning to target an important percentage of users with the experiment.

Moreover, uplifting a new important feature from nightly to beta is not the proper way to do things.
Attachment #8702668 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Felipe, approx how many users would be impacted by this?
Flags: needinfo?(felipc)
45% of the users will be part of the experiment, but a number of them won't be enabled due to a11y. So I'd say approx. 40%.
Flags: needinfo?(felipc)
Here is the list of bugs that are fixed in 46 (where we plan to enable APZ for release) and not fixed in 45: http://tinyurl.com/jmosybc

In that list, only bug 1230693 concerns me, and Kats has already suggested that we uplift that to 45 anyway. If anyone sees anything else in that list that concerns them, please flag it.

Sylvestre, my point here is that the "bad" that Kats refers to in comment 12 is mostly polish issues and shouldn't drive people away from beta any more than they'll be driven away in 46 when we plan to enable it. Based on that, can you reconsider your uplift approval decision?
Flags: needinfo?(sledru)
I agree with what Brad said. Although the list of bugs is scary-looking I don't think they will actually force users to stop using Firefox, specially if they have a way to turn off e10s/APZ. We will probably get a flood of bug reports for things that are already fixed in 46 though.
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #18)
> In that list, only bug 1230693 concerns me, and Kats has already suggested
> that we uplift that to 45 anyway. If anyone sees anything else in that list
> that concerns them, please flag it.

Bug 1168263 was backed out from Aurora45 as part of the massive preserve-3d backout (https://hg.mozilla.org/releases/mozilla-aurora/rev/64ec448f156d), so Aurora shouldn't be affected by bug 1230693 at this point. I intend to verify the various deps affected by that backout later today.
Kats, can you share the bug we should expect to see reported if we accept this in beta?
Thanks
Flags: needinfo?(sledru)
The list brad linked to in comment 18 probably covers the sorts of bugs we'd expect to see filed. Specifically, there will likely be bugs about page elements jittering or getting out of sync temporarily as you scroll, crashes/distorted rendering on some HiDPI displays or super-long pages, high-ish amounts of checkerboarding, and blurry fonts (bug 1122916 had the most amount of dupes when APZ was enabled on nightly).
Oh, also RTL scrollbar issues and issues with windowed flash plugins (bug 1152049)
The issues you describe seems pretty serious and could badly reflect on the image of Firefox.
40% of the beta population is way too much people to do an experiment which will decrease the quality of Firefox... Especially when we know the issues upfront.

So, please wait 46 to experiment e10s with APZ.
(In reply to Sylvestre Ledru [:sylvestre] from comment #24)
> So, please wait 46 to experiment e10s with APZ.

FWIW, this will also allow us to compare synchronous scroll with and without e10s on 45 (this probe appeared first on 45). Otherwise, if APZ was to get enabled in 45, then the biggest trigger for scrolls (mouse wheel) would have used APZ rather than synchronous.

So that's another useful thing when postponing APZ to 46.
Noting for 46 aurora as Asynchronous Panning and Zooming 
kats, how about a link to https://staktrace.com/spout/entry.php?id=834?  
Or, better, can you post something on a mozilla.org blog that I can link to from the release notes?
Flags: needinfo?(bugmail.mozilla)
Sorry for the delayed response. I'll draft a blog post for hacks.m.o, which seems like an appropriate place to put this announcement. Assuming APZ stays on the 46 train, should the article be posted when 46 goes to beta? To release? Or anytime is fine?

Leaving ni on me until I get it written.
Depends on: 1104356
There's a hacks blog post up now: https://hacks.mozilla.org/2016/02/smoother-scrolling-in-firefox-46-with-apz/ - feel free to link to that in the release notes.
Flags: needinfo?(bugmail.mozilla) → needinfo?(lhenry)
Depends on: 1247854
Depends on: 1239615
Thanks! I updated the link in the release notes.
Flags: needinfo?(lhenry)
Depends on: 1253667
Depends on: 1252822
Depends on: 1252947
Depends on: 1227799
I filed a new metabug for APZ release blockers (bug 1254668) since cpeterson said it was confusing that this bug was fixed. Any open release blockers should block that instead.
No longer depends on: 1227799, 1247854, 1252822, 1252947
This is also now out of the 46 relnotes, but I will put it back if APZ goes to release!
Depends on: 1257181
You need to log in before you can comment on or make changes to this bug.