1381335 - Intermittent browser/extensions/onboarding/test/browser/browser_onboarding_{notification{,2,3,4},tours,tourset}.js | Uncaught exception Should load onboarding overlay - timed out after 30 tries.

Status: RESOLVED FIXED

(Firefox :: General, defect, P1)

Description

[Ed Lee :Mardak]

Turns out when activity-stream is enabled, some reason the initial idle to trigger loading of onboarding can take a bit longer on Linux. From my testing, it often triggers on the 31st check, which is just one check too late for the current 30 checks.

Comment 1

[Ed Lee :Mardak]

Testing with browser.newtabpage.activity-stream.enabled=true

Without this patch, Linux32 debug e10s fails about 60% of the time while Linux64 debug e10s fails about 20% of the time:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=583d77d718566376cd43c969b786ca23ade0ebdf&filter-searchStr=linux%20debug%20e10s

With the patch, there's no failures:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=89c41e0c32d9edace1901608c4d125396927115e&filter-searchStr=linux%20debug%20e10s

Here's the maximum number of `waitForCondition` checks for the initial `onboarding-overlay` I saw when retriggering 20 times:
 1 30
 2 31
 3 27
 4 33
 5 28
 6 27
 7 41
 8 26
 9 38
10 27
11 38
12 36
13 37
14 38
15 45
16 37
17 37
18 28
19 33
20 29

Comment 2

[Ed Lee :Mardak]

Attachment: Bug 1381335 - Increase waitForCondition time for initial onboarding-overlay check. f=Fischer

Use the larger default 50 tries (and 100ms) on the initial loading of the overlay.

Review commit: https://reviewboard.mozilla.org/r/157622/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/157622/

Comment 3

[Ed Lee :Mardak]

Comment on attachment 8886885 [details]
Bug 1381335 - Increase waitForCondition time for initial onboarding-overlay check. f=Fischer

Just in case dmose needs to give an additional review

Comment 4

[Fischer [:Fischer]]

Comment on attachment 8886885 [details]
Bug 1381335 - Increase waitForCondition time for initial onboarding-overlay check. f=Fischer

(In reply to Ed Lee :Mardak from comment #1)
> Testing with browser.newtabpage.activity-stream.enabled=true
> 
> Without this patch, Linux32 debug e10s fails about 60% of the time while Linux64 debug e10s fails about 20% of the time:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=583d77d718566376cd43c969b786ca23ade0ebdf&filtersearchStr=linux%20debug%20e10s
> 
> With the patch, there's no failures:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=89c41e0c32d9edace1901608c4d125396927115e&filter-searchStr=linux%20debug%20e10s
Hi Ed,
Thanks for helping this out. It looks like the debug build runs kind of slower.
The reviewer of this project is :mossop, you would want to set a r? on :mossop.
This patch looks good to me.

Comment 5

[Ed Lee :Mardak]

Comment on attachment 8886885 [details]
Bug 1381335 - Increase waitForCondition time for initial onboarding-overlay check. f=Fischer

Thanks for the f+ Fischer. I believe any Firefox reviewer can r+ even for browser/extensions if s/he feels comfortable. I've added a few people to r? and if it looks good, feel free to push to autoland the commit.

Comment 6

[Ed Lee :Mardak]

Comment on attachment 8886885 [details]
Bug 1381335 - Increase waitForCondition time for initial onboarding-overlay check. f=Fischer

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157622/diff/1-2/

Comment 7

[Ed Lee :Mardak]

Comment on attachment 8886885 [details]
Bug 1381335 - Increase waitForCondition time for initial onboarding-overlay check. f=Fischer

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157622/diff/2-3/

Comment 8

[Ed Lee :Mardak]

Renaming so it's easier to find with treeherder if a similar failure comes up later.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8886885 [details]
Bug 1381335 - Increase waitForCondition time for initial onboarding-overlay check. f=Fischer

https://reviewboard.mozilla.org/r/157620/#review162902

Looks good to me; r=dmose

Comment 10

[Fischer [:Fischer]]

The timing of onboarding load is a bit uncertain. If we went for the dom mutation event to observe the onboaridng element, there could be some potential intermittent issue (hard to guarantee the timing of adding observer comes before the onboaridng element appended). Another possible solution for this is to send some message, like `notifyObserver` to notify the loading of the onboarding element in the onboarding.js so we could add an observer even before loading about:newtab. However, this is adding more costs just for testing. We don't want more costs so simply go for `waitForCondition`. Why 3s is because in the beginning we though 3s should be long enough(the load of onboarding is inside `requestIdleCallback`) but looks like the debug build could run slower a bit.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8886885 [details]
Bug 1381335 - Increase waitForCondition time for initial onboarding-overlay check. f=Fischer

https://reviewboard.mozilla.org/r/157622/#review162912

r=dmose

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec0dc3ef3bf7
Increase waitForCondition time for initial onboarding-overlay check. f=Fischer r=dmose

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 15

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

Bug is still alive: https://treeherder.mozilla.org/logviewer.html#?job_id=116587573&repo=autoland

Comment 17

[Ed Lee :Mardak]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #15)
> Bug is still alive:
> https://treeherder.mozilla.org/logviewer.html#?job_id=116587573&repo=autoland
This was newly regressed. Tracking in bug 1383070.

Comment 18

[Intermittent Failures Robot]

1 failures in 822 pushes (0.001 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 1

Platform breakdown:
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1381335&startday=2017-07-17&endday=2017-07-23&tree=all

Comment 19

[Intermittent Failures Robot]

2 failures in 1008 pushes (0.002 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* try: 2

Platform breakdown:
* linux64: 1
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1381335&startday=2017-07-24&endday=2017-07-30&tree=all