Stateless system permission retrieval
Making the design more flexible, so that each platform can implement
stateless or statefull permission retrieval.
Accesses are now done via a static api.
Bug: b/331784136
Change-Id: I3d14c58f6d9f914948231723e5efd47e8db37529
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5638632
Code-Coverage: [email protected] <[email protected]>
Reviewed-by: Greg Thompson <[email protected]>
Commit-Queue: Jan Láník <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1332921}
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index f0b98065..395f65b 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -78,7 +78,6 @@
#include "chrome/browser/metrics/shutdown_watcher_helper.h"
#include "chrome/browser/nacl_host/nacl_browser_delegate_impl.h"
#include "chrome/browser/net/system_network_context_manager.h"
-#include "chrome/browser/permissions/system/system_permission_settings.h"
#include "chrome/browser/policy/chrome_browser_policy_connector.h"
#include "chrome/browser/policy/messaging_layer/public/report_client.h"
#include "chrome/browser/prefs/chrome_command_line_pref_store.h"
@@ -1398,8 +1397,6 @@
headless::ReportHeadlessActionMetrics();
}
#endif // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
-
- SystemPermissionSettings::Create(profile);
}
void ChromeBrowserMainParts::PreBrowserStart() {
diff --git a/chrome/browser/global_features.cc b/chrome/browser/global_features.cc
index 75369d9f..3a36ca1 100644
--- a/chrome/browser/global_features.cc
+++ b/chrome/browser/global_features.cc
@@ -8,6 +8,7 @@
#include "base/memory/ptr_util.h"
#include "base/no_destructor.h"
#include "build/build_config.h"
+#include "chrome/browser/permissions/system/platform_handle.h"
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
// This causes a gn error on Android builds, because gn does not understand
@@ -47,6 +48,7 @@
}
void GlobalFeatures::Init() {
+ system_permissions_platform_handle_ = CreateSystemPermissionsPlatformHandle();
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
if (user_education::features::IsWhatsNewV2()) {
whats_new_registry_ = CreateWhatsNewRegistry();
@@ -54,6 +56,11 @@
#endif
}
+std::unique_ptr<system_permission_settings::PlatformHandle>
+GlobalFeatures::CreateSystemPermissionsPlatformHandle() {
+ return system_permission_settings::PlatformHandle::Create();
+}
+
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
std::unique_ptr<whats_new::WhatsNewRegistry>
GlobalFeatures::CreateWhatsNewRegistry() {
diff --git a/chrome/browser/global_features.h b/chrome/browser/global_features.h
index d91ece6..dd5e550 100644
--- a/chrome/browser/global_features.h
+++ b/chrome/browser/global_features.h
@@ -5,9 +5,14 @@
#ifndef CHROME_BROWSER_GLOBAL_FEATURES_H_
#define CHROME_BROWSER_GLOBAL_FEATURES_H_
+#include <memory.h>
+
#include "base/functional/callback.h"
#include "build/build_config.h"
+namespace system_permission_settings {
+class PlatformHandle;
+} // namespace system_permission_settings
namespace whats_new {
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
class WhatsNewRegistry;
@@ -36,6 +41,10 @@
// Public accessors for features, e.g.
// FooFeature* foo_feature() { return foo_feature_.get(); }
+ system_permission_settings::PlatformHandle*
+ system_permissions_platform_handle() {
+ return system_permissions_platform_handle_.get();
+ }
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
whats_new::WhatsNewRegistry* whats_new_registry() {
return whats_new_registry_.get();
@@ -49,6 +58,8 @@
// testing. e.g.
// virtual std::unique_ptr<FooFeature> CreateFooFeature();
+ virtual std::unique_ptr<system_permission_settings::PlatformHandle>
+ CreateSystemPermissionsPlatformHandle();
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
virtual std::unique_ptr<whats_new::WhatsNewRegistry> CreateWhatsNewRegistry();
#endif
@@ -57,6 +68,8 @@
// Features will each have a controller. e.g.
// std::unique_ptr<FooFeature> foo_feature_;
+ std::unique_ptr<system_permission_settings::PlatformHandle>
+ system_permissions_platform_handle_;
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
std::unique_ptr<whats_new::WhatsNewRegistry> whats_new_registry_;
#endif
diff --git a/chrome/browser/lacros/chrome_browser_main_extra_parts_lacros.h b/chrome/browser/lacros/chrome_browser_main_extra_parts_lacros.h
index ce0f97d..20aaa17 100644
--- a/chrome/browser/lacros/chrome_browser_main_extra_parts_lacros.h
+++ b/chrome/browser/lacros/chrome_browser_main_extra_parts_lacros.h
@@ -241,9 +241,6 @@
// Handles receiving requests from the Media App (which is in ash).
std::unique_ptr<crosapi::MediaAppLacros> media_app_;
-
- // Retrieves and caces the state of the system permissions on the device.
- std::unique_ptr<SystemPermissionSettings> system_permission_settings_;
};
#endif // CHROME_BROWSER_LACROS_CHROME_BROWSER_MAIN_EXTRA_PARTS_LACROS_H_
diff --git a/chrome/browser/permissions/BUILD.gn b/chrome/browser/permissions/BUILD.gn
index 856028e4a..6f50683e 100644
--- a/chrome/browser/permissions/BUILD.gn
+++ b/chrome/browser/permissions/BUILD.gn
@@ -53,6 +53,7 @@
"quiet_notification_permission_ui_config.h",
"quiet_notification_permission_ui_state.cc",
"quiet_notification_permission_ui_state.h",
+ "system/platform_handle.h",
"system/system_permission_settings.cc",
"system/system_permission_settings.h",
]
diff --git a/chrome/browser/permissions/system/platform_handle.h b/chrome/browser/permissions/system/platform_handle.h
new file mode 100644
index 0000000..e85b5d32
--- /dev/null
+++ b/chrome/browser/permissions/system/platform_handle.h
@@ -0,0 +1,56 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_PERMISSIONS_SYSTEM_PLATFORM_HANDLE_H_
+#define CHROME_BROWSER_PERMISSIONS_SYSTEM_PLATFORM_HANDLE_H_
+
+#include <memory>
+
+#include "chrome/browser/permissions/system/system_permission_settings.h"
+#include "components/content_settings/core/common/content_settings_types.h"
+
+namespace content {
+class WebContents;
+}
+
+namespace system_permission_settings {
+
+// A class that abstracts the access to the system-level permission settings.
+// This class is to be implemented separately for each platform.
+// A single instance is created at a startup and is accessible via
+// GlobalFeatures within the BrowserProcess.
+class PlatformHandle {
+ public:
+ virtual ~PlatformHandle() = default;
+
+ // Returns a new instance of the platform-specific implementation.
+ static std::unique_ptr<PlatformHandle> Create();
+
+ // Returns `true` if Chrome can request system-level permission. Returns
+ // `false` otherwise.
+ virtual bool CanPrompt(ContentSettingsType type) = 0;
+
+ // Returns true if the system blocks the access to the specified content type
+ // permission.
+
+ virtual bool IsDenied(ContentSettingsType type) = 0;
+ // Returns true if the system blocks the access to the specified content type
+ // permission.
+
+ virtual bool IsAllowed(ContentSettingsType type) = 0;
+
+ // Opens the OS page where the user can change the permission settings.
+ // Implementation is OS specific.
+ virtual void OpenSystemSettings(content::WebContents* web_contents,
+ ContentSettingsType type) = 0;
+
+ // Initiates a system permission request and invokes the provided callback
+ // once the user's decision is made.
+ virtual void Request(ContentSettingsType type,
+ SystemPermissionResponseCallback callback) = 0;
+};
+
+} // namespace system_permission_settings
+
+#endif // CHROME_BROWSER_PERMISSIONS_SYSTEM_PLATFORM_HANDLE_H_
diff --git a/chrome/browser/permissions/system/system_permission_settings.cc b/chrome/browser/permissions/system/system_permission_settings.cc
index 15bd096..29ec1d7 100644
--- a/chrome/browser/permissions/system/system_permission_settings.cc
+++ b/chrome/browser/permissions/system/system_permission_settings.cc
@@ -6,11 +6,16 @@
#include <map>
#include <memory>
+#include <utility>
-#include "base/supports_user_data.h"
+#include "base/check.h"
+#include "base/check_deref.h"
+#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/global_features.h"
+#include "chrome/browser/permissions/system/platform_handle.h"
+
+namespace system_permission_settings {
namespace {
std::map<ContentSettingsType, bool>& GlobalTestingBlockOverrides() {
@@ -18,53 +23,62 @@
return g_testing_block_overrides;
}
-const void* const kSystemPermissionSettingsKey = &kSystemPermissionSettingsKey;
+PlatformHandle* GetPlatformHandle() {
+#if !BUILDFLAG(IS_ANDROID)
+ return CHECK_DEREF(CHECK_DEREF(g_browser_process).GetFeatures())
+ .system_permissions_platform_handle();
+#else
+ return nullptr;
+#endif
+}
} // namespace
-std::unique_ptr<base::SupportsUserData::Data>
-SystemPermissionSettings::Clone() {
- return nullptr;
+// static
+bool CanPrompt(ContentSettingsType type) {
+ return GetPlatformHandle()->CanPrompt(type);
}
-void SystemPermissionSettings::Create(Profile* profile) {
- CHECK(profile);
- profile->SetUserData(kSystemPermissionSettingsKey, CreateImpl());
-}
-
-SystemPermissionSettings* SystemPermissionSettings::GetInstance() {
- Profile* profile = g_browser_process->profile_manager()->GetLastUsedProfile();
- CHECK(profile);
- return static_cast<SystemPermissionSettings*>(
- profile->GetUserData(kSystemPermissionSettingsKey));
-}
-
-bool SystemPermissionSettings::IsDenied(ContentSettingsType type) const {
+// static
+bool IsDenied(ContentSettingsType type) {
if (GlobalTestingBlockOverrides().find(type) !=
GlobalTestingBlockOverrides().end()) {
return GlobalTestingBlockOverrides().at(type);
}
- return IsDeniedImpl(type);
+ return GetPlatformHandle()->IsDenied(type);
}
-bool SystemPermissionSettings::IsAllowed(ContentSettingsType type) const {
+// static
+bool IsAllowed(ContentSettingsType type) {
if (GlobalTestingBlockOverrides().find(type) !=
GlobalTestingBlockOverrides().end()) {
- return GlobalTestingBlockOverrides().at(type);
+ return !GlobalTestingBlockOverrides().at(type);
}
- return IsAllowedImpl(type);
+ return GetPlatformHandle()->IsAllowed(type);
}
-ScopedSystemPermissionSettingsForTesting::
- ScopedSystemPermissionSettingsForTesting(ContentSettingsType type,
- bool blocked)
+// static
+void OpenSystemSettings(content::WebContents* web_contents,
+ ContentSettingsType type) {
+ GetPlatformHandle()->OpenSystemSettings(web_contents, type);
+}
+
+// static
+void Request(ContentSettingsType type,
+ SystemPermissionResponseCallback callback) {
+ GetPlatformHandle()->Request(type, std::move(callback));
+}
+
+ScopedSettingsForTesting::ScopedSettingsForTesting(ContentSettingsType type,
+ bool blocked)
: type_(type) {
CHECK(GlobalTestingBlockOverrides().find(type) ==
GlobalTestingBlockOverrides().end());
GlobalTestingBlockOverrides()[type] = blocked;
}
-ScopedSystemPermissionSettingsForTesting::
- ~ScopedSystemPermissionSettingsForTesting() {
+ScopedSettingsForTesting::~ScopedSettingsForTesting() {
GlobalTestingBlockOverrides().erase(type_);
}
+
+} // namespace system_permission_settings
diff --git a/chrome/browser/permissions/system/system_permission_settings.h b/chrome/browser/permissions/system/system_permission_settings.h
index 988e8d94..2159e962 100644
--- a/chrome/browser/permissions/system/system_permission_settings.h
+++ b/chrome/browser/permissions/system/system_permission_settings.h
@@ -5,69 +5,49 @@
#ifndef CHROME_BROWSER_PERMISSIONS_SYSTEM_SYSTEM_PERMISSION_SETTINGS_H_
#define CHROME_BROWSER_PERMISSIONS_SYSTEM_SYSTEM_PERMISSION_SETTINGS_H_
-#include <memory>
-
-#include "base/supports_user_data.h"
-#include "chrome/browser/profiles/profile.h"
#include "components/content_settings/core/common/content_settings_types.h"
-#include "content/public/browser/web_contents.h"
-// A class that abstracts the access to the system-level permission settings.
-//
-// There is a certain overlap with
-// https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/permissions/system_permission_delegate.h
-// this is intentional as explained in
-// https://chromium-review.googlesource.com/c/chromium/src/+/5424111/comment/5e007f7b_c2b9ff9f
-class SystemPermissionSettings : public base::SupportsUserData::Data {
+namespace content {
+class WebContents;
+}
+
+namespace system_permission_settings {
+
+using SystemPermissionResponseCallback = base::OnceCallback<void()>;
+
+// Returns `true` if Chrome can request system-level permission. Returns
+// `false` otherwise.
+bool CanPrompt(ContentSettingsType type);
+
+// Check whether the system blocks the access to the specified content type /
+// permission.
+bool IsDenied(ContentSettingsType type);
+
+// Check whether the system allows the access to the specified content type /
+// permission.
+// On some platforms, both IsDenied and IsAllowed may return false for the
+// same permission.
+bool IsAllowed(ContentSettingsType type);
+
+// Opens the OS page where the user can change the permission settings.
+// Implementation is OS specific.
+void OpenSystemSettings(content::WebContents* web_contents,
+ ContentSettingsType type);
+
+// Initiates a system permission request and invokes the provided callback
+// once the user's decision is made.
+void Request(ContentSettingsType type,
+ SystemPermissionResponseCallback callback);
+
+class ScopedSettingsForTesting {
public:
- using SystemPermissionResponseCallback = base::OnceCallback<void()>;
-
- // base::SupportsUserData::Data:
- std::unique_ptr<base::SupportsUserData::Data> Clone() override;
-
- // Creates a new instance of SystemPermissionSettings that is OS-specific and
- // saves it within the profile. Should be only used when initializing the
- // Profile.
- static void Create(Profile*);
-
- // Gets a cached instance of SystemPermissionSettings from Profile.
- static SystemPermissionSettings* GetInstance();
-
- // Returns `true` if Chrome can request system-level permission. Returns
- // `false` otherwise.
- virtual bool CanPrompt(ContentSettingsType type) const = 0;
-
- // Check whether the system blocks the access to the specified content type /
- // permission.
- bool IsDenied(ContentSettingsType type) const;
- // Check whether the system allows the access to the specified content type /
- // permission.
- bool IsAllowed(ContentSettingsType type) const;
-
- // Opens the OS page where the user can change the permission settings.
- // Implementation is OS specific.
- virtual void OpenSystemSettings(content::WebContents* web_contents,
- ContentSettingsType type) const = 0;
-
- // Initiates a system permission request and invokes the provided callback
- // once the user's decision is made.
- virtual void Request(ContentSettingsType type,
- SystemPermissionResponseCallback callback) = 0;
-
- private:
- virtual bool IsDeniedImpl(ContentSettingsType type) const = 0;
- virtual bool IsAllowedImpl(ContentSettingsType type) const = 0;
- static std::unique_ptr<SystemPermissionSettings> CreateImpl();
-};
-
-class ScopedSystemPermissionSettingsForTesting {
- public:
- ScopedSystemPermissionSettingsForTesting(ContentSettingsType type,
- bool blocked);
- ~ScopedSystemPermissionSettingsForTesting();
+ ScopedSettingsForTesting(ContentSettingsType type, bool blocked);
+ ~ScopedSettingsForTesting();
private:
ContentSettingsType type_;
};
+} // namespace system_permission_settings
+
#endif // CHROME_BROWSER_PERMISSIONS_SYSTEM_SYSTEM_PERMISSION_SETTINGS_H_
diff --git a/chrome/browser/permissions/system/system_permission_settings_chromeos.cc b/chrome/browser/permissions/system/system_permission_settings_chromeos.cc
index 607b250..1bc848fc 100644
--- a/chrome/browser/permissions/system/system_permission_settings_chromeos.cc
+++ b/chrome/browser/permissions/system/system_permission_settings_chromeos.cc
@@ -4,16 +4,23 @@
#include "chrome/browser/permissions/system/system_permission_settings.h"
+#include <utility>
+
#include "base/feature_list.h"
#include "chrome/browser/ash/privacy_hub/privacy_hub_util.h"
+#include "chrome/browser/permissions/system/platform_handle.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "components/content_settings/core/common/content_settings_types.h"
#include "components/content_settings/core/common/features.h"
-class SystemPermissionSettingsImpl : public SystemPermissionSettings {
- bool CanPrompt(ContentSettingsType type) const override { return false; }
+namespace system_permission_settings {
- bool IsDeniedImpl(ContentSettingsType type) const override {
+namespace {
+
+class PlatformHandleImpl : public PlatformHandle {
+ bool CanPrompt(ContentSettingsType type) override { return false; }
+
+ bool IsDenied(ContentSettingsType type) override {
if (base::FeatureList::IsEnabled(
content_settings::features::
kCrosSystemLevelPermissionBlockedWarnings)) {
@@ -22,12 +29,10 @@
return false;
}
- bool IsAllowedImpl(ContentSettingsType type) const override {
- return !IsDeniedImpl(type);
- }
+ bool IsAllowed(ContentSettingsType type) override { return !IsDenied(type); }
void OpenSystemSettings(content::WebContents*,
- ContentSettingsType type) const override {
+ ContentSettingsType type) override {
if (base::FeatureList::IsEnabled(
content_settings::features::
kCrosSystemLevelPermissionBlockedWarnings)) {
@@ -43,7 +48,11 @@
}
};
-std::unique_ptr<SystemPermissionSettings>
-SystemPermissionSettings::CreateImpl() {
- return std::make_unique<SystemPermissionSettingsImpl>();
+} // namespace
+
+// static
+std::unique_ptr<PlatformHandle> PlatformHandle::Create() {
+ return std::make_unique<PlatformHandleImpl>();
}
+
+} // namespace system_permission_settings
diff --git a/chrome/browser/permissions/system/system_permission_settings_default.cc b/chrome/browser/permissions/system/system_permission_settings_default.cc
index d280fb8..44c2eac 100644
--- a/chrome/browser/permissions/system/system_permission_settings_default.cc
+++ b/chrome/browser/permissions/system/system_permission_settings_default.cc
@@ -2,20 +2,29 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <utility>
+
#include "build/buildflag.h"
+#include "chrome/browser/permissions/system/platform_handle.h"
#include "chrome/browser/permissions/system/system_permission_settings.h"
#include "components/content_settings/core/common/content_settings_types.h"
static_assert(!BUILDFLAG(IS_CHROMEOS_ASH));
static_assert(!BUILDFLAG(IS_MAC));
-class SystemPermissionSettingsImpl : public SystemPermissionSettings {
- bool CanPrompt(ContentSettingsType type) const override { return false; }
- bool IsDeniedImpl(ContentSettingsType type) const override { return false; }
- bool IsAllowedImpl(ContentSettingsType type) const override { return true; }
+namespace system_permission_settings {
+
+namespace {
+
+class PlatformHandleImpl : public PlatformHandle {
+ bool CanPrompt(ContentSettingsType type) override { return false; }
+
+ bool IsDenied(ContentSettingsType type) override { return false; }
+
+ bool IsAllowed(ContentSettingsType type) override { return true; }
void OpenSystemSettings(content::WebContents*,
- ContentSettingsType type) const override {
+ ContentSettingsType type) override {
// no-op
NOTREACHED();
}
@@ -27,7 +36,11 @@
}
};
-std::unique_ptr<SystemPermissionSettings>
-SystemPermissionSettings::CreateImpl() {
- return std::make_unique<SystemPermissionSettingsImpl>();
+} // namespace
+
+// static
+std::unique_ptr<PlatformHandle> PlatformHandle::Create() {
+ return std::make_unique<PlatformHandleImpl>();
}
+
+} // namespace system_permission_settings
diff --git a/chrome/browser/permissions/system/system_permission_settings_mac.cc b/chrome/browser/permissions/system/system_permission_settings_mac.cc
index 296397f..8e491e38 100644
--- a/chrome/browser/permissions/system/system_permission_settings_mac.cc
+++ b/chrome/browser/permissions/system/system_permission_settings_mac.cc
@@ -5,11 +5,14 @@
#include "chrome/browser/permissions/system/system_permission_settings.h"
#include <memory>
+#include <utility>
+#include <vector>
#include "base/mac/mac_util.h"
#include "base/notreached.h"
#include "base/scoped_observation.h"
#include "chrome/browser/media/webrtc/system_media_capture_permissions_mac.h"
+#include "chrome/browser/permissions/system/platform_handle.h"
#include "chrome/browser/web_applications/os_integration/mac/app_shim_registry.h"
#include "chrome/browser/web_applications/os_integration/mac/web_app_shortcut_mac.h"
#include "chrome/browser/web_applications/web_app_tab_helper.h"
@@ -20,6 +23,8 @@
static_assert(BUILDFLAG(IS_MAC));
+namespace system_permission_settings {
+
namespace {
bool denied(system_media_permissions::SystemPermission permission) {
return system_media_permissions::SystemPermission::kDenied == permission;
@@ -29,31 +34,16 @@
return system_media_permissions::SystemPermission::kNotDetermined ==
permission;
}
-
bool allowed(system_media_permissions::SystemPermission permission) {
return system_media_permissions::SystemPermission::kAllowed == permission;
}
-} // namespace
-
-class SystemPermissionSettingsImpl
- : public SystemPermissionSettings,
+class PlatformHandleImpl
+ : public PlatformHandle,
public device::GeolocationSystemPermissionManager::PermissionObserver {
public:
- SystemPermissionSettingsImpl() {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- auto* geolocation_system_permission_manager =
- device::GeolocationSystemPermissionManager::GetInstance();
- CHECK(geolocation_system_permission_manager);
- observation_.Observe(geolocation_system_permission_manager);
- }
-
- ~SystemPermissionSettingsImpl() override {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- FlushGeolocationCallbacks();
- }
-
- bool CanPrompt(ContentSettingsType type) const override {
+ // PlatformHandle:
+ bool CanPrompt(ContentSettingsType type) override {
switch (type) {
case ContentSettingsType::MEDIASTREAM_CAMERA:
return prompt(
@@ -70,7 +60,7 @@
}
}
- bool IsDeniedImpl(ContentSettingsType type) const override {
+ bool IsDenied(ContentSettingsType type) override {
switch (type) {
case ContentSettingsType::MEDIASTREAM_CAMERA:
return denied(
@@ -87,7 +77,7 @@
}
}
- bool IsAllowedImpl(ContentSettingsType type) const override {
+ bool IsAllowed(ContentSettingsType type) override {
switch (type) {
case ContentSettingsType::MEDIASTREAM_CAMERA:
return allowed(
@@ -105,7 +95,7 @@
}
void OpenSystemSettings(content::WebContents* web_contents,
- ContentSettingsType type) const override {
+ ContentSettingsType type) override {
switch (type) {
case ContentSettingsType::NOTIFICATIONS: {
const webapps::AppId* app_id =
@@ -152,12 +142,18 @@
return;
}
case ContentSettingsType::GEOLOCATION: {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
geolocation_callbacks_.push_back(std::move(callback));
// The system permission prompt is modal and requires a user decision
// (Allow or Deny) before it can be dismissed.
if (geolocation_callbacks_.size() == 1u) {
- device::GeolocationSystemPermissionManager::GetInstance()
- ->RequestSystemPermission();
+ auto* geolocation_system_permission_manager =
+ device::GeolocationSystemPermissionManager::GetInstance();
+ CHECK(geolocation_system_permission_manager);
+ CHECK(!observation_.IsObserving());
+ // Lazily setup geolocation status observation
+ observation_.Observe(geolocation_system_permission_manager);
+ geolocation_system_permission_manager->RequestSystemPermission();
}
return;
}
@@ -167,10 +163,12 @@
}
}
- // device::GeolocationSystemPermissionManager::PermissionObserver
- // implementation.
+ // device::GeolocationSystemPermissionManager::PermissionObserver:
void OnSystemPermissionUpdated(
device::LocationSystemPermissionStatus new_status) override {
+ // No further observation needed as all the current requests will now be
+ // resolved
+ observation_.Reset();
FlushGeolocationCallbacks();
}
@@ -189,7 +187,11 @@
observation_{this};
};
-std::unique_ptr<SystemPermissionSettings>
-SystemPermissionSettings::CreateImpl() {
- return std::make_unique<SystemPermissionSettingsImpl>();
+} // namespace
+
+// static
+std::unique_ptr<PlatformHandle> PlatformHandle::Create() {
+ return std::make_unique<PlatformHandleImpl>();
}
+
+} // namespace system_permission_settings
diff --git a/chrome/browser/permissions/system/system_permission_settings_win.cc b/chrome/browser/permissions/system/system_permission_settings_win.cc
index ac5f9e4..09338c4 100644
--- a/chrome/browser/permissions/system/system_permission_settings_win.cc
+++ b/chrome/browser/permissions/system/system_permission_settings_win.cc
@@ -8,6 +8,7 @@
#include "base/notreached.h"
#include "base/scoped_observation.h"
+#include "chrome/browser/permissions/system/platform_handle.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/web_contents.h"
#include "services/device/public/cpp/device_features.h"
@@ -16,30 +17,16 @@
static_assert(BUILDFLAG(IS_WIN));
-class SystemPermissionSettingsWin
- : public SystemPermissionSettings,
+namespace system_permission_settings {
+
+namespace {
+
+class PlatformHandleImpl
+ : public PlatformHandle,
public device::GeolocationSystemPermissionManager::PermissionObserver {
public:
- SystemPermissionSettingsWin() {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- // Initialize system permission settings for geolocation based on the
- // feature flag.
- if (base::FeatureList::IsEnabled(features::kWinSystemLocationPermission)) {
- // If the feature is enabled, retrieve and observe the system permission
- // status.
- auto* geolocation_system_permission_manager =
- device::GeolocationSystemPermissionManager::GetInstance();
- CHECK(geolocation_system_permission_manager);
- observation_.Observe(geolocation_system_permission_manager);
- }
- }
-
- ~SystemPermissionSettingsWin() override {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- FlushGeolocationCallbacks();
- }
-
- bool CanPrompt(ContentSettingsType type) const override {
+ // PlatformHandle:
+ bool CanPrompt(ContentSettingsType type) override {
switch (type) {
case ContentSettingsType::GEOLOCATION: {
if (base::FeatureList::IsEnabled(
@@ -56,7 +43,7 @@
}
}
- bool IsDeniedImpl(ContentSettingsType type) const override {
+ bool IsDenied(ContentSettingsType type) override {
switch (type) {
case ContentSettingsType::GEOLOCATION:
if (base::FeatureList::IsEnabled(
@@ -72,7 +59,7 @@
}
}
- bool IsAllowedImpl(ContentSettingsType type) const override {
+ bool IsAllowed(ContentSettingsType type) override {
switch (type) {
case ContentSettingsType::GEOLOCATION:
if (base::FeatureList::IsEnabled(
@@ -89,7 +76,7 @@
}
void OpenSystemSettings(content::WebContents* web_contents,
- ContentSettingsType type) const override {
+ ContentSettingsType type) override {
switch (type) {
case ContentSettingsType::GEOLOCATION: {
if (base::FeatureList::IsEnabled(
@@ -108,14 +95,22 @@
SystemPermissionResponseCallback callback) override {
switch (type) {
case ContentSettingsType::GEOLOCATION: {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ if (!base::FeatureList::IsEnabled(
+ features::kWinSystemLocationPermission)) {
+ return;
+ }
geolocation_callbacks_.push_back(std::move(callback));
// The system permission prompt is modal and requires a user decision
// (Allow or Deny) before it can be dismissed.
- if (geolocation_callbacks_.size() == 1u &&
- base::FeatureList::IsEnabled(
- features::kWinSystemLocationPermission)) {
- device::GeolocationSystemPermissionManager::GetInstance()
- ->RequestSystemPermission();
+ if (geolocation_callbacks_.size() == 1u) {
+ auto* geolocation_system_permission_manager =
+ device::GeolocationSystemPermissionManager::GetInstance();
+ CHECK(geolocation_system_permission_manager);
+ CHECK(!observation_.IsObserving());
+ // Lazily setup geolocation status observation
+ observation_.Observe(geolocation_system_permission_manager);
+ geolocation_system_permission_manager->RequestSystemPermission();
}
return;
}
@@ -129,6 +124,7 @@
// implementation.
void OnSystemPermissionUpdated(
device::LocationSystemPermissionStatus new_status) override {
+ observation_.Reset();
FlushGeolocationCallbacks();
}
@@ -147,8 +143,11 @@
observation_{this};
};
+} // namespace
+
// static
-std::unique_ptr<SystemPermissionSettings>
-SystemPermissionSettings::CreateImpl() {
- return std::make_unique<SystemPermissionSettingsWin>();
+std::unique_ptr<PlatformHandle> PlatformHandle::Create() {
+ return std::make_unique<PlatformHandleImpl>();
}
+
+} // namespace system_permission_settings
diff --git a/chrome/browser/ui/page_info/chrome_page_info_ui_delegate.cc b/chrome/browser/ui/page_info/chrome_page_info_ui_delegate.cc
index e90beac..7a830cc 100644
--- a/chrome/browser/ui/page_info/chrome_page_info_ui_delegate.cc
+++ b/chrome/browser/ui/page_info/chrome_page_info_ui_delegate.cc
@@ -262,8 +262,7 @@
case ContentSettingsType::MEDIASTREAM_CAMERA:
if (base::FeatureList::IsEnabled(
content_settings::features::kLeftHandSideActivityIndicators) &&
- SystemPermissionSettings::GetInstance() &&
- SystemPermissionSettings::GetInstance()->IsDenied(type)) {
+ system_permission_settings::IsDenied(type)) {
*text_id = IDS_PAGE_INFO_CAMERA_SYSTEM_SETTINGS_DESCRIPTION;
*link_id = IDS_PAGE_INFO_SETTINGS_OF_A_SYSTEM_LINK;
return true;
@@ -272,8 +271,7 @@
case ContentSettingsType::MEDIASTREAM_MIC:
if (base::FeatureList::IsEnabled(
content_settings::features::kLeftHandSideActivityIndicators) &&
- SystemPermissionSettings::GetInstance() &&
- SystemPermissionSettings::GetInstance()->IsDenied(type)) {
+ system_permission_settings::IsDenied(type)) {
*text_id = IDS_PAGE_INFO_MICROPHONE_SYSTEM_SETTINGS_DESCRIPTION;
*link_id = IDS_PAGE_INFO_SETTINGS_OF_A_SYSTEM_LINK;
return true;
@@ -285,8 +283,7 @@
}
void ChromePageInfoUiDelegate::SettingsLinkClicked(ContentSettingsType type) {
- SystemPermissionSettings::GetInstance()->OpenSystemSettings(web_contents_,
- type);
+ system_permission_settings::OpenSystemSettings(web_contents_, type);
}
bool ChromePageInfoUiDelegate::IsBlockAutoPlayEnabled() {
diff --git a/chrome/browser/ui/views/permissions/chip/LHS_indicators_browsertest.cc b/chrome/browser/ui/views/permissions/chip/LHS_indicators_browsertest.cc
index fce8ef2b..f646a2fc 100644
--- a/chrome/browser/ui/views/permissions/chip/LHS_indicators_browsertest.cc
+++ b/chrome/browser/ui/views/permissions/chip/LHS_indicators_browsertest.cc
@@ -489,7 +489,7 @@
InvokeUi_PageInfo_camera_blocked_on_system_level) {
SetPermission(ContentSettingsType::MEDIASTREAM_CAMERA,
ContentSetting::CONTENT_SETTING_ALLOW);
- ScopedSystemPermissionSettingsForTesting scoped_system_permission(
+ system_permission_settings::ScopedSettingsForTesting scoped_system_permission(
ContentSettingsType::MEDIASTREAM_CAMERA, /*blocked=*/true);
UpdatePageInfo();
@@ -502,7 +502,7 @@
SetPermission(ContentSettingsType::MEDIASTREAM_MIC,
ContentSetting::CONTENT_SETTING_ALLOW);
- ScopedSystemPermissionSettingsForTesting scoped_system_permission(
+ system_permission_settings::ScopedSettingsForTesting scoped_system_permission(
ContentSettingsType::MEDIASTREAM_MIC, /*blocked=*/true);
UpdatePageInfo();
@@ -518,10 +518,12 @@
SetPermission(ContentSettingsType::MEDIASTREAM_MIC,
ContentSetting::CONTENT_SETTING_ALLOW);
- ScopedSystemPermissionSettingsForTesting scoped_system_permission_camera(
- ContentSettingsType::MEDIASTREAM_CAMERA, /*blocked=*/true);
- ScopedSystemPermissionSettingsForTesting scoped_system_permission_mic(
- ContentSettingsType::MEDIASTREAM_MIC, /*blocked=*/true);
+ system_permission_settings::ScopedSettingsForTesting
+ scoped_system_permission_camera(ContentSettingsType::MEDIASTREAM_CAMERA,
+ /*blocked=*/true);
+ system_permission_settings::ScopedSettingsForTesting
+ scoped_system_permission_mic(ContentSettingsType::MEDIASTREAM_MIC,
+ /*blocked=*/true);
UpdatePageInfo();
diff --git a/chrome/browser/ui/views/permissions/chip/dashboard_kombucha_interactive_uitest.cc b/chrome/browser/ui/views/permissions/chip/dashboard_kombucha_interactive_uitest.cc
index 4ffb8c7..82273d5 100644
--- a/chrome/browser/ui/views/permissions/chip/dashboard_kombucha_interactive_uitest.cc
+++ b/chrome/browser/ui/views/permissions/chip/dashboard_kombucha_interactive_uitest.cc
@@ -169,7 +169,7 @@
CameraUsingTestWithSystemBlock) {
SetPermission(ContentSettingsType::MEDIASTREAM_CAMERA, CONTENT_SETTING_ALLOW);
- ScopedSystemPermissionSettingsForTesting scoped_system_permission(
+ system_permission_settings::ScopedSettingsForTesting scoped_system_permission(
ContentSettingsType::MEDIASTREAM_CAMERA, true);
RunTestSequence(
@@ -250,7 +250,7 @@
MicrophoneUsingTestWithSystemBlock) {
SetPermission(ContentSettingsType::MEDIASTREAM_MIC, CONTENT_SETTING_ALLOW);
- ScopedSystemPermissionSettingsForTesting scoped_system_permission(
+ system_permission_settings::ScopedSettingsForTesting scoped_system_permission(
ContentSettingsType::MEDIASTREAM_MIC, true);
RunTestSequence(
diff --git a/chrome/browser/ui/views/permissions/embedded_permission_prompt.cc b/chrome/browser/ui/views/permissions/embedded_permission_prompt.cc
index d320770..3380e553 100644
--- a/chrome/browser/ui/views/permissions/embedded_permission_prompt.cc
+++ b/chrome/browser/ui/views/permissions/embedded_permission_prompt.cc
@@ -179,12 +179,12 @@
// whereas the "OS Prompt" view is only higher priority then the views that
// are associated with a site-level allowed state.
// TODO(crbug.com/40275129): Handle going to Windows settings.
- if (SystemPermissionSettings::GetInstance()->IsDenied(type)) {
+ if (system_permission_settings::IsDenied(type)) {
return Variant::kOsSystemSettings;
}
if (setting == CONTENT_SETTING_ALLOW &&
- SystemPermissionSettings::GetInstance()->CanPrompt(type)) {
+ system_permission_settings::CanPrompt(type)) {
return Variant::kOsPrompt;
}
@@ -376,7 +376,7 @@
if (os_prompt_variant_ == Variant::kUninitialized) {
for (const auto& request : delegate()->Requests()) {
- if (SystemPermissionSettings::GetInstance()->CanPrompt(
+ if (system_permission_settings::CanPrompt(
request->GetContentSettingsType())) {
os_prompt_variant_ = Variant::kOsPrompt;
break;
@@ -386,7 +386,7 @@
if (os_system_settings_variant_ == Variant::kUninitialized) {
for (const auto& request : delegate()->Requests()) {
- if (SystemPermissionSettings::GetInstance()->IsDenied(
+ if (system_permission_settings::IsDenied(
request->GetContentSettingsType())) {
os_system_settings_variant_ = Variant::kOsSystemSettings;
break;
@@ -481,7 +481,7 @@
RecordOsMetrics(permissions::OsScreenAction::SYSTEM_SETTINGS);
RecordPermissionActionUKM(
permissions::ElementAnchoredBubbleAction::kSystemSettings);
- SystemPermissionSettings::GetInstance()->OpenSystemSettings(
+ system_permission_settings::OpenSystemSettings(
delegate()->GetAssociatedWebContents(),
requests_[0]->GetContentSettingsType());
}
@@ -516,7 +516,7 @@
prompt_types_.end());
for (unsigned int idx = 0; idx < types.size(); idx++) {
- SystemPermissionSettings::GetInstance()->Request(
+ system_permission_settings::Request(
types[idx],
base::BindOnce(
&EmbeddedPermissionPrompt::OnRequestSystemPermissionResponse,
@@ -531,14 +531,14 @@
const ContentSettingsType request_type,
const ContentSettingsType other_request_type) {
bool permission_determined =
- !SystemPermissionSettings::GetInstance()->CanPrompt(request_type);
+ !system_permission_settings::CanPrompt(request_type);
// `other_permission_determined` is left with true in non-grouped scenario,
// which would make the final logic fully rely on `permission_determined`.
auto other_permission_determined = true;
if (other_request_type != ContentSettingsType::DEFAULT) {
other_permission_determined =
- !SystemPermissionSettings::GetInstance()->CanPrompt(other_request_type);
+ !system_permission_settings::CanPrompt(other_request_type);
}
if (permission_determined) {
