Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebase Omaha (=updater on Windows) to v1.3.36.111 #11096

Merged
merged 17 commits into from
Nov 25, 2021

Conversation

mherrmann
Copy link
Collaborator

@mherrmann mherrmann commented Nov 15, 2021

Resolves brave/brave-browser#11904. Further technical discussion in brave/omaha#36.

Overview

Omaha is Brave's auto-update technology on Windows. It is a separate application. Brave ships it as part of its installers and integrates with it in various ways so (for example) users can navigate to brave://settings/help to check for updates.

This PR updates Omaha to the latest upstream version. This affects all of Brave's installers and update features on Windows. Testing is highly important because a problem in the updater could brick the entire update fleet.

Roll-out strategy

Because Brave and Omaha are distinct applications, we can release them separately. Specifically: Brave's installers do include the updater (=Omaha). But when we simply upload a new version of Brave to the update server, then users do not automatically receive the new version of the updater. We might use this to limit the risk of this PR: If we only release new installers and upload Brave but not the updater to the update server, then only new users receive the new updater. This lets us test on a much smaller install base whether the new updater works as expected. Once we are confident that this is the case, we can then also upload the new updater to the update server. Then, the entire Windows user base will receive it.

The drawback of the approach described in the previous paragraph is that we need to make sure that new versions of Brave also work when the old updater is installed on the users' system. As this PR shows, the changes to integrate the new updater are relatively small. Those changes that are there should be backwards-compatible. So I believe it is a good idea to release the updater later.

An additional strategy to limit the roll-out risk can be to use the update server's "partial updates" feature. This lets us roll out new versions to a fraction of the update fleet. Given Brave's rapid release schedule, my suspicion is that it will not be feasible to roll out Brave and the new updater gradually at the same time. But it might be beneficial to begin as described in the previous paragraph, then roll out the updater gradually.

Test plan

See further below for specific instructions for performing tests and a list of tests that have already been performed.

Installation and re-installation

Test all Windows installers. To my knowlegde, these are:

  • Setup.exe
  • StandaloneSetup.exe
  • StandaloneSilentSetup.exe

There are also Windows-related .zip files on the GitHub releases page. They do not work for me on Win 7 ("failed to start because its side-by-side configuration is incorrect"). I believe they are less important.

There are 32 and 64 bit versions of each installer. They need to be tested separately. Likewise, the installers need to be tested on all supported Windows versions. I suspect the primary targets are 64 bit Windows 10 and 7.

Another test dimension is whether the user has admin privileges. The scenarios are "yes", "no" and "yes but the user declines admin rights in the UAC prompt".

Another dimension is initial installation vs. re-installing when Brave is already on the system. The latter can be multiplied in various scenarios: Re-installing the same version, installing a newer version, or an older one. It can also be multiplied by using a different installer for re-installation than for initial installation (eg. Setup.exe then StandaloneSetup.exe).

Each installer should set up Brave on the user's system. Setup.exe should fetch the latest version from the update server. StandaloneSetup.exe should work without internet access. StandaloneSilentSetup.exe should do the same without
showing any UI or launching Brave.

On-demand updates

It should be possible to check for updates on brave://settings/help. Similarly to above, there are many possible combinations of source and target versions. The easiest test will likely be to download an old StandaloneSetup.exe and check whether it can update to the new Brave. The most common non-happy path to be tested is lack of an internet connection when checking for an update. Testing should take into account what was decided in the roll-out strategy above. Specifically: If we will release Brave and the updater at the same time, then testing should mimic this. That is, an on-demand update check from an old version should update the updater and Brave. On the other hand, if we will release Brave before the updater, then testing should confirm that new Brave versions obtained via automatic updates still work despite then running on top of the old updater.

Background updates

Omaha applies updates silently, in the background. This can be tested as follows:

  1. Kill all instances of BraveUpdate.exe, BraveCrashHandler.exe and BraveCrashHandler64.exe. You might have to do this multiple times. I like to use the command taskkill /F /IM my.exe.
  2. Delete the registry value HKLM\Software\Wow6432Node\BraveSoftware\Update\LastChecked (or HKCU in case of a per-user installation) - if it exists.
  3. Run BraveSoftwareUpdateTaskMachineUA from the Windows Task Scheduler. Refresh with F5 until the task has ended.

