New issue
Advanced search Search tips

Issue 675942 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Sync events fired when not appropriate

Reported by stef...@gmail.com, Dec 20 2016

Issue description

Steps to reproduce the problem:
This has to do with Background sync (and I don't think any other browser supports it at all, that's why it's "just a Chromium problem). I am testing it on an Android tablet that has both Cellular and WiFi access, and test by toggling WiFi on and off (when WiFi is disabled the connection moves to cellular and the other way around), and also by (when connected to WiFi) connecting to another WiFi network.

All code is in the ServiceWorker, and the basic functionality is to a) listen to navigator.connection "typechange" events, and when that happens b) register to send a report to the server on "sync" event.

The core parts look something like (all in the ServiceWorker):

self.addEventListener('sync', (evt) => {
    console.log('SW sync event fired, event tag: ' + evt.tag);
    if (evt.tag == 'send_client_status_reports') {
        evt.waitUntil(sendClientReports());
    }
});
navigator.connection.addEventListener('typechange', networkChangeHandler);

function networkChangeHandler() {
    collectClientStatus().then(() => {
        console.log('gonna register for sending on sync event');
        // If we're online the event should fire immediately
        self.registration.sync.register('send_client_status_reports').then(() => {
            console.log('sync registration success');
        }).catch(e => {
            console.log('sync registation failed, error: ' + e);
        });
    })
    .catch(error => {
        logger.error(`Could not collectClientStatus ${error}\n${error.stack}`);
    });
}

function sendClientReports() {
   // basically just sends off the report created by collectClientStatus using
   // fetch with method 'POST'
   // and returns a Promise that is resolved if the POST is successful and rejected otherwise
}

What is the expected behavior?
When the 'typechange' event fires, it is (to me) expected that the 'sync' event would not fire until there is a working connection up.

What went wrong?
This is what I typically see:

Case 1: wifi -> cellular
------------------------
1. navigator.connection fires "typechange" (type: "none")
2. script registers to send report with on "sync" event
3. "sync" event fires immediately but sending (of course) fails (since 
network is "none")
4. navigator.connection fires another "typechange" (type: "cellular")
5. script registers to send reports with on "sync" event
---- time passes ----
6. "sync" event fires again and reports are sent.

Case 2: cellular -> wifi
------------------------
1. navigator.connection fires "typechange" (type: "wifi")
2. script registers to send report with on "sync" event
3. "sync" event fires immediately but sending fails
---- time passes ----
4. "sync" event fires again and report is sent.

Case 3: wifi -> wifi
--------------------
This is quite similar to Case 1, with the following additional steps 
between 5. and 6:
5.1. navigator.connection fires "typechange" (type: "wifi")
5.2. script registers to send report with on "sync" event

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 57.0.2950.3  Channel: n/a
OS Version: 6.0.1
Flash Version: -

There seems to be some kind of race condition, sometimes things just work (I assume then the new connection is able to get up quickly enough to be able to send
 
Thanks for the reproduction code. I've modified it slightly and put together https://cr.kungfoo.net/bgsync/fire/.

If you start on Android on wifi and then disable wifi (to switch to cell) I can reproduce. I see the following:

sw.js:15 registering for sending on sync event
sw.js:16 network type is none
sw.js:19 sync registration success
sw.js:2 SW sync event fired. Network type is none
https://cr.kungfoo.net/jkarlin/timing/size.php?size=1 Failed to load resource: net::ERR_NETWORK_CHANGED
sw.js:34 sync download failed
sw.js:1 Uncaught (in promise) undefined
sw.js:1 Service Worker termination by a timeout timer was canceled because DevTools is attached.
... 5 minutes later ...
sw.js:2 SW sync event fired. Network type is cellular
sw.js:30 sync download succeeded


So we're getting the net::ERR_NETWORK_CHANGED on the Fetch. I'm a bit surprised by that, as the code doesn't fire the sync event until the browser thinks the network type is !none. So either that aspect is broken, or the network stack doesn't act upon the network change (resetting existing connections) until a good bit later.

Comment 2 by stef...@gmail.com, Dec 21 2016

Thanks for testing, and I'm glad you could reproduce it. I get exactly the same result when testing https://cr.kungfoo.net/bgsync/fire/. 

A small proposal: could you remove the if statement 
("  if (navigator.connection.type != 'none') {
    // We want to see if the sync event fires when the network type is none.
    return;
  }") ?

and then test if you get "my" problem also when re-enabling WiFi? For me, as described, there is never a situation when the network is "none" in that case (instead it is immediately "wifi" when tested after the "typechange" event), but when "sync" fires (immediately) the server can't be reached.

Components: Services>Sync
Cc: iclell...@chromium.org
Components: -Services>Sync
Removing Services>Sync as it's not related.

Ian, would you be interested in tackling this one?
Owner: iclell...@chromium.org
Status: Assigned (was: Unconfirmed)
SGTM; I'll take a look.

Comment 6 by stef...@gmail.com, Jan 23 2017

Just curious, any progress on this one?

Sign in to add a comment