Closed Bug 1301517 Opened 3 years ago Closed 2 years ago

Cleanup the setting of post update values added by bug 1301288

Categories

(Toolkit :: Application Update, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox55 --- wontfix
firefox56 --- fixed

People

(Reporter: rstrong, Assigned: rstrong)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qf])

Attachments

(1 file, 1 obsolete file)

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.
Depends on: 1301288
Priority: -- → P4
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?
Whiteboard: [qf]
Here is a startup profile where this takes 114ms: https://perfht.ml/2qIAsMp
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.
Attached patch patch rev1 (obsolete) — Splinter Review
Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=395b4688bb79002afe664693b13c8b31f8405d35
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
Attached patch patch rev2Splinter Review
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.
Attachment #8871095 - Attachment is obsolete: true
Attachment #8871101 - Flags: review?(mhowell)
I forgot to mention that I also manually verified that this does the right thing with the what's new page.
Attachment #8871101 - Flags: review?(mhowell) → review+
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ac40f983b13
Cleanup the setting of post update values added by bug 1301288. r=mhowell
https://hg.mozilla.org/mozilla-central/rev/7ac40f983b13
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
This was backed out of 55 in bug 1386224
Target Milestone: mozilla55 → mozilla56
You need to log in before you can comment on or make changes to this bug.