746754 - Show prompts when checking for updates and downloading updates

Status: RESOLVED INVALID

(Firefox OS Graveyard :: General, defect)

Description

[Jim Straus]

Bug 740720 shows a prompt before applying an update.  This bug adds in the other possible prompts.

Comment 1

[Jim Straus]

Attachment: Adding additional update prompts

The additional changes.  Also there is a pull request for the gaia changes.

Comment 2

[[:fabrice] Fabrice Desré]

Jim, can you update this patch? I'll have some time to review

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 616763 [details] [diff] [review]
Adding additional update prompts

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

::: b2g/components/UpdatePrompt.js
@@ +40,5 @@
> +    browser.content.dispatchEvent(evt);
> +  },
> +
> +  showUpdateAvailable: function UP_showUpdateAvailable(aUpdate) {
> +    gUpdate = aUpdate;

Do not use gUpdate (See review of bug 740720)

@@ +48,5 @@
> +    let evt = browser.content.document.createEvent("CustomEvent");
> +    evt.initCustomEvent("mozChromeEvent", true, true,
> +                        {"type": "updates-check"});
> +    browser.content.dispatchEvent(evt);
> +  },

checkForUpdates seems duplicated?

@@ +78,4 @@
>  
>    handleEvent: function (e) {
>      let browser = Services.wm.getMostRecentWindow("navigator:browser");
>      browser.content.removeEventListener("mozContentEvent", this.handleEvent);

You need to have a unique id associated to your mozChromeEvent to be sure to not react on all mozContentEvent. Have somewhere a method to generate a uuid and use it to make sure the message you receive is the message you expect.

@@ +85,5 @@
>        gUpdate = null;
> +    } else if (e.detail.type == "download-update") {
> +      this.downloadUpdate(gUpdate);
> +      gUpdate = null;
> +    } else if (e.detail.type == "check-for-update") {

Where does this message comes from? Is it initate by the content? If so your code does not listen for mozContentEvent in a permanent fashion so I don't understand how this code works.

@@ +87,5 @@
> +      this.downloadUpdate(gUpdate);
> +      gUpdate = null;
> +    } else if (e.detail.type == "check-for-update") {
> +      this.checkNowForUpdates();
> +      gUpdate = null;

switch(e.detail.type) {
  case "updates-now":
    ...
    break;
  case "download-update":
    ...
    break;
   ...
}

Comment 4

[Marshall Culpepper [:marshall_law]]

See tracking bug 737598 to follow update prompt and interaction bugs, this one is worded a little vaguely.