Last Comment Bug 678586 - Trigger a delayed sync after pairing a new device
: Trigger a delayed sync after pairing a new device
Status: VERIFIED FIXED
[verified in services]
: verified-aurora, verified-beta
Product: Cloud Services
Classification: Client Software
Component: Firefox Sync: Backend (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla10
Assigned To: Philipp von Weitershausen [:philikon]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-12 12:21 PDT by Philipp von Weitershausen [:philikon]
Modified: 2011-11-03 11:48 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
v1 (2.05 KB, patch)
2011-09-29 16:24 PDT, Philipp von Weitershausen [:philikon]
rnewman: review+
Details | Diff | Splinter Review
v2 (3.11 KB, patch)
2011-10-01 16:50 PDT, Philipp von Weitershausen [:philikon]
asa: approval‑mozilla‑aurora+
asa: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Philipp von Weitershausen [:philikon] 2011-08-12 12:21:43 PDT
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] 2011-08-12 12:24:03 PDT
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] 2011-08-12 14:24:31 PDT
(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] 2011-08-12 14:41:51 PDT
(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] 2011-09-29 15:54:33 PDT
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] 2011-09-29 16:24:07 PDT
Created attachment 563591 [details] [diff] [review]
v1

Part of this is based on the SendCredentialsController introduced in bug 675823.
Comment 6 Richard Newman [:rnewman] 2011-09-29 22:16:33 PDT
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] 2011-10-01 16:50:22 PDT
Created attachment 564006 [details] [diff] [review]
v2

Addresses nit and also adds a test.
Comment 8 Philipp von Weitershausen [:philikon] 2011-10-02 01:25:40 PDT
https://hg.mozilla.org/services/services-central/rev/c44a046d72b4
Comment 9 Philipp von Weitershausen [:philikon] 2011-10-03 18:15:49 PDT
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) 2011-10-04 08:21:11 PDT
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] 2011-10-04 09:23:51 PDT
(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] 2011-10-04 10:08:45 PDT
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] 2011-10-04 10:15:01 PDT
(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] 2011-10-04 10:18:56 PDT
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] 2011-10-04 12:57:36 PDT
https://hg.mozilla.org/mozilla-central/rev/c44a046d72b4
Comment 16 Philipp von Weitershausen [:philikon] 2011-10-13 15:50:42 PDT
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] 2011-10-13 15:52:04 PDT
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] 2011-10-18 20:12:53 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/32fc4eb5feec
Comment 19 Philipp von Weitershausen [:philikon] 2011-10-18 20:36:41 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/e31583927cd7
Comment 20 Tracy Walker [:tracy] 2011-11-03 11:48:11 PDT
verified across channels

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

Note You need to log in before you can comment on or make changes to this bug.