1416807 - Don't schedule syncs while we're behind to a captive portal

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P2)

Description

[Edouard Oger [:eoger]]

I believe we have to listen to "captive-portal-login" here.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

(In reply to Edouard Oger [:eoger] from comment #0)
> I believe we have to listen to "captive-portal-login" here.

And probably also have _checkSync look at nsICaptivePortalService.state. More generally, I think we probably want a single helper in sync that roughly means "am I online", which checks the IOService, the network link service and this captive portal service.

Comment 2

[Edouard Oger [:eoger]]

Attachment: Bug 1416807 - Don't schedule syncs while we're behind to a captive portal.

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

Comment 3

[Edouard Oger [:eoger]]

Mocking CaptivePortalService and NetworkLinkService is a PITA, sorry :/

Comment 4

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8928683 [details]
Bug 1416807 - Don't schedule syncs while we're behind to a captive portal.

https://reviewboard.mozilla.org/r/199928/#review205112

Thanks!

::: services/sync/modules/policies.js:139
(Diff revision 1)
> +    // Finally, if both of these services don't tell us we're offline for sure,
> +    // we'll ask the captive portal service if we are behind a locked captive
> +    // portal.
> +    //
> +    // With these 3 services combined, we believe we can avoid all false positives
> +    // and make a good guest on whether an user is online or not in most cases.

s/guest/guess/

Comment 5

[Mark Hammond [:markh] [:mhammond]]

(In reply to Edouard Oger [:eoger] from comment #3)
> Mocking CaptivePortalService and NetworkLinkService is a PITA, sorry :/

Yeah, but that's not really a big deal - IMO, the biggest risk in this that is that the real services are unreliable - which mocking in tests isn't going to help with.

Comment 6

[Edouard Oger [:eoger]]

Comment on attachment 8928683 [details]
Bug 1416807 - Don't schedule syncs while we're behind to a captive portal.

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8967a5cf4c31
Don't schedule syncs while we're behind to a captive portal. r=markh

Comment 8

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/8967a5cf4c31