1459605 - Intermittent devtools/client/application/test/browser_application_panel_list-domain-workers.js | Uncaught exception - at resource://devtools/shared/base-loader.js -> resource://devtools/shared/client/debugger-client.js:602 - Error: 'unregister

Status: RESOLVED FIXED

(DevTools :: General, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: cbrindusan [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=177238992&repo=autoland

https://queue.taskcluster.net/v1/task/S20deXXmRqKgd4pI2PMiuA/runs/0/artifacts/public/logs/live_backing.log

https://treeherder.mozilla.org/logviewer.html#?job_id=177238992&repo=autoland&lineNumber=4238

[task 2018-05-07T10:18:26.263Z] 10:18:26     INFO - TEST-START | devtools/client/application/test/browser_application_panel_list-domain-workers.js
[task 2018-05-07T10:18:27.511Z] 10:18:27     INFO - TEST-INFO | started process screentopng
[task 2018-05-07T10:18:27.973Z] 10:18:27     INFO - TEST-INFO | screentopng: exit 0
[task 2018-05-07T10:18:27.974Z] 10:18:27     INFO - Buffered messages logged at 10:18:26
[task 2018-05-07T10:18:27.976Z] 10:18:27     INFO - Entering test bound 
[task 2018-05-07T10:18:27.977Z] 10:18:27     INFO - Adding a new tab with URL: http://example.com/browser/devtools/client/application/test/service-workers/simple.html
[task 2018-05-07T10:18:27.978Z] 10:18:27     INFO - Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 170}]
[task 2018-05-07T10:18:27.978Z] 10:18:27     INFO - Tab added and finished loading
[task 2018-05-07T10:18:27.979Z] 10:18:27     INFO - Buffered messages logged at 10:18:27
[task 2018-05-07T10:18:27.980Z] 10:18:27     INFO - Wait until the service worker appears in the application panel
[task 2018-05-07T10:18:27.981Z] 10:18:27     INFO - TEST-PASS | devtools/client/application/test/browser_application_panel_list-domain-workers.js | First service worker registration is displayed for the correct domain - 
[task 2018-05-07T10:18:27.984Z] 10:18:27     INFO - Navigate to another page for a different domain with no service worker
[task 2018-05-07T10:18:27.985Z] 10:18:27     INFO - Waiting for event: 'navigate' on TabTarget:[object XULElement].
[task 2018-05-07T10:18:27.985Z] 10:18:27     INFO - Got event: 'navigate' on TabTarget:[object XULElement].
[task 2018-05-07T10:18:27.988Z] 10:18:27     INFO - TEST-PASS | devtools/client/application/test/browser_application_panel_list-domain-workers.js | No service workers are shown for an empty page in a different domain. - 
[task 2018-05-07T10:18:27.989Z] 10:18:27     INFO - Navigate to another page for a different domain with another service worker
[task 2018-05-07T10:18:27.993Z] 10:18:27     INFO - Waiting for event: 'navigate' on TabTarget:[object XULElement].
[task 2018-05-07T10:18:27.994Z] 10:18:27     INFO - Got event: 'navigate' on TabTarget:[object XULElement].
[task 2018-05-07T10:18:27.994Z] 10:18:27     INFO - Got event: 'navigate' on TabTarget:[object XULElement].
[task 2018-05-07T10:18:27.995Z] 10:18:27     INFO - Wait until the service worker appears in the application panel
[task 2018-05-07T10:18:27.999Z] 10:18:27     INFO - TEST-PASS | devtools/client/application/test/browser_application_panel_list-domain-workers.js | Second service worker registration is displayed for the correct domain - 
[task 2018-05-07T10:18:28.000Z] 10:18:27     INFO - Unregister all service workers
[task 2018-05-07T10:18:28.000Z] 10:18:27     INFO - Buffered messages finished
[task 2018-05-07T10:18:28.001Z] 10:18:28     INFO - TEST-UNEXPECTED-FAIL | devtools/client/application/test/browser_application_panel_list-domain-workers.js | Uncaught exception - at resource://devtools/shared/base-loader.js -> resource://devtools/shared/client/debugger-client.js:602 - Error: 'unregister' request packet has no destination.
[task 2018-05-07T10:18:28.001Z] 10:18:28     INFO - Stack trace:
[task 2018-05-07T10:18:28.002Z] 10:18:28     INFO - request@resource://devtools/shared/base-loader.js -> resource://devtools/shared/client/debugger-client.js:602:13
[task 2018-05-07T10:18:28.002Z] 10:18:28     INFO - unregisterAllWorkers@chrome://mochitests/content/browser/devtools/client/application/test/head.js:64:11

Comment 1

[Intermittent Failures Robot]

26 failures in 140 pushes (0.186 failures/push) were associated with this bug yesterday.    

Repository breakdown:
* mozilla-inbound: 11
* autoland: 6
* try: 5
* mozilla-central: 4

