1301517 - Cleanup the setting of post update values added by bug 1301288

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P4)

Description

[Robert Strong (they/them - no direct email)]

In bug 1301288 I went with a simple fix since the patch in that bug will likely need to be uplifted. A more complex fix was attempted but the test failures showed that there was an undesired change in behavior. The code should perform all update post processing that doesn't need to display a user interface in post-update-processing and it should display user interface in the following observe cases
      case "sessionstore-windows-restored":
      case "mail-startup-done":
      case "xul-window-visible":

as well as when those observe cases are not applicable.

Comment 1

[Florian Quèze [:florian]]

I don't understand why deciding to show a piece of UI involves writing a file to disk during startup, but this causes main thread IO during the first startup after an update (ie. it will impact the 57 first run experience for everybody). Can we get this fixed soon?

Comment 2

[Florian Quèze [:florian]]

Here is a startup profile where this takes 114ms: https://perfht.ml/2qIAsMp

Comment 3

[Robert Strong (they/them - no direct email)]

Good point and I will se what can be done.

You might also want to also get into the discussion in the release drivers mailing list about the What's New page that is displayed to everyone on update which has a MUCH bigger impact that this and other bugs.

Comment 4

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=395b4688bb79002afe664693b13c8b31f8405d35

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: patch rev2

Matt, this will keep more of an individual update's history which is something I've been wanting to do for some time now. With these changes the latest update in updates.xml is always the latest update even before applying the update. This makes it so that after a successful update the code in nsBrowserContentHandler.js will always get the correct custom update property without the hack added in bug 1301288.

I'll verify the try build is in good shape before landing.

Comment 6

[Robert Strong (they/them - no direct email)]

I forgot to mention that I also manually verified that this does the right thing with the what's new page.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ac40f983b13
Cleanup the setting of post update values added by bug 1301288. r=mhowell

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7ac40f983b13

Comment 9

[Robert Strong (they/them - no direct email)]

This was backed out of 55 in bug 1386224