1424943 - Intermittent dom/workers/test/serviceworkers/test_error_reporting.html | monitorConsole | number of messages [{errorMessage:"Failed to get service worker\u2019s client(s): Storage access is restricted in this context due to user settings or priva

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

This is failing 2 out of 11 times here:

https://treeherder.mozilla.org/logviewer.html#?job_id=151339987&repo=mozilla-inbound&lineNumber=4289

I'd like to investigate a fix for this tomorrow instead of backing out bug 1293277 if possible.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

I think the problem is that the test expect the console message to be reported by the time the fetch('getclients') completes here:

https://searchfox.org/mozilla-central/rev/9d920555ec81f1c9d4b7fa3b08e23eb88efb60e1/dom/workers/test/serviceworkers/test_error_reporting.html#246

But the fetch event handler does not actually wait for the various Clients API calls:

https://searchfox.org/mozilla-central/rev/9d920555ec81f1c9d4b7fa3b08e23eb88efb60e1/dom/workers/test/serviceworkers/sw_storage_not_allow.js#5

The new clients code is definitely slower since it has to do some IPC bounces the old code did not.  I think this is letting the network fetch win the race sometimes.

The solution is to probably just use a pass-through fetch handler and to delay completing respondWith() promise until the various client API promises resolve.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Fix test_error_reporting.html not to race network vs Clients API. r=tt

Tom, can you review this patch?  It makes test_error_reporting.html's service worker wait for the Clients API calls to resolve or reject before completing the outer fetch() call.

The clients API used to consistently win the race with network because it was completely local to the process.  After bug 1293277, however, we now do some IPC messaging which makes clients API slower and sometimes it loses to the network request now.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=482caa5024d838bbef61bfc974b33b9e7938347e

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

This seems to fix the problem in that try push.  I did 20 retriggers and no instances of this failure.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Oops, I did my previous retriggers on debug, not opt.  Let me try again.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Ok, 20 retriggers on opt with the test actually running also show that the intermittent is fixed with this patch.

Comment 6

[Tom Tung [:tt, :ttung]]

Comment on attachment 8936524 [details] [diff] [review]
Fix test_error_reporting.html not to race network vs Clients API. r=tt

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

Oh, I should ensure |clients.matchAll()| in the service worker to be done before finishing the fetch request in the content. Thanks!

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Thanks!  Marking checkin-needed since I'm not at my work computer at the moment.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

I'll land this.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c885e730dded
Fix test_error_reporting.html not to race network vs Clients API. r=tt

Comment 10

[Natalia Csoregi [:nataliaCs]]

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

Comment 11

[Intermittent Failures Robot]

5 failures in 423 pushes (0.012 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* mozilla-inbound: 4
* autoland: 1

Platform breakdown:
* linux64: 5

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