You can see Brave and Omaha's versions in the registry, at ...\Update\Clients\.... You can also enable Omaha's logging as described here. This has changed from previous Omaha versions.

Update notifications

Chromium has a feature where, some time after an update was applied, a button appears in the top-right corner that tells the user to restart the browser:

image

This can be triggered from the command line via:

brave-browser --simulate-outdated

It would not hurt to test whether the feature still works (also when not simulated) in the new Brave.

Updating the updater

We need to test whether the updater is able to update itself. Specifically: The current updater needs to be able to update itself to the new version. And the new updater needs to be able to update itself to another future version.

To test this, we can upload a new version of the updater to the staging server (get in touch with @mherrmann for example). Then, follow the steps in "Cleaning up after a test" below. Install an old copy of BraveBrowserSetup.exe. This should install the current updater. If you installed as admin, check the pv value in registry at HKLM\SOFTWARE\WOW6432Node\BraveSoftware\Update\Clients\{B131C935-9BE6-41DA-9599-1F776BEB8019}. If you installed without admin privileges, check in HKCU\SOFTWARE\BraveSoftware\Update\.... You should see the current updater version. To connect to the staging server, create the registry value ...\BraveSoftware\UpdateDev\url with value https://updates.brave.com/service/update2. Then, perform a background update as described above. After a few minutes, this should update the updater's pv value to the new version.

The same steps can be repeated to test whether the new updater can update to a potentially newer version. Get in touch with @mherrmann to have him upload an even newer version of the updater to the staging server.

brave://policy

Quite a few changes in upstream Omaha since our last rebase affect chrome://policy. We should test whether this page still works in Brave if it is used, and if not, whether it at least does not lead to crashes.

Other Omaha features

Omaha has many other features but I don't know whether Brave is using them. An example that comes to mind are Group Policies for managing enterprise installations.

Test execution

Cleaning up after a test

In the following, HKLM stands for HKEY_LOCAL_MACHINE and HKCU stands for HKEY_CURRENT_USER in registry. One of the two will be used depending on whether you installed Brave as admin or without admin permissions. In the case of HKLM an additional subkey WOW6432Node needs to be used (see below).

  1. Uninstall Brave (all channels). Once you are done, there should only be one subkey in [HKLM/HKCU]\SOFTWARE\[WOW6432Node]\BraveSoftware\Update\Clients. (This is the key for the updater itself.)
  2. Delete the local profile files from %LOCALAPPDATA%\BraveSoftware.
  3. Uninstall Omaha: Execute the command line in reg. value [HKLM/HKCU]\SOFTWARE\[WOW6432Node]\BraveSoftware\Update\UninstallCmdLine. This should empty the registry key ...\BraveSoftware\Update. If not, kill all instances of BraveUpdate.exe, BraveCrashHandler.exe, BraveCrashHandler64.exe in the Task Manager then try again. Also make sure to run the command prompt as Administrator if Brave was installed as admin.

Tests performed so far

Test OS Performed by Pass?
Setup.exe accept UAC 64 bit Win 10 @mherrmann Yes
Setup.exe deny UAC accept install 64 bit Win 10 @mherrmann No [1]
Setup.exe deny UAC deny install 64 bit Win 10 @mherrmann Yes
Setup.exe without internet accept UAC 64 bit Win 10 @mherrmann Yes
StandaloneSetup.exe accept UAC 64 bit Win 10 @mherrmann Yes
StandaloneSetup.exe deny UAC accept install 64 bit Win 10 @mherrmann No [1]
StandaloneSetup.exe deny UAC deny install 64 bit Win 10 @mherrmann Yes
StandaloneSetup.exe without internet accept UAC 64 bit Win 10 @mherrmann Yes
brave://settings/help without internet after SS.exe 64 bit Win 10 @mherrmann Yes
StandaloneSilentSetup.exe as non-admin 64 bit Win 10 @mherrmann Yes
StandaloneSilentSetup.exe as admin 64 bit Win 10 @mherrmann No [2]
On-demand update after SS.exe 1.34.17 -> 1.34.20 64 bit Win 10 @mherrmann Yes
Background update after SS.exe 1.34.17 -> 1.34.20 64 bit Win 10 @mherrmann Yes
Setup.exe as admin 64 bit Win 7 @mherrmann Yes
Setup.exe as non-admin (deny UAC accept install) 64 bit Win 7 @mherrmann No [1]
StandaloneSetup.exe as admin 64 bit Win 7 @mherrmann Yes
On-demand update system-wide install as non-admin 64 bit Win 7 @mherrmann Yes
Background update system-wide install 64 bit Win 7 @mherrmann Yes
SS.exe as non-admin (deny UAC accept install) 64 bit Win 7 @mherrmann No [1]
Background update user install 64 bit Win 7 @mherrmann Yes
StandaloneSilentSetup.exe as non-admin 64 bit Win 7 @mherrmann Yes
Update updater from current to new 64 bit Win 10 @mihaiplesa, @mherrmann Yes
Update updater from new to dummy newer 64 bit Win 10 @mihaiplesa, @mherrmann Yes
Disable updates via a Group Policy 64 bit Win 10 @mherrmann Yes

