diff --git a/app/BUILD.gn b/app/BUILD.gn index 5ba3fc0ca92e..76070554cc9c 100644 --- a/app/BUILD.gn +++ b/app/BUILD.gn @@ -120,6 +120,7 @@ source_set("unit_tests") { deps = [ "//base", "//base/test:test_support", + "//brave/components/variations:buildflags", "//chrome/browser", "//chrome/browser/enterprise/connectors/analysis:features", "//chrome/browser/ui", diff --git a/app/brave_main_delegate.cc b/app/brave_main_delegate.cc index bdb0f0956234..b926a3d5fb10 100644 --- a/app/brave_main_delegate.cc +++ b/app/brave_main_delegate.cc @@ -22,7 +22,7 @@ #include "brave/components/constants/brave_switches.h" #include "brave/components/speedreader/common/buildflags/buildflags.h" #include "brave/components/update_client/buildflags.h" -#include "brave/components/variations/buildflags.h" +#include "brave/components/variations/command_line_utils.h" #include "brave/renderer/brave_content_renderer_client.h" #include "brave/utility/brave_content_utility_client.h" #include "build/build_config.h" @@ -35,7 +35,6 @@ #include "components/dom_distiller/core/dom_distiller_switches.h" #include "components/embedder_support/switches.h" #include "components/sync/base/command_line_switches.h" -#include "components/variations/variations_switches.h" #include "google_apis/gaia/gaia_switches.h" #if BUILDFLAG(IS_LINUX) @@ -169,21 +168,7 @@ void BraveMainDelegate::AppendCommandLineOptions() { brave_sync_service_url.c_str()); } - // Brave variations - if (!command_line->HasSwitch(variations::switches::kVariationsServerURL)) { - command_line->AppendSwitchASCII(variations::switches::kVariationsServerURL, - BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL)); - } - // Insecure fall-back for variations is set to the same (secure) URL. This is - // done so that if VariationsService tries to fall back to insecure url the - // check for kHttpScheme in VariationsService::MaybeRetryOverHTTP would - // prevent it from doing so as we don't want to use an insecure fall-back. - if (!command_line->HasSwitch( - variations::switches::kVariationsInsecureServerURL)) { - command_line->AppendSwitchASCII( - variations::switches::kVariationsInsecureServerURL, - BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL)); - } + variations::AppendBraveCommandLineOptions(*command_line); } std::optional BraveMainDelegate::BasicStartupComplete() { diff --git a/app/brave_main_delegate_unittest.cc b/app/brave_main_delegate_unittest.cc index e9e0918ad174..956438e84e39 100644 --- a/app/brave_main_delegate_unittest.cc +++ b/app/brave_main_delegate_unittest.cc @@ -46,16 +46,9 @@ TEST(BraveMainDelegateUnitTest, OverrideSwitchFromCommandLine) { base::CommandLine& command_line = *base::CommandLine::ForCurrentProcess(); const std::string override_sync_url = "https://sync.com"; const std::string override_origin_trials_public_key = "public_key"; - const std::string override_variations_url = "https://variations.com"; - const std::string override_insecure_variations_url = "https://variations.com"; command_line.AppendSwitchASCII(syncer::kSyncServiceURL, override_sync_url); command_line.AppendSwitchASCII(embedder_support::kOriginTrialPublicKey, override_origin_trials_public_key); - command_line.AppendSwitchASCII(variations::switches::kVariationsServerURL, - override_variations_url); - command_line.AppendSwitchASCII( - variations::switches::kVariationsInsecureServerURL, - override_insecure_variations_url); BraveMainDelegate::AppendCommandLineOptions(); @@ -66,14 +59,4 @@ TEST(BraveMainDelegateUnitTest, OverrideSwitchFromCommandLine) { override_origin_trials_public_key.c_str(), command_line.GetSwitchValueASCII(embedder_support::kOriginTrialPublicKey) .c_str()); - ASSERT_STREQ( - override_variations_url.c_str(), - command_line - .GetSwitchValueASCII(variations::switches::kVariationsServerURL) - .c_str()); - ASSERT_STREQ(override_insecure_variations_url.c_str(), - command_line - .GetSwitchValueASCII( - variations::switches::kVariationsInsecureServerURL) - .c_str()); } diff --git a/browser/sources.gni b/browser/sources.gni index ab87ca04bfce..5936aa9d47db 100644 --- a/browser/sources.gni +++ b/browser/sources.gni @@ -446,7 +446,7 @@ if (is_win && is_official_build) { brave_chrome_browser_public_deps = [ "//brave/browser:browser_process", "//brave/components/brave_sync:constants", - "//brave/components/variations:constants", + "//brave/components/variations", ] if (is_mac) { diff --git a/chromium_src/components/version_ui/version_handler_helper.cc b/chromium_src/components/version_ui/version_handler_helper.cc index 5b1727d4c674..57466edd8fb0 100644 --- a/chromium_src/components/version_ui/version_handler_helper.cc +++ b/chromium_src/components/version_ui/version_handler_helper.cc @@ -25,8 +25,13 @@ base::Value::List GetVariationsList() { } base::Value::List variations_list; - for (std::string& variation : variations) + const std::string& seed_version = variations::GetSeedVersion(); + if (!seed_version.empty() && seed_version != "1") { + variations_list.Append(seed_version); + } + for (std::string& variation : variations) { variations_list.Append(std::move(variation)); + } return variations_list; } diff --git a/components/variations/BUILD.gn b/components/variations/BUILD.gn index d624aaa14430..74e362cb28a4 100644 --- a/components/variations/BUILD.gn +++ b/components/variations/BUILD.gn @@ -11,6 +11,30 @@ buildflag_header("buildflags") { flags = [ "BRAVE_VARIATIONS_SERVER_URL=\"$brave_variations_server_url\"" ] } -group("constants") { - public_deps = [ ":buildflags" ] +static_library("variations") { + sources = [ + "command_line_utils.cc", + "command_line_utils.h", + "switches.h", + ] + + deps = [ + ":buildflags", + "//base", + "//components/variations", + ] +} + +source_set("unit_tests") { + testonly = true + + sources = [ "command_line_utils_unittest.cc" ] + + deps = [ + ":buildflags", + ":variations", + "//base", + "//components/variations", + "//testing/gtest", + ] } diff --git a/components/variations/command_line_utils.cc b/components/variations/command_line_utils.cc new file mode 100644 index 000000000000..8994aec5399d --- /dev/null +++ b/components/variations/command_line_utils.cc @@ -0,0 +1,63 @@ +// Copyright (c) 2024 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +#include "brave/components/variations/command_line_utils.h" + +#include + +#include "base/command_line.h" +#include "base/strings/string_util.h" +#include "brave/components/variations/buildflags.h" +#include "brave/components/variations/switches.h" +#include "components/variations/variations_switches.h" + +namespace variations { + +namespace { + +// A GitHub workflow in the brave/brave-variations repository generates the test +// seed and uploads it to a URL with the following template, where $1 is the +// pull request number. +constexpr char kVariationsPrTestSeedUrlTemplate[] = + "https://griffin.brave.com/pull/$1/seed"; + +} // namespace + +void AppendBraveCommandLineOptions(base::CommandLine& command_line) { + std::string variations_server_url = BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL); + + if (command_line.HasSwitch(switches::kVariationsPR)) { + variations_server_url = base::ReplaceStringPlaceholders( + kVariationsPrTestSeedUrlTemplate, + {command_line.GetSwitchValueASCII(switches::kVariationsPR)}, nullptr); + + // Generated seed is not signed, so we need to disable signature check. + command_line.AppendSwitch( + variations::switches::kAcceptEmptySeedSignatureForTesting); + + // Disable fetch throttling to force the fetch at startup on mobile + // platforms. + command_line.AppendSwitch( + variations::switches::kDisableVariationsSeedFetchThrottling); + } + + if (!command_line.HasSwitch(variations::switches::kVariationsServerURL)) { + command_line.AppendSwitchASCII(variations::switches::kVariationsServerURL, + variations_server_url); + } + + // Insecure fall-back for variations is set to the same (secure) URL. This + // is done so that if VariationsService tries to fall back to insecure url + // the check for kHttpScheme in VariationsService::MaybeRetryOverHTTP would + // prevent it from doing so as we don't want to use an insecure fall-back. + if (!command_line.HasSwitch( + variations::switches::kVariationsInsecureServerURL)) { + command_line.AppendSwitchASCII( + variations::switches::kVariationsInsecureServerURL, + variations_server_url); + } +} + +} // namespace variations diff --git a/components/variations/command_line_utils.h b/components/variations/command_line_utils.h new file mode 100644 index 000000000000..88844027cf3f --- /dev/null +++ b/components/variations/command_line_utils.h @@ -0,0 +1,21 @@ +// Copyright (c) 2024 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +#ifndef BRAVE_COMPONENTS_VARIATIONS_COMMAND_LINE_UTILS_H_ +#define BRAVE_COMPONENTS_VARIATIONS_COMMAND_LINE_UTILS_H_ + +namespace base { +class CommandLine; +} + +namespace variations { + +// Appends Brave-specific command line options to fetch variations seed from the +// correct server. +void AppendBraveCommandLineOptions(base::CommandLine& command_line); + +} // namespace variations + +#endif // BRAVE_COMPONENTS_VARIATIONS_COMMAND_LINE_UTILS_H_ diff --git a/components/variations/command_line_utils_unittest.cc b/components/variations/command_line_utils_unittest.cc new file mode 100644 index 000000000000..705a06cf76f6 --- /dev/null +++ b/components/variations/command_line_utils_unittest.cc @@ -0,0 +1,70 @@ +// Copyright (c) 2024 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +#include "brave/components/variations/command_line_utils.h" + +#include + +#include "base/command_line.h" +#include "brave/components/variations/buildflags.h" +#include "brave/components/variations/switches.h" +#include "components/variations/variations_switches.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace variations { + +TEST(VariationsCommandLineUtils, DefaultVariationsServerUrl) { + base::CommandLine command_line(base::CommandLine::NO_PROGRAM); + AppendBraveCommandLineOptions(command_line); + + EXPECT_EQ(command_line.GetSwitchValueASCII( + variations::switches::kVariationsServerURL), + BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL)); + EXPECT_EQ(command_line.GetSwitchValueASCII( + variations::switches::kVariationsInsecureServerURL), + BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL)); + EXPECT_FALSE(command_line.HasSwitch( + variations::switches::kAcceptEmptySeedSignatureForTesting)); + EXPECT_FALSE(command_line.HasSwitch( + variations::switches::kDisableVariationsSeedFetchThrottling)); +} + +TEST(VariationsCommandLineUtils, OverrideVariationsServerUrl) { + base::CommandLine command_line(base::CommandLine::NO_PROGRAM); + const std::string override_variations_url = "https://variations.com"; + const std::string override_insecure_variations_url = "http://insecure.com"; + command_line.AppendSwitchASCII(variations::switches::kVariationsServerURL, + override_variations_url); + command_line.AppendSwitchASCII( + variations::switches::kVariationsInsecureServerURL, + override_insecure_variations_url); + AppendBraveCommandLineOptions(command_line); + + EXPECT_EQ(override_variations_url, + command_line.GetSwitchValueASCII( + variations::switches::kVariationsServerURL)); + EXPECT_EQ(override_insecure_variations_url, + command_line.GetSwitchValueASCII( + variations::switches::kVariationsInsecureServerURL)); +} + +TEST(VariationsCommandLineUtils, SetVariationsPrParameter) { + base::CommandLine command_line(base::CommandLine::NO_PROGRAM); + command_line.AppendSwitchASCII(variations::switches::kVariationsPR, "1234"); + AppendBraveCommandLineOptions(command_line); + + EXPECT_EQ(command_line.GetSwitchValueASCII( + variations::switches::kVariationsServerURL), + "https://griffin.brave.com/pull/1234/seed"); + EXPECT_EQ(command_line.GetSwitchValueASCII( + variations::switches::kVariationsInsecureServerURL), + "https://griffin.brave.com/pull/1234/seed"); + EXPECT_TRUE(command_line.HasSwitch( + variations::switches::kAcceptEmptySeedSignatureForTesting)); + EXPECT_TRUE(command_line.HasSwitch( + variations::switches::kDisableVariationsSeedFetchThrottling)); +} + +} // namespace variations diff --git a/components/variations/switches.h b/components/variations/switches.h new file mode 100644 index 000000000000..93f0d5d9580c --- /dev/null +++ b/components/variations/switches.h @@ -0,0 +1,18 @@ +// Copyright (c) 2024 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +#ifndef BRAVE_COMPONENTS_VARIATIONS_SWITCHES_H_ +#define BRAVE_COMPONENTS_VARIATIONS_SWITCHES_H_ + +namespace variations::switches { + +// If this flag is set to a brave/brave-variations pull request number, the +// browser will point "--variations-server-url" to a test seed URL from this +// pull request. +inline constexpr char kVariationsPR[] = "variations-pr"; + +} // namespace variations::switches + +#endif // BRAVE_COMPONENTS_VARIATIONS_SWITCHES_H_ diff --git a/ios/app/BUILD.gn b/ios/app/BUILD.gn index 9bff336d3663..12f8958a1f4d 100644 --- a/ios/app/BUILD.gn +++ b/ios/app/BUILD.gn @@ -35,7 +35,7 @@ source_set("app") { "//brave/components/p3a:buildflags", "//brave/components/skus/browser", "//brave/components/update_client:buildflags", - "//brave/components/variations:constants", + "//brave/components/variations", "//brave/ios/app/resources", "//brave/ios/browser", "//brave/ios/browser/api/ads", diff --git a/ios/app/brave_core_switches.h b/ios/app/brave_core_switches.h index b2af640dce3f..d5fb933c5c25 100644 --- a/ios/app/brave_core_switches.h +++ b/ios/app/brave_core_switches.h @@ -24,6 +24,11 @@ OBJC_EXPORT const BraveCoreSwitchKey BraveCoreSwitchKeyVModule; /// /// Expected value: A URL string OBJC_EXPORT const BraveCoreSwitchKey BraveCoreSwitchKeySyncURL; +/// Sets the variations URL to a test seed generated in brave/brave-variations +/// pull request. +/// +/// Expected value: A brave/brave-variations pull request number +OBJC_EXPORT const BraveCoreSwitchKey BraveCoreSwitchKeyVariationsPR; /// Overrides the variations seed URL. Defaults to production /// /// Expected value: A URL string diff --git a/ios/app/brave_core_switches.mm b/ios/app/brave_core_switches.mm index e339a165c775..bedf69c1fa54 100644 --- a/ios/app/brave_core_switches.mm +++ b/ios/app/brave_core_switches.mm @@ -9,6 +9,7 @@ #include "base/strings/sys_string_conversions.h" #include "brave/components/brave_component_updater/browser/switches.h" #include "brave/components/p3a/switches.h" +#include "brave/components/variations/switches.h" #include "components/component_updater/component_updater_switches.h" #include "components/sync/base/command_line_switches.h" #include "components/variations/variations_switches.h" @@ -23,6 +24,8 @@ base::SysUTF8ToNSString(switches::kVModule); const BraveCoreSwitchKey BraveCoreSwitchKeySyncURL = base::SysUTF8ToNSString(syncer::kSyncServiceURL); +const BraveCoreSwitchKey BraveCoreSwitchKeyVariationsPR = + base::SysUTF8ToNSString(variations::switches::kVariationsPR); const BraveCoreSwitchKey BraveCoreSwitchKeyVariationsURL = base::SysUTF8ToNSString(variations::switches::kVariationsServerURL); // There is no exposed switch for rewards diff --git a/ios/app/brave_main_delegate.mm b/ios/app/brave_main_delegate.mm index 0fc0c8c81d05..da9d1aff6274 100644 --- a/ios/app/brave_main_delegate.mm +++ b/ios/app/brave_main_delegate.mm @@ -16,7 +16,7 @@ #include "brave/components/brave_component_updater/browser/switches.h" #include "brave/components/brave_sync/buildflags.h" #include "brave/components/update_client/buildflags.h" -#include "brave/components/variations/buildflags.h" +#include "brave/components/variations/command_line_utils.h" #include "components/browser_sync/browser_sync_switches.h" #include "components/component_updater/component_updater_switches.h" #include "components/sync/base/command_line_switches.h" @@ -72,19 +72,7 @@ @implementation BundleLookupClass BUILDFLAG(BRAVE_SYNC_ENDPOINT)); } - // Brave variations - if (!command_line->HasSwitch(variations::switches::kVariationsServerURL)) { - command_line->AppendSwitchASCII(variations::switches::kVariationsServerURL, - BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL)); - - // Insecure fall-back for variations is set to the same (secure) URL. This - // is done so that if VariationsService tries to fall back to insecure url - // the check for kHttpScheme in VariationsService::MaybeRetryOverHTTP would - // prevent it from doing so as we don't want to use an insecure fall-back. - command_line->AppendSwitchASCII( - variations::switches::kVariationsInsecureServerURL, - BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL)); - } + variations::AppendBraveCommandLineOptions(*command_line); if (!command_line->HasSwitch(switches::kVModule)) { command_line->AppendSwitchASCII(switches::kVModule, "*/brave/*=0"); diff --git a/ios/brave-ios/Sources/Brave/Frontend/Settings/Debug/BraveCoreDebugSwitchesView.swift b/ios/brave-ios/Sources/Brave/Frontend/Settings/Debug/BraveCoreDebugSwitchesView.swift index 90a74dd96cfc..21aff7bab7cb 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Settings/Debug/BraveCoreDebugSwitchesView.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Settings/Debug/BraveCoreDebugSwitchesView.swift @@ -17,6 +17,8 @@ extension BraveCoreSwitchKey { return "Component Updater" case .syncURL: return "Sync URL" + case .variationsPR: + return "Variations PR" case .variationsURL: return "Variations URL" case .p3aDoNotRandomizeUploadInterval: @@ -317,14 +319,18 @@ struct BraveCoreDebugSwitchesView: View { } Section { Group { - // Sync URL NavigationLink { BasicStringInputView(coreSwitch: .syncURL) .keyboardType(.URL) } label: { SwitchContainer(.syncURL) } - // Variations URL + NavigationLink { + BasicStringInputView(coreSwitch: .variationsPR) + .keyboardType(.numberPad) + } label: { + SwitchContainer(.variationsPR) + } NavigationLink { BasicStringInputView(coreSwitch: .variationsURL) .keyboardType(.URL) diff --git a/test/BUILD.gn b/test/BUILD.gn index cab56ae22bb9..2f700833b790 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -265,6 +265,7 @@ test("brave_unit_tests") { "//brave/components/time_period_storage", "//brave/components/tor/buildflags", "//brave/components/url_sanitizer/browser:unittests", + "//brave/components/variations:unit_tests", "//brave/components/version_info:unit_tests", "//brave/extensions:common", "//brave/mojo/brave_ast_patcher:unit_tests",