Closed Bug 787383 Opened 7 years ago Closed 7 years ago

B2G Updates: Show update download progress

Categories

(Toolkit :: Application Update, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18
blocking-basecamp +

People

(Reporter: marshall, Assigned: marshall)

References

Details

Attachments

(1 file, 1 obsolete file)

This will mainly be forwarding of update progress messages through mozChromeEvents initially
Attached patch update progress - v1 (obsolete) — Splinter Review
Attachment #657263 - Flags: review?(fabrice)
Comment on attachment 657263 [details] [diff] [review]
update progress - v1

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

::: b2g/components/UpdatePrompt.js
@@ +71,5 @@
>      // a prompt for some reason
>      this._applyPromptTimer = this.createTimer(APPLY_PROMPT_TIMEOUT);
>    },
>  
> +  showUpdateError: function UP_showUpdateError(update) {

s/update/aUpdate (fix everywhere else also)

::: toolkit/mozapps/update/nsUpdateService.js
@@ +3117,5 @@
>                                               PREF_APP_UPDATE_BACKGROUND_INTERVAL,
>                                               DOWNLOAD_BACKGROUND_INTERVAL)
>                                     : DOWNLOAD_FOREGROUND_INTERVAL;
>      this._request.init(uri, patchFile, DOWNLOAD_CHUNK_SIZE, interval);
> +    this._request.start(this, update);

Do we really need this change? Neither firefox desktop neither Firefox on Android need that, and I don't think we have different needs.
Attachment #657263 - Flags: review?(fabrice) → review-
(In reply to Fabrice Desré [:fabrice] from comment #2)
> ::: toolkit/mozapps/update/nsUpdateService.js
> @@ +3117,5 @@
> >                                               PREF_APP_UPDATE_BACKGROUND_INTERVAL,
> >                                               DOWNLOAD_BACKGROUND_INTERVAL)
> >                                     : DOWNLOAD_FOREGROUND_INTERVAL;
> >      this._request.init(uri, patchFile, DOWNLOAD_CHUNK_SIZE, interval);
> > +    this._request.start(this, update);
> 
> Do we really need this change? Neither firefox desktop neither Firefox on
> Android need that, and I don't think we have different needs.

You're right -- it isn't strictly necessary (I'm probably wrapping too much metadata into each update mozChromeEvent). I'll remove this in my next patch, and simplify the logic in sendUpdateEvent while I'm at it.
review comments addressed, split sendUpdateEvent into multiple functions for
clarity.
Attachment #657263 - Attachment is obsolete: true
Attachment #658920 - Flags: review?(fabrice)
Attachment #658920 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/mozilla-central/rev/2d2dced396c0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
blocking-basecamp: ? → +
You need to log in before you can comment on or make changes to this bug.