chromium/src
0
Fork 0
Code Activity

geolocation: Add kLocationProviderManager feature flag

Browse Source 
This CL adds `kLocationProviderManager` feature flag and associated
parameters to enable flexible mode configuration of the
LocationProviderManager (implementation to follow). The configuration
will determine which location provider is used as the source for
Geolocation API.

Change-Id: I98a2a25f7b05002fb6a1244fde5091a96bad2756
Bug: 333294295
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5493772
Commit-Queue: Alvin Ji <alvinji@chromium.org>
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1301641}
This commit is contained in:
alvinji
2024-05-15 23:06:21 +00:00
committed by Chromium LUCI CQ
parent 241daff096
commit cc04eb8474
11 changed files with 93 additions and 57 deletions

38
chrome/browser/about_flags.cc

@ -3837,6 +3837,23 @@ const FeatureEntry::FeatureVariation kComposeProactiveNudgeVariations[] = {
std::size(kComposeProactiveNudge_LargeUI_100), nullptr}};
#endif // ENABLE_COMPOSE
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
const FeatureEntry::FeatureParam kLocationProviderManagerModeNetworkOnly[] = {
{"LocationProviderManagerMode", "NetworkOnly"}};
const FeatureEntry::FeatureParam kLocationProviderManagerModePlatformOnly[] = {
{"LocationProviderManagerMode", "PlatformOnly"}};
const FeatureEntry::FeatureParam kLocationProviderManagerModeHybridPlatform[] =
{{"LocationProviderManagerMode", "HybridPlatform"}};
const FeatureEntry::FeatureVariation kLocationProviderManagerVariations[] = {
{"NetworkOnly", kLocationProviderManagerModeNetworkOnly,
std::size(kLocationProviderManagerModeNetworkOnly), nullptr},
{"PlatformOnly", kLocationProviderManagerModePlatformOnly,
std::size(kLocationProviderManagerModePlatformOnly), nullptr},
{"HybridPlatform", kLocationProviderManagerModeHybridPlatform,
std::size(kLocationProviderManagerModeHybridPlatform), nullptr}};
#endif // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
// RECORDING USER METRICS FOR FLAGS:
// -----------------------------------------------------------------------------
// The first line of the entry is the internal name.
@ -8135,19 +8152,14 @@ const FeatureEntry kFeatureEntries[] = {
FEATURE_VALUE_TYPE(ash::assistant::features::kAssistantAudioEraser)},
#endif
#if BUILDFLAG(IS_WIN)
{"enable-winrt-geolocation-implementation",
flag_descriptions::kWinrtGeolocationImplementationName,
flag_descriptions::kWinrtGeolocationImplementationDescription, kOsWin,
FEATURE_VALUE_TYPE(features::kWinrtGeolocationImplementation)},
#endif
#if BUILDFLAG(IS_MAC)
{"enable-core-location-backend",
flag_descriptions::kMacCoreLocationBackendName,
flag_descriptions::kMacCoreLocationBackendDescription, kOsMac,
FEATURE_VALUE_TYPE(features::kMacCoreLocationBackend)},
#endif
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
{"enable-location-provider-manager",
flag_descriptions::kLocationProviderManagerName,
flag_descriptions::kLocationProviderManagerDescription, kOsMac | kOsWin,
FEATURE_WITH_PARAMS_VALUE_TYPE(features::kLocationProviderManager,
kLocationProviderManagerVariations,
"LocationProviderManager")},
#endif // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
#if !BUILDFLAG(IS_ANDROID)
{"mute-notification-snooze-action",

15
chrome/browser/flag-metadata.json

@ -2558,11 +2558,6 @@
"owners": [ "keishi@chromium.org" ],
"expiry_milestone": 85
},
{
"name": "enable-core-location-backend",
"owners": [ "deviceapi-team@google.com" ],
"expiry_milestone": 135
},
{
"name": "enable-cros-autocorrect-by-default",
"owners": [ "curtismcmullan@chromium.org", "essential-inputs-team@google.com" ],
@ -3331,6 +3326,11 @@
"owners": ["evliu@google.com", "robsc@chromium.org", "amoylan@chromium.org"],
"expiry_milestone": 140
},
{
"name": "enable-location-provider-manager",
"owners": [ "deviceapi-team@google.com" ],
"expiry_milestone": 150
},
{
"name": "enable-lock-screen-notification",
"owners": [ "newcomer@chromium.org" ],
@ -4197,11 +4197,6 @@
// XInput. See https://crbug.com/1499321.
"expiry_milestone": -1
},
{
"name": "enable-winrt-geolocation-implementation",
"owners": [ "pelavall@microsoft.com", "deviceapi-team@google.com" ],
"expiry_milestone": 150
},
{
"name": "enable-zero-copy",
"owners": [ "ccameron@chromium.org", "chrome-gpu@google.com" ],

18
chrome/browser/flag_descriptions.cc

@ -1159,22 +1159,11 @@ const char kAccessibilitySelectToSpeakShortcutDescription[] =
"This option enables the keyboard shortcut to enable or activate Select to "
"Speak.";
const char kMacCoreLocationBackendName[] = "Core Location Backend";
const char kMacCoreLocationBackendDescription[] =
"Enables usage of the Core Location APIs as the backend for Geolocation "
"API";
const char kNewMacNotificationAPIName[] =
"Determines which notification API to use on macOS devices";
const char kNewMacNotificationAPIDescription[] =
"Enables the usage of Apple's new notification API";
const char kWinrtGeolocationImplementationName[] =
"WinRT Geolocation Implementation";
const char kWinrtGeolocationImplementationDescription[] =
"Enables usage of the Windows.Devices.Geolocation WinRT APIs on Windows "
"for geolocation";
const char kEnableFencedFramesName[] = "Enable the <fencedframe> element.";
const char kEnableFencedFramesDescription[] =
"Fenced frames are an experimental web platform feature that allows "
@ -5489,6 +5478,13 @@ const char kUseAdHocSigningForWebAppShimsDescription[] =
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
const char kLocationProviderManagerName[] =
"Enable location provider manager for Geolocation API";
const char kLocationProviderManagerDescription[] =
"Enables usage of the location provider manager to select between "
"the operating system's location API or the network-based provider "
"as the data source for Geolocation API.";
const char kUseAngleName[] = "Choose ANGLE graphics backend";
const char kUseAngleDefault[] = "Default";
const char kUseAngleGL[] = "OpenGL";

9
chrome/browser/flag_descriptions.h

@ -692,15 +692,9 @@ extern const char kAccessibilitySelectToSpeakShortcutDescription[];
extern const char kAccessibilitySelectToSpeakPrefsMigrationName[];
extern const char kAccessibilitySelectToSpeakPrefsMigrationDescription[];
extern const char kMacCoreLocationBackendName[];
extern const char kMacCoreLocationBackendDescription[];
extern const char kNewMacNotificationAPIName[];
extern const char kNewMacNotificationAPIDescription[];
extern const char kWinrtGeolocationImplementationName[];
extern const char kWinrtGeolocationImplementationDescription[];
extern const char kEnableGenericSensorExtraClassesName[];
extern const char kEnableGenericSensorExtraClassesDescription[];
@ -3190,6 +3184,9 @@ extern const char kUseAdHocSigningForWebAppShimsDescription[];
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
extern const char kLocationProviderManagerName[];
extern const char kLocationProviderManagerDescription[];
extern const char kUseAngleName[];
extern const char kUseAngleDefault[];

8
services/device/geolocation/core_location_provider.cc

@ -2,14 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <Security/Security.h>
#include "services/device/geolocation/core_location_provider.h"
#include "base/apple/scoped_cftyperef.h"
#include "base/task/single_thread_task_runner.h"
#include "services/device/public/cpp/device_features.h"
#include "services/device/public/cpp/geolocation/location_system_permission_status.h"
#include "services/device/public/mojom/geolocation_internals.mojom-shared.h"
namespace device {
@ -83,7 +82,10 @@ void CoreLocationProvider::OnPositionError(
std::unique_ptr<LocationProvider> NewSystemLocationProvider(
SystemGeolocationSource& system_geolocation_source) {
if (!base::FeatureList::IsEnabled(features::kMacCoreLocationBackend)) {
// TODO(crbug.com/333294295): Move this logic into LocationProviderManager to
// enable mode-based selection of location providers.
if (features::kLocationProviderManagerParam.Get() !=
device::mojom::LocationProviderManagerMode::kPlatformOnly) {
return nullptr;
}

7
services/device/geolocation/win/location_provider_winrt.cc

@ -18,6 +18,7 @@
#include "base/win/core_winrt_util.h"
#include "services/device/public/cpp/device_features.h"
#include "services/device/public/cpp/geolocation/geoposition.h"
#include "services/device/public/mojom/geolocation_internals.mojom-shared.h"
namespace device {
@ -455,8 +456,10 @@ HRESULT LocationProviderWinrt::GetGeolocator(IGeolocator** geo_locator) {
}
std::unique_ptr<LocationProvider> NewSystemLocationProvider() {
if (!base::FeatureList::IsEnabled(
features::kWinrtGeolocationImplementation) ||
// TODO(crbug.com/333294295): Move this logic into LocationProviderManager to
// enable mode-based selection of location providers.
if (features::kLocationProviderManagerParam.Get() !=
device::mojom::LocationProviderManagerMode::kPlatformOnly ||
!IsSystemLocationSettingEnabled()) {
return nullptr;
}

1
services/device/public/cpp/BUILD.gn

@ -12,6 +12,7 @@ component("device_features") {
"device_features_export.h",
]
configs += [ "//build/config/compiler:wexit_time_destructors" ]
deps = [ "//services/device/public/mojom:geolocation_internals" ]
public_deps = [ "//base" ]
defines = [ "DEVICE_FEATURES_IMPLEMENTATION" ]

33
services/device/public/cpp/device_features.cc

@ -17,16 +17,6 @@ BASE_FEATURE(kComputePressureBreakCalibrationMitigation,
BASE_FEATURE(kGenericSensorExtraClasses,
"GenericSensorExtraClasses",
base::FEATURE_DISABLED_BY_DEFAULT);
// Enables usage of the Windows.Devices.Geolocation WinRT API for the
// LocationProvider instead of the NetworkLocationProvider on Windows.
BASE_FEATURE(kWinrtGeolocationImplementation,
"WinrtGeolocationImplementation",
base::FEATURE_DISABLED_BY_DEFAULT);
// Enables usage of the CoreLocation API for LocationProvider instead of
// NetworkLocationProvider for macOS or iOS.
BASE_FEATURE(kMacCoreLocationBackend,
"MacCoreLocationBackend",
base::FEATURE_DISABLED_BY_DEFAULT);
// Enable serial communication for SPP devices.
BASE_FEATURE(kEnableBluetoothSerialPortProfileInSerialApi,
"EnableBluetoothSerialPortProfileInSerialApi",
@ -42,4 +32,27 @@ BASE_FEATURE(kSerialPortConnected,
"SerialPortConnected",
base::FEATURE_DISABLED_BY_DEFAULT);
// Enables usage of the location provider manager to select between
// the operating system's location API or our network-based provider
// as the source of location data for Geolocation API.
BASE_FEATURE(kLocationProviderManager,
"LocationProviderManager",
base::FEATURE_DISABLED_BY_DEFAULT);
const base::FeatureParam<device::mojom::LocationProviderManagerMode>::Option
location_provider_manager_mode_options[] = {
{device::mojom::LocationProviderManagerMode::kNetworkOnly,
"NetworkOnly"},
{device::mojom::LocationProviderManagerMode::kPlatformOnly,
"PlatformOnly"},
{device::mojom::LocationProviderManagerMode::kHybridPlatform,
"HybridPlatform"},
};
const base::FeatureParam<device::mojom::LocationProviderManagerMode>
kLocationProviderManagerParam{
&kLocationProviderManager, "LocationProviderManagerMode",
device::mojom::LocationProviderManagerMode::kNetworkOnly,
&location_provider_manager_mode_options};
} // namespace features

9
services/device/public/cpp/device_features.h

@ -9,8 +9,10 @@
#define SERVICES_DEVICE_PUBLIC_CPP_DEVICE_FEATURES_H_
#include "base/feature_list.h"
#include "base/metrics/field_trial_params.h"
#include "build/build_config.h"
#include "services/device/public/cpp/device_features_export.h"
#include "services/device/public/mojom/geolocation_internals.mojom-shared.h"
namespace features {
@ -19,13 +21,16 @@ namespace features {
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(
kComputePressureBreakCalibrationMitigation);
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(kGenericSensorExtraClasses);
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(kWinrtGeolocationImplementation);
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(kMacCoreLocationBackend);
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(
kEnableBluetoothSerialPortProfileInSerialApi);
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(kGeolocationDiagnosticsObserver);
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(kSerialPortConnected);
DEVICE_FEATURES_EXPORT BASE_DECLARE_FEATURE(kLocationProviderManager);
extern const DEVICE_FEATURES_EXPORT
base::FeatureParam<device::mojom::LocationProviderManagerMode>
kLocationProviderManagerParam;
} // namespace features
#endif // SERVICES_DEVICE_PUBLIC_CPP_DEVICE_FEATURES_H_

10
services/device/public/mojom/geolocation_internals.mojom

@ -134,6 +134,16 @@ struct NetworkLocationResponse {
double? accuracy;
};
// Determines which type of location provider LocationProviderManager should
// create and use for geolocation.
enum LocationProviderManagerMode {
kNetworkOnly,
kPlatformOnly,
kCustomOnly,
kHybridPlatform,
kHybridFallbackNetwork,
};
// Interface for chrome://location-internals to be notified when the debug data
// is updated.
interface GeolocationInternalsObserver {

2
tools/metrics/histograms/enums.xml

@ -19553,6 +19553,7 @@ from previous Chrome versions.
<int value="-754475378" label="FedCmSkipWellKnownForSameSite:disabled"/>
<int value="-753988121" label="FlossIsAvailabilityCheckNeeded:enabled"/>
<int value="-753833897" label="AppLaunchAutomation:disabled"/>
<int value="-753170441" label="LocationProviderManager:disabled"/>
<int value="-752321357" label="AutofillEnableGoogleIssuedCard:enabled"/>
<int value="-750175757" label="ClientLoFi:enabled"/>
<int value="-749550261" label="SeamlessRefreshRateSwitching:enabled"/>
@ -24457,6 +24458,7 @@ from previous Chrome versions.
<int value="1405459667" label="enable-fast-text-autosizing"/>
<int value="1406046556" label="enable-slimming-paint-v175"/>
<int value="1406354320" label="MacViewsWebUIDialogs:enabled"/>
<int value="1407248208" label="LocationProviderManager:enabled"/>
<int value="1407616323" label="OfflineIndicatorAlwaysHttpProbe:disabled"/>
<int value="1407625309"
label="disable-minimize-on-second-launcher-item-click"/>