Skip to content

Commit

Permalink
Revert "[google_apis] Allow overriding use_official_google_api_keys"
Browse files Browse the repository at this point in the history
This reverts commit 0ae6b65.

Reason for revert: crbug.com/1358960

Original change's description:
> [google_apis] Allow overriding use_official_google_api_keys
>
> Previously, is_chrome_branded would override a false value. Fix that and
> update the C++ logic to be compatible. Retains the existing behavior
> for iOS only due to iOS issue https://crbug.com/1171510, which caused
> the previous attempt, https://crrev.com/c/2657805, to be reverted.
>
> Bug: 1294915, 1183709
> Change-Id: I68d8babe6461a91117396488b2a27772c9952f80
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3756721
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Commit-Queue: David Dorwin <ddorwin@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1041234}

Bug: 1294915, 1183709
Change-Id: I192d4b59097f5db2218447a7983d216b910850a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3864681
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rohit Agarwal <roagarwal@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1042202}
  • Loading branch information
Rohit Agarwal authored and Chromium LUCI CQ committed Sep 1, 2022
1 parent d534d73 commit fc90637
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 52 deletions.
64 changes: 22 additions & 42 deletions google_apis/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ declare_args() {
# Set the variable to false to not use the internal file, even for
# Chrome-branded builds or when it exists in your checkout.
#
# Leave it set to "" to have the variable implicitly set to true for
# Chrome-branded builds or if
# //src/google_apis/internal/google_chrome_api_keys.h is present and false
# otherwise.
# This does not apply to iOS builds, which use different mechanisms and always
# evaluate to use_official_google_api_keys=false.
# See https://crbug.com/1183709.
# Leave it unset or set to "" to have the variable
# implicitly set to true if you have
# src/google_apis/internal/google_chrome_api_keys.h in your
# checkout, and implicitly set to false if not.
#
# Note that Chrome-branded builds always behave as if the variable
# was explicitly set to true, i.e. they always use official keys,
# and will fail to build if the internal file is missing.
use_official_google_api_keys = ""

# Set these to bake the specified API keys and OAuth client
Expand All @@ -35,8 +36,9 @@ declare_args() {
# require server-side APIs may fail to work if no keys are
# provided.
#
# Note that if `use_official_google_api_keys` has been set to true
# (explicitly or implicitly), these values will be ignored and the official
# Note that when building a Chrome-branded build or if
# `use_official_google_api_keys` has been set to `true` (explicitly or
# implicitly), these values will be ignored and the official
# keys will be used instead.
google_api_key = ""

Expand All @@ -48,42 +50,21 @@ declare_args() {
}

if (use_official_google_api_keys == "") {
# No override set. Determine the default behavior.
if (is_chrome_branded) {
# Chrome-branded builds default to behaving as if
#`use_official_google_api_keys` was explicitly set to true and will fail to
# build if the file is missing.
use_official_google_api_keys = true

# TODO(crbug.com/1294931): Remove this override when fixing the issue.
if (is_fuchsia) {
use_official_google_api_keys = false
}
} else {
# Check if the key file exists.
check_internal_result =
exec_script("build/check_internal.py",
[ rebase_path("internal/google_chrome_api_keys.h",
root_build_dir) ],
"value")
use_official_google_api_keys = check_internal_result == 1
}
# Default behavior, check if the key file exists.
check_internal_result =
exec_script("build/check_internal.py",
[ rebase_path("internal/google_chrome_api_keys.h",
root_build_dir) ],
"value")
use_official_google_api_keys = check_internal_result == 1
}

# Official API keys should always be used for Chrome-branded builds except on
# iOS (see https://crbug.com/1183709) and Fuchsia (see the description of
# https://crbug.com/1171510 for background).
assert(
use_official_google_api_keys || !is_chrome_branded || is_ios || is_fuchsia)

# This arg should always be false on iOS. See https://crbug.com/1183709.
assert(!use_official_google_api_keys || !is_ios)

config("key_defines") {
defines = []

# On iOS, Chrome branding controls this define. See https://crbug.com/1183709.
if (use_official_google_api_keys || (is_ios && is_chrome_branded)) {
# TODO(crbug.com/1294915): Refactor so use_official_google_api_keys can be
# used for Fuchsia.
if (!is_fuchsia && (is_chrome_branded || use_official_google_api_keys)) {
defines += [ "USE_OFFICIAL_GOOGLE_API_KEYS=1" ]
}
if (google_api_key != "") {
Expand Down Expand Up @@ -162,8 +143,7 @@ template("google_apis_tmpl") {
"//services/network/public/cpp",
]

# On iOS, Chrome branding controls this deps. See https://crbug.com/1183709.
if (use_official_google_api_keys || (is_ios && is_chrome_branded)) {
if (is_chrome_branded || use_official_google_api_keys) {
deps += [ "internal:generate_metrics_key_header" ]
}

Expand Down
11 changes: 3 additions & 8 deletions google_apis/google_api_keys.cc
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,7 @@ class APIKeyCache {
}

std::string api_key() const { return api_key_; }
#if BUILDFLAG(IS_IOS) || \
(BUILDFLAG(IS_FUCHSIA) && !defined(USE_OFFICIAL_GOOGLE_API_KEYS))
#if BUILDFLAG(IS_IOS) || BUILDFLAG(IS_FUCHSIA)
void set_api_key(const std::string& api_key) { api_key_ = api_key; }
#endif
std::string api_key_non_stable() const { return api_key_non_stable_; }
Expand Down Expand Up @@ -302,10 +301,7 @@ class APIKeyCache {
}

if (key_value == DUMMY_API_TOKEN) {
// iOS does not use USE_OFFICIAL_GOOGLE_API_KEYS. See https://crbug.com/1183709.
// TODO(Tcrbug.com/1183709): The GN logic is inconsistent with this.
#if defined(USE_OFFICIAL_GOOGLE_API_KEYS) || \
(BUILDFLAG(IS_IOS) && BUILDFLAG(GOOGLE_CHROME_BRANDING))
#if BUILDFLAG(GOOGLE_CHROME_BRANDING) && !BUILDFLAG(IS_FUCHSIA)
// No key should be unset in an official build except the
// GOOGLE_DEFAULT_* keys. The default keys don't trigger this
// check as their "unset" value is not DUMMY_API_TOKEN.
Expand Down Expand Up @@ -373,8 +369,7 @@ std::string GetFresnelAPIKey() {
return g_api_key_cache.Get().api_key_fresnel();
}

#if BUILDFLAG(IS_IOS) || \
(BUILDFLAG(IS_FUCHSIA) && !defined(USE_OFFICIAL_GOOGLE_API_KEYS))
#if BUILDFLAG(IS_IOS) || BUILDFLAG(IS_FUCHSIA)
void SetAPIKey(const std::string& api_key) {
g_api_key_cache.Get().set_api_key(api_key);
}
Expand Down
3 changes: 1 addition & 2 deletions google_apis/google_api_keys.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ std::string GetReadAloudAPIKey();
// Retrieves the Fresnel API Key.
std::string GetFresnelAPIKey();

#if BUILDFLAG(IS_IOS) || \
(BUILDFLAG(IS_FUCHSIA) && !defined(USE_OFFICIAL_GOOGLE_API_KEYS))
#if BUILDFLAG(IS_IOS) || BUILDFLAG(IS_FUCHSIA)
// Sets the API key. This should be called as early as possible before this
// API key is even accessed. It must be called before GetAPIKey.
// TODO(https://crbug.com/1166007): Enforce this is called before GetAPIKey.
Expand Down

0 comments on commit fc90637

Please sign in to comment.