update badge shown after fallback to complete update (Tor 19411)

RESOLVED WORKSFORME

Status

()

defect
P3
normal
RESOLVED WORKSFORME
3 years ago
2 months ago

People

(Reporter: mcs, Unassigned)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tor][tor-standalone])

Attachments

(1 attachment)

If a partial update fails to apply and the update falls back to downloading a full update, the update badge is displayed when it should not be (with the corresponding "Restart to Update" text inside the hamburger menu).

Steps to reproduce:
1. Download and install a Firefox that is slightly out of date.
2. Artificially modify a file that is usually updated between releases so that a partial update is very likely to fail. I re-zipped the top-level omni.ja file.
3. Ensure that app.update.badge == true.
4. Trigger an update check, e.g., open "About Firefox"

Expected Result:
No badge will be shown until the update is properly staged and the browser is ready for restart.

Actual Result:
The badge is displayed as soon as the partial update fails (while the complete MAR file is still being downloaded).

Here is the corresponding Tor Browser ticket:
https://trac.torproject.org/projects/tor/ticket/19411
Posted patch proposed fixSplinter Review
Here is the fix we applied to Tor Browser. There may also be other cases that this code does not handle correctly.
Whiteboard: [tor]
Comment on attachment 8763231 [details] [diff] [review]
proposed fix

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

Not sure if this shouldn't just check for anything not in a particular list of known-good states. I dug around in the updater code for a bit but it was hairy (seems we generally send two observer notifications in short succession for which we both listen?) so I think it's likely best if :rstrong or someone else more familiar with the updater code reviews this.
Attachment #8763231 - Flags: review?(robert.strong.bugs)
Stephen, I tried to reproduce on Windows without success... could you check why the observer was notified on your Mac? It doesn't look like it should be and it doesn't appear to be on Windows. You should be able to just log the parameters here
https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#2631

with
Services.console.logStringMessage("*** topic: " + topic + ", status: " + status);

Then change app.update.url.override to the update url for the previous days build, change app.update.staging to false, and try to update.
I assumed that this happened on startup when it could be for the staging phase. It would be good to check with app.update.staging set to true as well.
I think I was able to reproduce this when staging.
Mark, can you verify whether this was seen before or after restarting? Thanks!
Flags: needinfo?(mcs)
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #6)
> Mark, can you verify whether this was seen before or after restarting?

Before restarting. More precisely: "The badge is displayed as soon as the partial update fails (while the complete MAR file is still being downloaded)."
Flags: needinfo?(mcs)
Thanks for the info Mark. I'm going to run a few tests locally before reviewing this and I had some other work take priority. I hope to have this done by Friday.
Priority: -- → P3
Blocks: meta_tor
Robert, is there anything you still need from us? We really like to get this into ESR 52.
Flags: needinfo?(robert.strong.bugs)
Whiteboard: [tor] → [tor][tor-standalone]
I've had a couple of other things I've had to finish up and the staging code has been changed recently so this needs to be checked again. If I'm not able to get this landed in time for the nightly to aurora merge I'll request that it is uplifted.
Flags: needinfo?(robert.strong.bugs)
Summary: update badge shown after fallback to complete update → update badge shown after fallback to complete update (Tor 19411)
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #10)
> I've had a couple of other things I've had to finish up and the staging code
> has been changed recently so this needs to be checked again. If I'm not able
> to get this landed in time for the nightly to aurora merge I'll request that
> it is uplifted.

Thanks. Where are we with that one right now? Anything we can help with given that even the aurora merge window is closing soon?
Flags: needinfo?(robert.strong.bugs)
I would like rstrong to confirm, but my reading of the code that implements the doorhanger-based UI tells me that this bug has been fixed. Updating the badge is now done in a different way by new code. Also, I could not reproduce the original problem using a Firefox 60-based browser.

Robert: I guess the answer to my questions in comment:11 is that we are done here? Can you confirm that?

Definitely should be working now.

Status: NEW → RESOLVED
Closed: 4 months ago
Flags: needinfo?(robert.strong.bugs)
Resolution: --- → WORKSFORME
Attachment #8763231 - Flags: review?(robert.strong.bugs)
You need to log in before you can comment on or make changes to this bug.