1146068 - Update reading list scheduler to match requirements of the sync engine

Status: RESOLVED FIXED

(Firefox Graveyard :: Reading List, defect)

Description

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

Attachment: 0005-fix-scheduler-to-match-readinglist-sync-engine-imple.patch

Now we have a real RL sync engine, the scheduler needs some tweaks.

Drew, you gave r+-over-the-shoulder of part of this, but I added some other things, in particular, that if an item is added/changed/etc while a sync is in progress we start a new sync immediately after the existing one completes - it may have missed that change and thus may be a long time before it is actually synced.  This includes a test. We also had a preference for "how long after the RL is dirty until we sync" with a default of 2 mins, but talking with rnewman we decided this should be immediate - there's a good chance the user might add an item then shut their laptop - so that's been removed too.

Comment 1

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

Attachment: 0006-fix-log-manager-to-automatically-set-log-level-for-a.patch

Here are the log manager fixes we discussed - these are almost identical to what you have in the sync patch, but I also clarified an unrelated comment as I found it difficult to read.  I figure we may as well keep it in this bug as it is part of the scheduler if you squint enough (ie, we rely on the scheduler to take care of RL logs)

Comment 2

[Marco Mucci [:MarcoM]]

Hi Mark, can you provide a point value.

Comment 3

[Drew Willcoxon :adw]

Comment on attachment 8581217 [details] [diff] [review]
0005-fix-scheduler-to-match-readinglist-sync-engine-imple.patch

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

::: browser/components/readinglist/Scheduler.jsm
@@ +122,5 @@
>      this._setupTimer();
>    },
>  
> +  _setupRLListener() {
> +    let maybeSync = () => {

Just a nit, but this doesn't have to be inside _setupRLListener.  IMO it should be a method on the scheduler.  Just a suggestion, feel free to leave it as is.

Comment 4

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

https://hg.mozilla.org/integration/fx-team/rev/c12a9bd2cc2b
https://hg.mozilla.org/integration/fx-team/rev/497bcc0a0143

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c12a9bd2cc2b
https://hg.mozilla.org/mozilla-central/rev/497bcc0a0143

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d93c3c3ac322
https://hg.mozilla.org/releases/mozilla-aurora/rev/20d4b2b21a8f