1091161 - Separate gInitializeTimerFunc from the actual initialize callback so we can retry initialization on demand

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Matthew N. [:MattN]]

Attachment: v.1 patch

This moves the initialize callback function called after the timer to its own function so if we want to retry a failed initialization, we can just call that new function instead of waiting for the timer again if we called gInitializeTimerFunc. This is needed for the retry button in bug 1074670.

More work may be needed in a follow-up to make sure delayedInitialize can be retried successfully to resolve failures at any point in initialization.

Other cleanup:
* flattened MozLoopService.register to just call MozLoopServiceInternal.promiseRegisteredWithServers since the pref checks didn't seemed to just be duplicated without a good reason I could see. I also renamed it to promiseRegisteredWithServers to reduce the number of function names which added confusion.
* switched to Timer.jsm to get rid of the timer global

All tests pass for me.

Comment 1

[Maire Reavy [:mreavy]]

Needed for bug 1074670

Comment 2

[Paul Kerr [:pkerr]]

Comment on attachment 8513721 [details] [diff] [review]
v.1 patch

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

::: browser/components/loop/MozLoopService.jsm
@@ +947,5 @@
> +   * Can be called more than once (e.g. if the initial setup fails at some phase).
> +   * @param {Deferred} deferredInitialization
> +   */
> +  delayedInitialize: Task.async(function*(deferredInitialization) {
> +    yield this.promiseRegisteredWithServers().then(Task.async(function*() {

The logic flow, I think, is correct. Is there a reason you cannot directly use the yield statements to step through the asynchronous promise returning calls without the nested then() calls?

Comment 3

[Matthew N. [:MattN]]

(In reply to Paul Kerr [:pkerr] from comment #2)
> The logic flow, I think, is correct. Is there a reason you cannot directly
> use the yield statements to step through the asynchronous promise returning
> calls without the nested then() calls?

The logic flow was the same as before for the most part. I had tried to get rid of the the nesting the other day but ran into an issue. I tried again after your review comment and now it looks much nicer. Good call!

Pushed: https://hg.mozilla.org/integration/fx-team/rev/0b4f5a605288

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0b4f5a605288

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/989937667860

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8513721 [details] [diff] [review]
v.1 patch

Approval Request Comment
Landed on aurora per IRC with lsblakk with a=loop-only