Closed Bug 1381335 Opened 2 years ago Closed 2 years ago

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.

Categories

(Firefox :: General, defect, P1)

All
Linux
defect

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: Mardak, Assigned: Mardak)

References

Details

(Keywords: intermittent-failure, Whiteboard: [photon-onboarding])

Attachments

(1 file)

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.
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 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
Attachment #8886885 - Flags: review?(dmose)
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.
Attachment #8886885 - Flags: review?(fliu) → feedback+
Assignee: nobody → edilee
Priority: -- → P1
Whiteboard: [photon-onboarding]
Status: NEW → ASSIGNED
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.
Attachment #8886885 - Flags: review?(usarracini)
Attachment #8886885 - Flags: review?(dtownsend)
Renaming so it's easier to find with treeherder if a similar failure comes up later.
OS: Unspecified → Linux
Hardware: Unspecified → All
Summary: Increase waitForCondition time for initial onboarding-overlay check → 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.
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
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 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
Attachment #8886885 - Flags: review?(dmose) → review+
Pushed by edilee@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ec0dc3ef3bf7
Increase waitForCondition time for initial onboarding-overlay check. f=Fischer r=dmose
Blocks: 1381569
https://hg.mozilla.org/mozilla-central/rev/ec0dc3ef3bf7
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Duplicate of this bug: 1381679
Bug is still alive: https://treeherder.mozilla.org/logviewer.html#?job_id=116587573&repo=autoland
Status: RESOLVED → REOPENED
Flags: needinfo?(edilee)
Resolution: FIXED → ---
(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.
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Flags: needinfo?(edilee)
Resolution: --- → FIXED
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.