678586 - Trigger a delayed sync after pairing a new device

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Philipp von Weitershausen [:philikon]]

When you add a new device it'd be good to trigger a sync after a little while (having given the new client enough time to sync down and up).

Comment 1

[Richard Newman [:rnewman]]

Possible interaction with Bug 675824, which involves monitoring sync status on the other device to know when to start syncing and showing progress.

Might split out the shared work into a "credential exchange progress notification channel" bug...?

Comment 2

[Philipp von Weitershausen [:philikon]]

(In reply to Richard Newman [:rnewman] from comment #1)
> Possible interaction with Bug 675824, which involves monitoring sync status
> on the other device to know when to start syncing and showing progress.

Aye.

> Might split out the shared work into a "credential exchange progress
> notification channel" bug...?

Pretty sure this will also need an AbstractSingletonProxyFactoryBean.

Comment 3

[Richard Newman [:rnewman]]

(In reply to Philipp von Weitershausen [:philikon] from comment #2)

> Pretty sure this will also need an AbstractSingletonProxyFactoryBean.

…ControllerImpl.

Comment 4

[Philipp von Weitershausen [:philikon]]

My crude implementation just schedules another Sync for activeDeviceInterval after a successful exchange. I think that's sufficiently simple yet effective.

Comment 5

[Philipp von Weitershausen [:philikon]]

Attachment: v1

Part of this is based on the SendCredentialsController introduced in bug 675823.

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 563591 [details] [diff] [review]
v1

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

::: browser/base/content/syncAddDevice.js
@@ +123,5 @@
>          delete self._jpakeclient;
>          self.wizard.pageIndex = DEVICE_CONNECTED_PAGE;
> +
> +        // Schedule a Sync for soonish to fetch the data uploaded by the
> +        // device we just paired with.

I would prefer "to fetch the data uploaded by the device with which we just paired", but then I'm a grammar nazi.

Comment 7

[Philipp von Weitershausen [:philikon]]

Attachment: v2

Addresses nit and also adds a test.

Comment 8

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/services/services-central/rev/c44a046d72b4

Comment 9

[Philipp von Weitershausen [:philikon]]

STRs for QA:

* Pair a new device (A) with an already Sync-connected device (B)
* B should trigger a Sync with the "active interval" (by default 5 minutes), regardless of what its regular interval would be (could be 24h or 1h if it was the only device up until then.)

Comment 10

[Mihaela Velimiroviciu (:mihaelav)]

Build identifier: Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111003 Firefox/10.0a1
BuildID: 20111003153830

Connected a new device (A) to a device that already had Sync configured (B); a sync was performed in B after about 1-2 minutes.

Comment 11

[Philipp von Weitershausen [:philikon]]

(In reply to Mihaela Velimiroviciu [QA] from comment #10)
> Connected a new device (A) to a device that already had Sync configured (B);
> a sync was performed in B after about 1-2 minutes.

Was B the only device connected to the account? Because B should wait for about 5 minutes before syncing again, so if it syncs earlier than that, something else is going on.

Comment 12

[Tracy Walker [:tracy]]

So far, what I've seen is a sync is fired immediately.  That sets the interval to singleDevice 3600.  But it actually fires a sync five minutes later. I'm still investigating.

 
I also saw something bad adding an XP client to account. trying to reproduce it.

Comment 13

[Philipp von Weitershausen [:philikon]]

(In reply to Tracy Walker [:tracy] from comment #12)
> So far, what I've seen is a sync is fired immediately.

Do you mean bug 688520 or is it actually kicking off a sync immediately after the pairing?

Comment 14

[Tracy Walker [:tracy]]

Ah yes, that's what it is.  nevermind.

I'm also not able to reproduce the issue on XP I was looking into.  I think it may have been miss timing of account deletion and recycling.  everything is fine with clean accounts

Comment 15

[Philipp von Weitershausen [:philikon]]

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

Comment 16

[Philipp von Weitershausen [:philikon]]

This bug essentially makes bringing back heartbeats (bug 693758) superfluous. It is a much smaller patch, has less risk, and avoids many unnecessary network requests. I am therefore nominating this instead of bug 693758 to go into Aurora and Beta to avoid poor user experience introduced by increasing the sync intervals (bug 694149).

Comment 17

[Philipp von Weitershausen [:philikon]]

Comment on attachment 564006 [details] [diff] [review]
v2

Nominating this patch for Aurora and Beta. Please see comment 16 for justification.

Comment 18

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/32fc4eb5feec

Comment 19

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e31583927cd7

Comment 20

[Tracy Walker [:tracy]]

verified across channels

note to self:  watch value of services.sync.nextSync to verify this.  (don't look for a log)