1: Launches two browser windows. It seems this issue is already present in current versions of Brave. See brave/brave-browser#1833.
2: Performs a per-user instead of a system-wide installation. It seems this issue is already present in current versions of Brave. See brave/brave-browser#18711.

Submitter Checklist

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist

  • The associated issue milestone is set to the smallest version that the
    changes has landed on
  • The new Omaha is uploaded to the update server. After merging the PR, users who newly install Brave will receive the new updater. But existing users who receive Brave via automatic updates will be stuck on the old updater. To fix this, the updater (i.e. Omaha) itself needs to be uploaded to the update server. This is very risky because a problem in the new updater can potentially brick the entire update fleet. See the roll-out plan above.

@mherrmann mherrmann requested review from fmarier and a team as code owners November 15, 2021 17:15
@mherrmann mherrmann added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 dependencies Pull requests that update a dependency file labels Nov 15, 2021
Copy link
Member

@fmarier fmarier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

script/brave_license_helper.py changes look good.

Copy link
Contributor

@iefremov iefremov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unlocking DEPS

@mherrmann mherrmann force-pushed the rebase-omaha-v1-3-36-111 branch from 64d08cc to fcecf37 Compare November 16, 2021 17:17
@mherrmann
Copy link
Collaborator Author

@mihaiplesa what do you think of my roll-out strategy above?

@mherrmann mherrmann force-pushed the rebase-omaha-v1-3-36-111 branch from fcecf37 to e14b39b Compare November 17, 2021 09:40
@simonhong
Copy link
Member

simonhong commented Nov 17, 2021

@mihaiplesa what do you think of my roll-out strategy above?

I think testing with new users first and later updating omaha itself seems very reasonable and safe
if new brave browser works well with current omaha(1.3.101). and maybe that's what we did before.
@mihaiplesa Curious did we update omaha itself from 1.3.99 to 1.3.101?

@mherrmann mherrmann force-pushed the rebase-omaha-v1-3-36-111 branch from e14b39b to 4f1f266 Compare November 17, 2021 13:51
@mihaiplesa
Copy link
Collaborator

@mherrmann thanks for explaining in detail. We bundle the installer inside the browser installation package so it's not separate to allow us to test installer updates vs browser updates in isolation. Partial roll-out sounds good.

@mherrmann mherrmann force-pushed the rebase-omaha-v1-3-36-111 branch from 4f1f266 to 1925598 Compare November 17, 2021 16:43
@mihaiplesa mihaiplesa removed CI/skip-android Do not run CI builds for Android CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-ios Do not run CI builds for iOS labels Nov 19, 2021
Copy link
Member

@goodov goodov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mherrmann mherrmann merged commit 6c7b442 into master Nov 25, 2021
@mherrmann mherrmann deleted the rebase-omaha-v1-3-36-111 branch November 25, 2021 09:40
@mihaiplesa mihaiplesa added this to the 1.34.x - Nightly milestone Nov 25, 2021
@mherrmann mherrmann mentioned this pull request Feb 9, 2022
25 tasks
@mherrmann mherrmann mentioned this pull request Jan 27, 2023
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rebase brave omaha to v1.3.36.111
6 participants