Platform breakdown:
* linux64: 11
* linux32: 9
* osx-10-10: 3
* linux64-devedition: 2
* linux64-nightly: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1459605&startday=2018-05-08&endday=2018-05-08&tree=trunk

Comment 2

[Cosmin Sabou [:CosminS]]

Hi, did some retriggers to find where this originated and found the following;

Worked on this range: http://tinyurl.com/yc8u56sa

Seems like this fail started from here: http://tinyurl.com/y9r7so22 where it was worked on devtools/client/application/test/browser_application_panel_list-domain-workers.js 

Julian, could you take a look at this? Thank you.

Comment 3

[Intermittent Failures Robot]

16 failures in 127 pushes (0.126 failures/push) were associated with this bug yesterday.    

Repository breakdown:
* autoland: 11
* mozilla-inbound: 2
* mozilla-central: 2
* try: 1

Platform breakdown:
* linux64: 9
* osx-10-10: 5
* linux32: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1459605&startday=2018-05-09&endday=2018-05-09&tree=trunk

Comment 4

[Julian Descottes [:jdescottes]]

Thanks for logging! 

The issue is most likely that we attempt to unregister workers that are not yet fully registered and don't have a registration actor. See the code branch at https://searchfox.org/mozilla-central/rev/eb6c5214a63e20a3fff455e92c876287a8d2e188/devtools/shared/client/root-client.js#172-180

2 options here:
- force each test to wait until workers are registered before calling unregisterAllWorkers
- wait for all worker forms to have a valid registration actor before attempting to unregister

Second option should be cleaner but implies:
- modifying the waitUntil() helper to support async predicates (or creating a separate asyncWaitUntil)
- also implies that the service worker will finish registering even if the page is closed

I have try pushes with both approaches ongoing, will report and clear ni? when they are done.

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1459605 - Wait for all workers to have registrationActors before unregistering;

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

Comment 6

[Julian Descottes [:jdescottes]]

Went for the second option, adding a new dedicated asyncWaitUntil (as expected, modifying waitUntil to support both sync and async makes some other tests fail spectacularly).

This seems to fix the issue: 
- with patch https://treeherder.mozilla.org/#/jobs?repo=try&revision=356c1ab0f3333d8925da679eea35bf3296fa81f4&selectedJob=177794593 no failure in 70 runs of dt3
- without patch https://treeherder.mozilla.org/#/jobs?repo=try&revision=78cc5b1a3f90f271a2ee773cb127afa0c5ed98d0&selectedJob=177815180 10 failures in 80 runs of dt3

Comment 7

[Belén [:ladybenko]]

Comment on attachment 8974627 [details]
Bug 1459605 - Wait for all workers to have registrationActors before unregistering;

https://reviewboard.mozilla.org/r/243010/#review248830

`+r` with a minor suggestion about the `info` messages. Thanks a lot!

::: devtools/client/application/test/head.js:69
(Diff revision 1)
> +    const allWorkersRegistered =
> +      workers.service.every(worker => !!worker.registrationActor);
> +    return allWorkersRegistered;
> +  });
>  
> -  for (let worker of service) {
> +  info("All workers have a valid registrationActor and can be unregistered");

Could we swap the two `info` messages so they better reflect what is going on? i.e. the first one informing of the check for the registration actor, and then have the second one with the unregister message.

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8974627 [details]
Bug 1459605 - Wait for all workers to have registrationActors before unregistering;

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

Comment 9

[Julian Descottes [:jdescottes]]

(In reply to Belén [:ladybenko] from comment #7)
> Comment on attachment 8974627 [details]
> Bug 1459605 - Wait for all workers to have registrationActors before
> unregistering;
> 
> https://reviewboard.mozilla.org/r/243010/#review248830
> 
> `+r` with a minor suggestion about the `info` messages. Thanks a lot!

Thanks for the review!
> 
> ::: devtools/client/application/test/head.js:69
> (Diff revision 1)
> > +    const allWorkersRegistered =
> > +      workers.service.every(worker => !!worker.registrationActor);
> > +    return allWorkersRegistered;
> > +  });
> >  
> > -  for (let worker of service) {
> > +  info("All workers have a valid registrationActor and can be unregistered");
> 
> Could we swap the two `info` messages so they better reflect what is going
> on? i.e. the first one informing of the check for the registration actor,
> and then have the second one with the unregister message.

Good point, done.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/149fcd977c3d
Wait for all workers to have registrationActors before unregistering;r=ladybenko

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/149fcd977c3d

Comment 12

[Intermittent Failures Robot]

63 failures in 660 pushes (0.095 failures/push) were associated with this bug in the last 7 days. 

This is the #19 most frequent failure this week.  

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **  

Repository breakdown:
* autoland: 26
* mozilla-inbound: 15
* try: 14
* mozilla-central: 8

Platform breakdown:
* linux64: 33
* linux32: 15
* osx-10-10: 9
* linux64-devedition: 3
* windows7-32: 1
* linux64-nightly: 1
* linux32-devedition: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1459605&startday=2018-05-07&endday=2018-05-13&tree=trunk