Closed Bug 1424943 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

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.
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.
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
Attachment #8936524 - Flags: review?(ttung)
This seems to fix the problem in that try push.  I did 20 retriggers and no instances of this failure.
Oops, I did my previous retriggers on debug, not opt.  Let me try again.
Ok, 20 retriggers on opt with the test actually running also show that the intermittent is fixed with this patch.
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!
Attachment #8936524 - Flags: review?(ttung) → review+
Thanks!  Marking checkin-needed since I'm not at my work computer at the moment.
Keywords: checkin-needed
I'll land this.
Keywords: checkin-needed
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c885e730dded
Fix test_error_reporting.html not to race network vs Clients API. r=tt
https://hg.mozilla.org/mozilla-central/rev/c885e730dded
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.