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

Bump the Windows updater (Omaha) to v1.3.361.113 #12895

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

mherrmann
Copy link
Collaborator

@mherrmann mherrmann commented Apr 5, 2022

Resolves brave/brave-browser#22060

Roll-out strategy

Please see "Roll-out strategy" in #11096.

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
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • 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:

Test Plan:

For general update/install functionality, please see "Test Plan" in #11096. It is very important to test this, as otherwise we risk breaking Brave's entire update fleet on Windows.

To test whether /installerEvent pings were successfully disabled, use Fiddler Classic or a similar tool to inspect the network traffic. Previously, this description said to inspect the updater logs. However, they do not include the necessary information for release builds.

@mherrmann mherrmann added 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 Apr 5, 2022
@mherrmann mherrmann added this to the 1.39.x - Nightly milestone Apr 5, 2022
@mherrmann mherrmann requested a review from a team as a code owner April 5, 2022 07:52
@mherrmann mherrmann self-assigned this Apr 5, 2022
@github-actions github-actions bot added the CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps) label Apr 5, 2022
@mherrmann
Copy link
Collaborator Author

mherrmann commented Apr 5, 2022

Test results from CI build of 06bb512 (cf. #11096):

Test OS Performed by Pass?
Setup.exe accept UAC 64 bit Win 10 @mherrmann Yes
On-demand update system-wide install 64 bit Win 10 @mherrmann Yes
Background update system-wide install 64 bit Win 10 @mherrmann Yes
StandaloneSetup.exe accept UAC 64 bit Win 10 @mherrmann Yes (1)
Background update of the updater from .111 to this version .113 64 bit Win 10 @mherrmann Yes
Background update of the updater from this version .113 to a newer version 64 bit Win 10 @mherrmann Yes

1: Passes when testing an "official" build of Brave produced with CI job test-brave-browser-build-windows-x64. When testing a PR build, brave/brave-browser#22122 occurs. This is not unique to this PR.

@mherrmann mherrmann requested a review from goodov April 6, 2022 14:23
@mherrmann mherrmann merged commit 5fcad3e into master Apr 7, 2022
@mherrmann mherrmann deleted the bump-windows-updater-to-v1.3.361.113 branch April 7, 2022 05:52
@mherrmann
Copy link
Collaborator Author

Smoke tests of these changes when it comes to Brave's installers below. The changes were just released in Nightly 1.39.39. The following tests passed:

  1. Setup.exe accept UAC.
  2. On-demand update (without a new version) in brave://settings/help
  3. StandaloneSetup.exe accept UAC.
  4. StandaloneSilentSetup32.exe (installs per-user as in BraveBrowserStandaloneSilentSetup.exe unusable with automated install system because of %LOCALAPPDATA%% brave-browser#18711).

Will test updates once a newer nightly is out.

@mherrmann
Copy link
Collaborator Author

Some more smoke tests on 64 bit Windows 10 now that new updates are available:

  1. StandaloneSetup.exe reject UAC to install as user: Works but opens two browser windows. This also happens for the current release version. I've opened an issue to document this bug.
  2. Background update (non-admin installation): Works and used a delta update file per Omaha's logs.
  3. On-demand update (non-admin installation): Likewise.

The above were performed with Nightly 1.39.39, updating to 1.39.42.

@mherrmann
Copy link
Collaborator Author

@GeetaSarvadnya noticed that - contrary to the initial PR description above - /installerEvent does not show up in the updater's logs (for non-development builds of the updater). Instead, it is necessary to inspect the network traffic with a tool such as Fiddler. I've updated the description above.

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented May 10, 2022

@michael Tested the issue again with the old updater and new updater. Observed that I am not getting installerEvent in Fiddler when the old updater is running. Let me know I can provide the recorded file if you want.

1.39.92 standalone installer - when this new binary is installed new updater 1.3.361.113 is been used and the same is shown in the registry I am not seeing installerEvent logs in fiddler as EXPECTED
image (1)

1.34.60 standalone installer - when this old binary is installed old updater 1.3.361.111 is been used and the same is shown in the registry, but I am NOT seeing installerEvent logs in fiddler
image (2)

@mherrmann
Copy link
Collaborator Author

It's strange. On my laptop, I immediately got /installerEvent with Brave 1.34.60 in Fiddler. On my desktop PC (also Win 10), it would not show up at first. I had to re-install Fiddler.

image

I also clicked the "WinConfig" button in Fiddler. Maybe that helped.

image

I'm on Fiddler v5.0.20211.51073. (And was also on that version before re-installing.)

When Fiddler didn't show /installerEvent, it also didn't show /service/update2 (without trailing /json). However, this path is called when BraveBetaSetup.exe runs. It is the update server endpoint that answers "what's the latest version". So it seems that it was Fiddler that failed to capture the relevant requests.

@GeetaSarvadnya I'd recommend re-installing Fiddler, maybe experimenting with "WinConfig", until you see /service/update2 show up in Fiddler when you run BraveBetaSetup.exe. (Again, /service/update2 should not have a trailing /json. That is a different request.) Then you should also see /installerEvent when running BraveBetaSetup.exe 1.34.60 on a clean system without Omaha. And you should not see it when running a current Beta. You should still see /service/update2 then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps) CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-macos-x64 Do not run CI builds for macOS x64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump Brave's Windows updater (Omaha) to v1.3.361.113
4 participants