Skip to content

Commit

Permalink
Support brave-variations test seed testing process (uplift to 1.68.x) (
Browse files Browse the repository at this point in the history
…#24627)

Uplift of #24604 (squashed) to beta
  • Loading branch information
brave-builds authored Jul 17, 2024
1 parent ef36475 commit 0a51ff8
Show file tree
Hide file tree
Showing 16 changed files with 228 additions and 55 deletions.
1 change: 1 addition & 0 deletions app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 2 additions & 17 deletions app/brave_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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<int> BraveMainDelegate::BasicStartupComplete() {
Expand Down
17 changes: 0 additions & 17 deletions app/brave_main_delegate_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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());
}
2 changes: 1 addition & 1 deletion browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 6 additions & 1 deletion chromium_src/components/version_ui/version_handler_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
28 changes: 26 additions & 2 deletions components/variations/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
}
63 changes: 63 additions & 0 deletions components/variations/command_line_utils.cc
Original file line number Diff line number Diff line change
@@ -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 <string>

#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
21 changes: 21 additions & 0 deletions components/variations/command_line_utils.h
Original file line number Diff line number Diff line change
@@ -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_
70 changes: 70 additions & 0 deletions components/variations/command_line_utils_unittest.cc
Original file line number Diff line number Diff line change
@@ -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 <string>

#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
18 changes: 18 additions & 0 deletions components/variations/switches.h
Original file line number Diff line number Diff line change
@@ -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_
2 changes: 1 addition & 1 deletion ios/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 5 additions & 0 deletions ios/app/brave_core_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions ios/app/brave_core_switches.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
16 changes: 2 additions & 14 deletions ios/app/brave_main_delegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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");
Expand Down
Loading

0 comments on commit 0a51ff8

Please sign in to comment.