Do not compare flavor strings with == - #2050
Merged
svillar merged 1 commit intoAug 17, 2026
Merged
Conversation
Follow-up to the same fix in RuntimeImpl: these two remaining comparisons resolve correctly today only because the flavor field inlines to an interned literal.
Contributor
Author
|
@svillar PTAL !! |
Contributor
Author
|
thanks !! @svillar |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2049
What
Follow-up to 9100032 ("Do not compare strings with =="), which fixed
BuildConfig.FLAVOR_abi == "x64"in the ChromiumRuntimeImpl. This changes the two remaining comparisons of the same shape, both in shared code:SettingsStore.java:164-AUDIO_ENABLEDSession.java:672- the background-capture guard incaptureBackgroundBitmap()Style matches the earlier fix (
BuildConfig.FLAVOR_backend.equals("chromium")). A grep overapp/src/**/*.javaconfirms these were the last two.Why
Being precise about severity: both evaluate correctly today.
BuildConfig.FLAVOR_backendis generated as apublic static final Stringwith a literal initializer, so it is a compile-time constant that javac inlines, and the comparison ends up between two interned literals. So this is a robustness and readability change, not a bug fix.It is still worth making for the same reasons as 9100032: the code reads as a value comparison while it is a reference comparison, and it silently relies on the field staying an inlined constant. If either side ever stopped being one, both checks would quietly become
false- flipping the audio default (SettingsStore.java:715,ControllerOptionsView.java:99) and disabling the Chromium background-capture guard - with no compiler warning.No behavior change in any current build variant.
Testing
./gradlew assembleNoapiArm64GeckoGenericDebug./gradlew testNoapiArm64GeckoGenericDebugUnitTest