New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 827473 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Sync XHR + Service Worker behaving differently in Canary

Project Member Reported by bradnelson@chromium.org, Mar 30 2018

Issue description

This example of a sync XHR hitting a service worker is behaving differently in Canary vs Stable:

https://github.com/awtcode/sync
(Run local server + look in console, repeatedly retries in canary vs success in stable)

It looks like something has changed recently.

 
Cc: ikilpatrick@chromium.org
Owner: bradnelson@chromium.org
Ian, Alex, any thoughts on who to route this to?
I'll bisect tomorrow, but if you happen to know offhand.

Comment 2 by falken@chromium.org, Mar 30 2018

Owner: horo@chromium.org
horo was working on this, probably https://chromium-review.googlesource.com/982776
Cc: knightac...@gmail.com lafo...@chromium.org
I've bisected the issue down to this commit:
https://chromium-review.googlesource.com/c/chromium/src/+/974989

Since this commit Autocad has been broken.
We should not let this issue propagate beyond Dev.

Tsuyoshi, are you able to look into this issue?

Comment 4 by horo@chromium.org, Apr 2 2018

Status: Started (was: Assigned)

Comment 5 by horo@chromium.org, Apr 2 2018

Ah, yes.
The CL accidentally changed the Service Worker handling logic of synchronous XHR from dedicated worker.
I will create a CL to fix it.

Comment 6 by horo@chromium.org, Apr 2 2018

Labels: M-67
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb9fd1a8249db7ca306513e8d8baf41d3e770674

commit bb9fd1a8249db7ca306513e8d8baf41d3e770674
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Mon Apr 02 03:27:45 2018

Set SkipServiceWorker flag for synchronous loads from the main thread.

Before 5e1b52dd7e828fb2e4bd69f599c0c30eec3e873c, synchronous XHR on worker
was handled by service workers. It is because the |is_sync_load| was false
when the sync request is from worker thread. But after the CL, the
|is_sync_load| flag for the sync request from worker became true, so the request
will not go to the service worker.

This CL will fix this by
 - Set the SkipServiceWorker flag for synchronous loads from the main thread
   in the renderer process. (FetchParameters.cpp)
 - Don't set skip_service_worker even if is_sync_load is true in the browser
   process. (resource_dispatcher_host_impl.cc)

Bug:  706331 , 827473 
Change-Id: I186bc97f3f8d298e0a04942d0ec4b708b3022cc1
Reviewed-on: https://chromium-review.googlesource.com/989376
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547407}
[modify] https://crrev.com/bb9fd1a8249db7ca306513e8d8baf41d3e770674/content/browser/loader/resource_dispatcher_host_impl.cc
[add] https://crrev.com/bb9fd1a8249db7ca306513e8d8baf41d3e770674/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-xhr-sync-on-worker.https.html
[add] https://crrev.com/bb9fd1a8249db7ca306513e8d8baf41d3e770674/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/fetch-request-xhr-sync-on-worker-worker.js
[modify] https://crrev.com/bb9fd1a8249db7ca306513e8d8baf41d3e770674/third_party/WebKit/Source/platform/loader/fetch/FetchParameters.cpp

Comment 8 by horo@chromium.org, Apr 2 2018

Status: Fixed (was: Started)
Thanks for the fast fix!

Sign in to add a comment