1889785 - Show What's New Page when the relevant update was installed by a different profile

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Robin Steuber (she/her) [:bytesized]]

I haven't actually tested this, but I believe that the What's New Page won't be shown if the update was installed by a different profile (this includes using the Background Update Task). I think that we rely on being able to pull an update object out of the update manager to show the What's New Page. But that object won't be present if _postUpdateProcessing has run since the update was installed.

We should verify that this is actually a problem before we spend time fixing it, but I'd bet that it is.

Comment 1

[Nick Rishel [:nrishel]]

I believe the STR for this would be:

1. Download an outdated Firefox.
2. Disable internet access.
3. Install and run Firefox (to clear first startup flows).
4. Close Firefox.
5. Enable internet access.
6. Run background updater in Task Scheduler manually 3 times (located under Task Scheduler Library > Mozillla > Firefox Background Update #).
7. Run Firefox.

Expected result: What's New page shows.

Actual result (if this bug is present): No What's new page shows.

Comment 2

[Robin Steuber (she/her) [:bytesized]]

I think that step 6 may need to involve running the background update task multiple times (maybe 3, to be safe). Otherwise it may not fully finish installing the update.

Comment 3

[Nick Rishel [:nrishel]]

Updated the STR, thanks!

Comment 4

[Monica Chiorean]

I was able to reproduce it on Win11x64 using different builds from 118/122 updated to 124.0.2. Last time I reproduce it using FF build 120.0, but there are multiple scenarios where FF is not updated after step7, only after a second restart and whats new page is displayed.

Comment 5

[Eric Chen]

I have a question about the following code in _postUpdateProcessing. I understand that cleanupReadyUpdate() will be called after a successful update to remove the update object. So how do we ensure the "What's New Page" is shown before the object is removed, in the context of a regular update? (meaning just the same profile updating and opening Firefox)

Comment 6

[Robin Steuber (she/her) [:bytesized]]

(In reply to Eric Chen from comment #5)

I have a question about the following code in _postUpdateProcessing. I understand that cleanupReadyUpdate() will be called after a successful update to remove the update object. So how do we ensure the "What's New Page" is shown before the object is removed, in the context of a regular update? (meaning just the same profile updating and opening Firefox)

Ugh. The answer is that this is currently a horrifying mess relying on the fairly arbitrary order that things happen in at startup. I'm in the middle of fixing it here.

Comment 7

[Eric Chen]

Attachment: Bug 1889785 - Ensure What's New Page is displayed when the relevant update is installed by a different profile r=bytesized!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16b8d7d77d7d
Ensure What's New Page is displayed when the relevant update is installed by a different profile r=bytesized,application-update-reviewers,firefox-desktop-core-reviewers

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/16b8d7d77d7d

Comment 10

[Daniel Bodea [:danibodea]]

While attempting to confirm this fix, I hit the same problem mentioned by Monica in comment 4, namely that the background updater may not in fact update the browser in the background and that may result in relaunching the same version in step 7 (originally installed in step 3). More exactly, if the user opens the Task Scheduler and runs the Firefox Background Update task manually and then refreshes the list of tasks to check if the status changes, he might indefinitely see the message "The task is currently running. (0x41301)" or even simply "(0x1)". In this situation, I waited for the task to complete for at least 30 minutes, but the status would not change to "Completed successfully" so I was forced to re-run the task multiple times until the right message is displayed: "The operation completed successfully. (0x0)". Even after all these extra steps, the browser may STILL not be updated.

I have run the original steps and the extra ones mentioned above in an attempt to reproduce this issue by installing Release v127.0 and after many tries I managed to update it in the background to Release v128.0.3 and the "What's New Page" was not displayed, successfully reproducing the reported issue.

I've attempted to confirm this fix by installing Beta v127.0 (RC) and updating to the latest v129.0b9, but the What's New Page would still not be displayed. I've also attempted to reproduce it in the Nightly channel by installing Nightly v129.0a1 updating in the background to Nightly v130.0a1, but the What's New Page would still not be displayed.

This being said, I believe that the original steps to reproduce are not consistent enough to reproduce or verify a fix and/or the Firefox Background Update task is not working as expected. Even after 5 tries in each channel, I could not get the What's New Page to be displayed after a background update. This was tested in Windows 10 and 11.

Eric Chen, could you help us address this situation further by answering some questions:

1. Is there a way to improve the steps to reproduce? Could you give us some tips on how to use them more efficiently?
2. Can you test the fix yourself to confirm whether it works on your system?
3. Are there any third-party applications that could influence the result of this test?
4. When exactly should the What's New Page be displayed? Are there any exceptions we should know about?
   Thank you!

Comment 11

[Eric Chen]

Hi Daniel, thanks for letting me know! I'm currently investigating, I'm just wondering when is the due date for verifying this patch, since 129 is launching in a week.

Comment 12

[Daniel Bodea [:danibodea]]

I believe it would be best to verify the report by Friday, August 2nd, since the RC2 129.0 should be signed off by then, however, I'm not sure what the deadline would be for another uplift.

Comment 13

[Eric Chen]

Hi Daniel, I was able to reproduce the issue by updating 129 Nightly to 130 Nightly, however, I did not run into the issue where the background update task takes a long time to complete. It completed in about ~5 seconds for me (I tried 3 times).
I'm going to continue investigate why the WNP is not shown after the background update

Comment 14

[Eric Chen]

Hi Robin, I heavily suspect that UpdateManager.lastUpdateInstalled.platformVersion is null during the startup after a background update (it is null in the browser console), causing this block (https://searchfox.org/mozilla-central/source/browser/components/BrowserContentHandler.sys.mjs#759-769) to evaluate to true and wipe the update, and thus the WNP is not shown.

When I try updating with a regular Firefox restart, the WNP is shown and UpdateManager.lastUpdateInstalled.platformVersion reads the correct value.

Furthermore, I outputted the logs after a background update, and it had: AUS:SVC UpdateManager:_loadXMLFileIntoArray - XML file does not exist. path: C:\ProgramData\Mozilla...\updates\6F193CCC56814779\active-update.xml, so I believe the code that sets the platform version will not be reached.

Does this seem like a reasonable cause of the patch not working?

Comment 15

[Robin Steuber (she/her) [:bytesized]]

Well we don't necessarily need active-update.xml to exist, but we need either active-update.xml or updates.xml. If active-update.xml is missing, it should just mean that _postUpdateProcessing was already run by the background updater, which I believe is what ought to happen normally.

You are saying that UpdateManager.lastUpdateInstalled is non-null though but platformVersion is null? That is quite curious. You should turn app.update.background.loglevel up to a higher level like debug in the default profile and collect a log from the update directory (listed in about:support), where it will be named backgroundupdate.moz_log. That should give a better idea of what's going on.

To be clear though, are you using the STR in Comment 1?

Comment 16

[Eric Chen]

Yeah, I'm using the STR from comment 1.

I believe that after a background update, lastUpdateInstalled is retrieved from this._getUpdates() since updateInstalledAtStartup will be wiped at that point. this._getUpdates() reads updates.xml which is written here by .saveUpdates().

The issue is that .saveUpdates() writes to the XML file the attributes of readyUpdate , which I don't think has a platformVersion value, we only set that for updateInstalledAtStartup. And this causes the platformVersion value to be missing when we read the value of lastUpdateInstalled during startup.

I checked the update.xml file and platformVersion is indeed missing from the XML data.

Sorry if this is a bit lengthy - do you think this makes sense? A potential fix would be to set platformVersion for readyUpdate as well.

Comment 17

[Robin Steuber (she/her) [:bytesized]]

(In reply to Eric Chen from comment #16)

I checked the update.xml file and platformVersion is indeed missing from the XML data.

Alright, yeah, that is clearly the problem. So the way forward is figuring out why the background update task isn't writing that out.

A potential fix would be to set platformVersion for readyUpdate as well.

This is the lightly trimmed version of the code that sets the platformVersion:

this.#updateInstalledAtStartup = this._readyUpdate;
this.#updateInstalledAtStartup.platformVersion = Services.appinfo.platformVersion;
this.saveUpdates();

They are the same object. We only assigned the _readyUpdate object to #updateInstalledAtStartup , we didn't make a clone of it or anything. Setting platformVersion on one should be the same as setting it on both.

---

Oh, I left out a step before when I was talking about looking at backgroundupdate.moz_log. You should also enable app.update.log as well so that you get the messages from UpdateService. You should probably also add some additional LOG calls so that when you look at the log, there is more detail around #updateInstalledAtStartup.platformVersion being set and saved. You may also want to put some log messages in _saveUpdatesXML. It's called asynchronously, so I guess it's conceivable it isn't being run and the updates never get saved back to the disk, despite our saveUpdates() call. I would be pretty surprised though if we were consistently, successfully removing active-update.xml and then failing to write out updates.xml, right after. And since _saveUpdatesXML is delayed, these should happen at the same time, even though the code that would make the platformVersion change is in a completely separate place from the code that would remove active-update.xml.

Alternately, you could connect a debugger to the task. Though, this could be a timing problem. Which could be a little tricky to pin down in the debugger.

Let me explain the timing problem I'm considering. I don't like it very much because it seems like the effects would be inconsistent, whereas it sounds like you are experiencing this consistently. But we do not currently do a great job of properly, like, making sure the update service has fully completed writing everything to the disk before the background update task exits. The timeout on beginning to write out data is 200ms and the horrifying delay to accommodate that is 500ms. Maybe try increasing the 500ms to something way more than necessary (and making sure to wait this long when you run the background update task) and see if that causes things to be written out properly? I don't know. I have my doubts about how plausible this answer is, but I'm not sure what else it could be without debugging it.