1525413 - Parallelize and wait for endpoint requests before rendering Discovery Stream

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P1)

Description

[Scott [:thecount] Downe]

To deal with loading jank, for now we're going to wait until everything is ready until we render the layout.

Comment 1

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4761

Comment 2

[Scott [:thecount] Downe]

For QA I would say just make sure dev-test-all or basic load and nothing looks out of place.

This bug is likely to break in very noticeable and dramatic ways, so if it's displaying content still I think we're good.

Comment 3

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/ba5eb152fe23

Comment 4

[Kate Hudson :k88hudson]

Looks like this depends on Bug 1520258 to be uplifted

Comment 5

[Kate Hudson :k88hudson]

Attachment: Bug 1525413 - Parallelize and wait for endpoint requests before rendering Discovery Stream

Comment 6

[Kate Hudson :k88hudson]

Can you write the uplift request for this?

Comment 7

[Scott [:thecount] Downe]

Comment on attachment 9043424 [details]
Bug 1525413 - Parallelize and wait for endpoint requests before rendering Discovery Stream

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

None

User impact if declined

Experiment data less reliable. User performance issues.

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

No

Needs manual test from QE?

Yes

If yes, steps to reproduce

1. Set the pref browser.newtabpage.activity-stream.discoverystream.config to {"enabled":true,"show_spocs":true,"layout_endpoint":"https://getpocket.com/v3/newtab/layout?version=1&consumer_key=40249-e88c401e1b1f2242d9e441c4&layout_variant=dev-test-all"}
2. Load up a new tab.
3. Watch for when new tab loads, it loads all in one shot, and not concurrently, ie elements shouldn't load until everything is ready, and load and render once, and not load elements as they come in, causing the page to jump around.

I'm not able to throttle this using responsive design mode tools as responsive design mode does wonky things in about:home, but with the above steps, I have been able to do it fast enough to see that everything gets loaded at once. If you need to try it again, you can change the pref from the above one in step 1 to {"enabled":true,"show_spocs":true,"layout_endpoint":"https://getpocket.com/v3/newtab/layout?version=1&consumer_key=40249-e88c401e1b1f2242d9e441c4&layout_variant=dev-test-all" }, notice the extra space after dev-test-all" which triggers a fresh load/cache clear.

List of other uplifts needed

Bug 1520258

Risk to taking this patch

Medium

Why is the change risky/not risky? (and alternatives if risky)

It's preffed off and has tests, but really hard to verify because it relies on network requests..

String changes made/needed

None

Comment 8

[Kate Hudson :k88hudson]

Can you verify this one?

Comment 9

[Brahmini Nagabandi]

There are 2 different issues found on Mac OS and Windows OS respectively.

Windows :

"dev-test-all" end-point doesn't work on Windows 10 Pro.

Tracking here : bug 1527816

Mac OS :

Layout has some words cutoff specifically for the letter g on some cards (maybe css issue?)

Tracking here : bug 1527819

Comment 10

[Kate Hudson :k88hudson]

Brahmini, it sounds like those issues are unrelated to this patch and don't need to block the uplift.

1. For Bug 1527816, we're unable to reproduce this/don't have any reports of nightly not working on Windows 10 – are you sure you have your configuration set up correctly?
2. For Bug 1527819, this reproduce-able on nightly but unrelated and non-blocking for this uplift.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9043424 [details]
Bug 1525413 - Parallelize and wait for endpoint requests before rendering Discovery Stream

Let's go ahead with this uplift for beta 8 since the issues found by QA aren't blocking.

Comment 12

[Scott [:thecount] Downe]

Clearing need info, Kate said it well.

Comment 13

[Brahmini Nagabandi]

I had no clue about the pref browser.newtabpage.activity-stream.discoverystream.optOut.0 (which was true), so having incorrect value on Windows caused the issue.

After pref reset, it works as expected.

Thanks for the clarification.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/0f8d776acfbf

Comment 15

[Brahmini Nagabandi]

Tested bug 1527816.

Closing as verified.

Comment 16

[Ciprian Muresan [:cmuresan], Ecosystem QA]

I have verified that the issue is no longer reproducible on the latest Beta 66.0b8 (Build ID 20190214195736) on Windows 10 x64, Mac 10.14, and Arch Linux 4.14.3.