platform_packages_apps_Updater icon indicating copy to clipboard operation
platform_packages_apps_Updater copied to clipboard

Alert User of Pending Reboot When Manual Update Check Is Blocked

Open haok1402 opened this issue 8 months ago • 20 comments

Description:

This PR improves the system updater by notifying users when they attempt a manual update check while an update is already prepared and awaiting a reboot. Instead of ignoring the user’s tap on the “Check for updates” button, the system now displays an alert, prompting the user to reboot.

Due to the critical nature of System Updater, this proposed change is the smallest imagined code change. If desired, it could be expanded on. For example, it would be possible to store the build number of the pending update and include that in the displayed alert.

Changes:

  • Previously, if KEY_WAITING_FOR_REBOOT was true inside the clickListener, the update check was ignored without notifying the user.
  • Now, an alert dialog is shown to inform the user that a reboot is required before another update check can be performed.

Testing:

Tested on Pixel 6a, 2025032100 release to ensure:

  • Update check works normally when no reboot is pending.
  • Alert dialog correctly appears when a reboot is required.
  • No regressions or unexpected behavior.

Screenshot from the testing can be found here.

Expected Behavior:

When the user checks for updates:

  • If no reboot is pending, the system initiates the update check.
  • If a reboot is pending, an alert dialog informs the user before proceeding.

I am happy to revise and resubmit the code if any changes are desired.

Resolves #101

haok1402 avatar Apr 08 '25 01:04 haok1402

@haok1402 It would be very helpful to prevent fully disabling System Updater notifications and reboot notifications while still allowing explicitly disabling the others (disabling errors is sketchy but it would need to support automatically retrying before reporting errors at least for automatic updates prior to prevent disabling it). This is done for some other system app notifications but these are easily the most important and should be done for it too. It seems many users try to disable the Already up-to-date notifications or the progress notification and accidentally end up disabling all of them instead.

thestinger avatar May 15 '25 00:05 thestinger

@thestinger I believe @haok1402 is traveling and may not be able to respond for a few days. Meanwhile, is it easy to indicate one of the system apps that currently prevents notification disabling?

de0u avatar May 15 '25 02:05 de0u

It's done for GmsCompat. I'm not sure exactly where. I think there are cases where overall notifications can't be disabled but specific channels can be disabled.

thestinger avatar May 15 '25 02:05 thestinger

@thestinger Thanks for the info! I'm interested and will start looking into it.

haok1402 avatar May 19 '25 03:05 haok1402

I've investigated how GmsCompat handles this and confirmed that it uses fixed notifications, with certain channels marked as blockable — users can't disable notifications entirely but a few specific channels. Based on that, I plan to make and test similar changes to System Updater: mark its notifications as fixed and set the reboot channel as non-blockable.

@thestinger Would you prefer these changes as part of this PR (to retain the current discussion/context), or submitted in a new PR that references this one?

haok1402 avatar May 20 '25 20:05 haok1402

Just as a new fully separate PR.

thestinger avatar May 20 '25 20:05 thestinger

I have an implementation that seems to work on the old release. Planning to test on 2025052000 and submit the PR in the next few days.

haok1402 avatar May 27 '25 22:05 haok1402

This might still be useful since I think there are still ways to clear away the reboot notification despite it being marked persistent, the permission being marked fixed and the channel not being opted into being blockable.

thestinger avatar Jun 01 '25 00:06 thestinger

@haok1402 Can you rebase it instead of merging? git rebase upstream/16

thestinger avatar Jul 23 '25 16:07 thestinger

@thestinger Yes, fixing it.

haok1402 avatar Jul 23 '25 16:07 haok1402

@haok1402 Needs to be rebased again.

thestinger avatar Oct 25 '25 21:10 thestinger

@thestinger Sorry, my bad; I thought the GitHub button can do the trick; Let me pull down and run the rebase.

haok1402 avatar Oct 25 '25 21:10 haok1402

@thestinger I believe @haok1402 will also be re-testing. If either of you can flip the PR back to "draft", that might be appropriate?

de0u avatar Oct 25 '25 21:10 de0u

@de0u Yes; I flipped it back to draft. #105 was flipped to draft as well; also need some re-testeing on that.

haok1402 avatar Oct 25 '25 21:10 haok1402

We did successfully ship a variant of @haok1402's earlier changes making the notification channels fixed, so that's good. I considered that a lot more important. We also got rid of the 3 day timeout introduced with Android 15 QPR1. There are still some other issues such as what happens if an update check occurs in early boot after an existing update is having post-boot code run, which should just delay doing the new update until that finishes.

thestinger avatar Oct 25 '25 21:10 thestinger

@thestinger Two questions...

  1. Given that the problem of users accidentally disabling/disabling "please reboot" notifications was solved "the other way" in PR #107 and PR #118, is this PR (to encourage a user to reboot if an update is pending) still wanted? If so I believe @haok1402 is willing to rebase it and retest it. Or would you prefer to just leave it open for now in case at some later point it is thought to be useful?

  2. Are you interested in a new PR to address cases where an automatic check or even possibly a manual check might happen too early, such as right after a reboot while optimization is still running from a previous update? If so, do you want me to file an issue for that? I believe @haok1402 is willing to work on that too (after PR #105 is sorted out, I suspect).

de0u avatar Oct 28 '25 20:10 de0u

@de0u This would still be useful because the notification can still be lost including via Do Not Disturb.

thestinger avatar Oct 28 '25 20:10 thestinger

It would probably good if the app had a generic status display in the settings.

thestinger avatar Oct 28 '25 20:10 thestinger

Are you interested in a new PR to address cases where an automatic check or even possibly a manual check might happen too early, such as right after a reboot while optimization is still running from a previous update?

Yes, but note it's not the app optimization which is relevant. The app optimization is our own thing unrelated to update_engine. The issue is that it seems update_engine does some post-update work which can sometimes take longer than it used to and opens up a real window where weird behavior can happen because our app both gets an error but also gets progress updates and then thinks the update finished when the post-update stuff from what it was already doing is done.

thestinger avatar Oct 28 '25 20:10 thestinger

In that case, it asks people to reboot to update, when it's for the last update which was doing post-boot work, and it has deleted the downloaded update from the error. After people reboot, it has to do it again.

thestinger avatar Oct 28 '25 20:10 thestinger