From e27347b73a179c36c88ca3f9b91df17705feb3ed Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 8 Oct 2026 21:22:53 -0500 Subject: [PATCH] test: emulate the position in the geolocation permission-granted spec (#54698) * test: emulate the position in the geolocation permission-granted spec The test only ran on official builds, because other builds replace the system location provider with FakeLocationProvider, which always reports an error. On official builds it queried the real network location service, which fails on CI with "Failed to query location from network service." Set the position with the DevTools Emulation.setGeolocationOverride command instead, so the test runs on every build and doesn't depend on the network. The permission request still goes through the session's handler, and the test now checks that it did and that the emulated coordinates come back. Co-Authored-By: Claude Opus 5.5 * chore: remove FakeLocationProvider It was added to mock geolocation responses in tests. The geolocation spec now sets the position with Emulation.setGeolocationOverride instead, so the fake provider, the enable_fake_location_provider GN arg, and the OVERRIDE_LOCATION_PROVIDER buildflag are no longer needed. The permission-denied spec no longer depends on the fake provider: a denied request fails before any provider is queried. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- BUILD.gn | 7 ---- buildflags/BUILD.gn | 1 - buildflags/buildflags.gni | 6 --- shell/browser/electron_browser_client.cc | 14 ------- shell/browser/electron_browser_client.h | 2 - shell/browser/fake_location_provider.cc | 52 ------------------------ shell/browser/fake_location_provider.h | 41 ------------------- shell/common/api/features.cc | 6 --- spec/chromium.spec.ts | 22 +++++++--- typings/internal-ambient.d.ts | 1 - 10 files changed, 16 insertions(+), 136 deletions(-) delete mode 100644 shell/browser/fake_location_provider.cc delete mode 100644 shell/browser/fake_location_provider.h diff --git a/BUILD.gn b/BUILD.gn index 014f3fdc69..2a2d3b7925 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -664,13 +664,6 @@ source_set("electron_lib") { defines += [ "NODE_SHARED_MODE" ] } - if (enable_fake_location_provider) { - sources += [ - "shell/browser/fake_location_provider.cc", - "shell/browser/fake_location_provider.h", - ] - } - if (enable_prompt_api) { sources += [ "shell/browser/ai/proxying_ai_manager.cc", diff --git a/buildflags/BUILD.gn b/buildflags/BUILD.gn index 60a7185756..ed43662ba1 100644 --- a/buildflags/BUILD.gn +++ b/buildflags/BUILD.gn @@ -13,7 +13,6 @@ buildflag_header("buildflags") { "ENABLE_ELECTRON_EXTENSIONS=$enable_electron_extensions", "ENABLE_BUILTIN_SPELLCHECKER=$enable_builtin_spellchecker", "ENABLE_PROMPT_API=$enable_prompt_api", - "OVERRIDE_LOCATION_PROVIDER=$enable_fake_location_provider", ] if (electron_vendor_version != "") { diff --git a/buildflags/buildflags.gni b/buildflags/buildflags.gni index fd07d575c7..51d0ede2f7 100644 --- a/buildflags/buildflags.gni +++ b/buildflags/buildflags.gni @@ -5,12 +5,6 @@ declare_args() { enable_pdf_viewer = true - # Provide a fake location provider for mocking - # the geolocation responses. Disable it if you - # need to test with chromium's location provider. - # Should not be enabled for release build. - enable_fake_location_provider = !is_official_build - # Enable Chrome extensions support. enable_electron_extensions = true diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index 5830e4afbf..5d3add0fa5 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -72,7 +72,6 @@ #include "printing/buildflags/buildflags.h" #include "sandbox/policy/switches.h" #include "services/device/public/cpp/geolocation/geolocation_system_permission_manager.h" -#include "services/device/public/cpp/geolocation/location_provider.h" #include "services/network/public/cpp/features.h" #include "services/network/public/cpp/is_potentially_trustworthy.h" #include "services/network/public/cpp/network_switches.h" @@ -165,10 +164,6 @@ #include "components/spellcheck/common/spellcheck.mojom.h" // nogncheck #endif -#if BUILDFLAG(OVERRIDE_LOCATION_PROVIDER) -#include "shell/browser/fake_location_provider.h" -#endif // BUILDFLAG(OVERRIDE_LOCATION_PROVIDER) - #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) #include "base/functional/bind.h" #include "chrome/common/webui_url_constants.h" @@ -956,15 +951,6 @@ ElectronBrowserClient::CreateClientCertStore( #endif } -std::unique_ptr -ElectronBrowserClient::OverrideSystemLocationProvider() { -#if BUILDFLAG(OVERRIDE_LOCATION_PROVIDER) - return std::make_unique(); -#else - return {}; -#endif -} - void ElectronBrowserClient::ConfigureNetworkContextParams( content::BrowserContext* browser_context, bool in_memory, diff --git a/shell/browser/electron_browser_client.h b/shell/browser/electron_browser_client.h index 66840ad0cf..3ed278f936 100644 --- a/shell/browser/electron_browser_client.h +++ b/shell/browser/electron_browser_client.h @@ -225,8 +225,6 @@ class ElectronBrowserClient : public content::ContentBrowserClient, std::vector* additional_schemes) override; void GetAdditionalWebUISchemes( std::vector* additional_schemes) override; - std::unique_ptr OverrideSystemLocationProvider() - override; void ConfigureNetworkContextParams( content::BrowserContext* browser_context, bool in_memory, diff --git a/shell/browser/fake_location_provider.cc b/shell/browser/fake_location_provider.cc deleted file mode 100644 index 3705ef94ea..0000000000 --- a/shell/browser/fake_location_provider.cc +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (c) 2018 GitHub, Inc. -// Use of this source code is governed by the MIT license that can be -// found in the LICENSE file. - -#include "shell/browser/fake_location_provider.h" - -#include "services/device/public/mojom/geoposition.mojom-shared.h" -#include "services/device/public/mojom/geoposition.mojom.h" - -namespace electron { - -FakeLocationProvider::FakeLocationProvider() { - result_ = device::mojom::GeopositionResult::NewError( - device::mojom::GeopositionError::New( - device::mojom::GeopositionErrorCode::kPositionUnavailable, - "Position unavailable.", "")); -} - -FakeLocationProvider::~FakeLocationProvider() = default; - -void FakeLocationProvider::FillDiagnostics( - device::mojom::GeolocationDiagnostics& diagnostics) { - diagnostics.provider_state = state_; -} - -void FakeLocationProvider::SetUpdateCallback( - const LocationProviderUpdateCallback& callback) { - callback_ = callback; -} - -void FakeLocationProvider::StartProvider(bool high_accuracy) { - state_ = - high_accuracy - ? device::mojom::GeolocationDiagnostics::ProviderState::kHighAccuracy - : device::mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy; -} - -void FakeLocationProvider::StopProvider() { - state_ = device::mojom::GeolocationDiagnostics::ProviderState::kStopped; -} - -const device::mojom::GeopositionResult* FakeLocationProvider::GetPosition() { - return result_.get(); -} - -void FakeLocationProvider::OnPermissionGranted() { - if (!callback_.is_null()) { - callback_.Run(this, result_.Clone()); - } -} - -} // namespace electron diff --git a/shell/browser/fake_location_provider.h b/shell/browser/fake_location_provider.h deleted file mode 100644 index 7bb8248ced..0000000000 --- a/shell/browser/fake_location_provider.h +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) 2018 GitHub, Inc. -// Use of this source code is governed by the MIT license that can be -// found in the LICENSE file. - -#ifndef ELECTRON_SHELL_BROWSER_FAKE_LOCATION_PROVIDER_H_ -#define ELECTRON_SHELL_BROWSER_FAKE_LOCATION_PROVIDER_H_ - -#include "services/device/public/cpp/geolocation/location_provider.h" -#include "services/device/public/mojom/geoposition.mojom.h" - -namespace electron { - -class FakeLocationProvider : public device::LocationProvider { - public: - FakeLocationProvider(); - ~FakeLocationProvider() override; - - // disable copy - FakeLocationProvider(const FakeLocationProvider&) = delete; - FakeLocationProvider& operator=(const FakeLocationProvider&) = delete; - - // LocationProvider Implementation: - void FillDiagnostics( - device::mojom::GeolocationDiagnostics& diagnostics) override; - void SetUpdateCallback( - const LocationProviderUpdateCallback& callback) override; - void StartProvider(bool high_accuracy) override; - void StopProvider() override; - const device::mojom::GeopositionResult* GetPosition() override; - void OnPermissionGranted() override; - - private: - device::mojom::GeolocationDiagnostics::ProviderState state_ = - device::mojom::GeolocationDiagnostics::ProviderState::kStopped; - device::mojom::GeopositionResultPtr result_; - LocationProviderUpdateCallback callback_; -}; - -} // namespace electron - -#endif // ELECTRON_SHELL_BROWSER_FAKE_LOCATION_PROVIDER_H_ diff --git a/shell/common/api/features.cc b/shell/common/api/features.cc index a7b3bda390..0ccd2a1c4e 100644 --- a/shell/common/api/features.cc +++ b/shell/common/api/features.cc @@ -18,10 +18,6 @@ bool IsPDFViewerEnabled() { return BUILDFLAG(ENABLE_PDF_VIEWER); } -bool IsFakeLocationProviderEnabled() { - return BUILDFLAG(OVERRIDE_LOCATION_PROVIDER); -} - bool IsPrintingEnabled() { return BUILDFLAG(ENABLE_PRINTING); } @@ -50,8 +46,6 @@ void Initialize(v8::Local exports, gin_helper::Dictionary dict{isolate, exports}; dict.SetMethod<&IsBuiltinSpellCheckerEnabled>("isBuiltinSpellCheckerEnabled"); dict.SetMethod<&IsPDFViewerEnabled>("isPDFViewerEnabled"); - dict.SetMethod<&IsFakeLocationProviderEnabled>( - "isFakeLocationProviderEnabled"); dict.SetMethod<&IsPrintingEnabled>("isPrintingEnabled"); dict.SetMethod<&IsPromptAPIEnabled>("isPromptAPIEnabled"); dict.SetMethod<&IsComponentBuild>("isComponentBuild"); diff --git a/spec/chromium.spec.ts b/spec/chromium.spec.ts index c2091184d7..397c00e0a0 100644 --- a/spec/chromium.spec.ts +++ b/spec/chromium.spec.ts @@ -1257,7 +1257,7 @@ describe('chromium features', () => { }); describe('navigator.geolocation', () => { - ifit(features.isFakeLocationProviderEnabled())('returns error when permission is denied', async () => { + it('returns error when permission is denied', async () => { const w = new BrowserWindow({ show: false, webPreferences: { @@ -1279,23 +1279,33 @@ describe('chromium features', () => { expect(channel).to.equal('success', 'unexpected response from geolocation api'); }); - ifit(!features.isFakeLocationProviderEnabled())('returns position when permission is granted', async () => { + it('returns position when permission is granted', async () => { const w = new BrowserWindow({ show: false, webPreferences: { partition: 'geolocation-spec' } }); - w.webContents.session.setPermissionRequestHandler((_wc, _permission, callback) => { + const requested: string[] = []; + w.webContents.session.setPermissionRequestHandler((_wc, permission, callback) => { + requested.push(permission); callback(true); }); + // Supply the position over CDP so the test doesn't depend on a real provider: + // the network location service fails on CI. + // The permission request still goes through the session's handler. + const override = { latitude: 37.7749, longitude: -122.4194, accuracy: 10 }; + w.webContents.debugger.attach(); + await w.webContents.debugger.sendCommand('Emulation.setGeolocationOverride', override); await w.loadURL(`file://${fixturesPath}/pages/blank.html`); const position = await w.webContents.executeJavaScript(`new Promise((resolve, reject) => navigator.geolocation.getCurrentPosition( - x => resolve({coords: x.coords, timestamp: x.timestamp}), + ({ coords: { latitude, longitude, accuracy }, timestamp }) => + resolve({ coords: { latitude, longitude, accuracy }, timestamp }), err => reject(new Error(err.message))))`); - expect(position).to.have.property('coords'); - expect(position).to.have.property('timestamp'); + expect(requested).to.include('geolocation'); + expect(position.coords).to.deep.equal(override); + expect(position.timestamp).to.be.a('number'); }); ifdescribe(process.platform === 'darwin')('with --disable-geolocation', () => { diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index cfa9e6d3d7..7a1069dfa0 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -20,7 +20,6 @@ declare namespace NodeJS { interface FeaturesBinding { isBuiltinSpellCheckerEnabled(): boolean; isPDFViewerEnabled(): boolean; - isFakeLocationProviderEnabled(): boolean; isPrintingEnabled(): boolean; isPromptAPIEnabled(): boolean; isComponentBuild(): boolean;