From cbeffe5e7419bb964f158f41caa7dbf6f918a845 Mon Sep 17 00:00:00 2001 From: Claudio DeSouza Date: Thu, 19 Jun 2025 15:34:19 +0100 Subject: [PATCH] [cr139] `PermissionDecision` replaces `is_one_time` args `PermissionDecision::kAllowThisTime` is the equivalent of what was being provided through this argument. Chromium changes: https://chromium.googlesource.com/chromium/src/+/2cb5adab5b8a566cb47ab8f0c35621134a98695e commit 2cb5adab5b8a566cb47ab8f0c35621134a98695e Author: Florian Jacky Date: Wed Jun 18 13:21:50 2025 -0700 [PermissionOptions] Turn one time grants into a PermissionDecision Bug: 411025625 Change-Id: I84231344779e3be770dd9b97020159b7a47952d2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6632615 Reviewed-by: Nico Weber Commit-Queue: Florian Jacky Cr-Commit-Position: refs/heads/main@{#1475822} --- .../permission_lifetime_manager_unittest.cc | 40 +++++++++---------- .../widevine/widevine_permission_request.cc | 1 - .../widevine/widevine_permission_request.h | 1 - ...content_setting_permission_context_base.cc | 9 ++--- .../content_setting_permission_context_base.h | 1 - .../brave_wallet_permission_context.cc | 1 - .../permission_lifetime_manager.cc | 6 +-- .../permissions/permission_lifetime_manager.h | 3 +- 8 files changed, 27 insertions(+), 35 deletions(-) diff --git a/browser/permissions/permission_lifetime_manager_unittest.cc b/browser/permissions/permission_lifetime_manager_unittest.cc index aacf9827924..f5f89f00d52 100644 --- a/browser/permissions/permission_lifetime_manager_unittest.cc +++ b/browser/permissions/permission_lifetime_manager_unittest.cc @@ -254,7 +254,7 @@ TEST_F(PermissionLifetimeManagerTest, SetAndResetAfterExpiration) { content_setting)); const base::Time expected_expiration_time = base::Time::Now() + *request->GetLifetime(); - manager()->PermissionDecided(*request, kOrigin, kOrigin, decision, false); + manager()->PermissionDecided(*request, kOrigin, kOrigin, decision); EXPECT_TRUE(timer().IsRunning()); browser_task_environment_.RunUntilIdle(); @@ -293,7 +293,7 @@ TEST_F(PermissionLifetimeManagerTest, DifferentTypePermissions) { const base::Time expected_expiration_time = base::Time::Now() + *request->GetLifetime(); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_TRUE(timer().IsRunning()); browser_task_environment_.FastForwardBy(kOneSecond); @@ -303,7 +303,7 @@ TEST_F(PermissionLifetimeManagerTest, DifferentTypePermissions) { const base::Time expected_expiration_time2 = base::Time::Now() + *request2->GetLifetime(); manager()->PermissionDecided(*request2, kOrigin2, kOrigin2, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); browser_task_environment_.RunUntilIdle(); // Check data stored in prefs. @@ -345,7 +345,7 @@ TEST_F(PermissionLifetimeManagerTest, TwoPermissionsSameTime) { const base::Time expected_expiration_time = base::Time::Now() + *request->GetLifetime(); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_TRUE(timer().IsRunning()); browser_task_environment_.FastForwardBy(kOneSecond); @@ -355,7 +355,7 @@ TEST_F(PermissionLifetimeManagerTest, TwoPermissionsSameTime) { base::Time::Now() + *request2->GetLifetime(); ASSERT_EQ(expected_expiration_time, expected_expiration_time2); manager()->PermissionDecided(*request2, kOrigin2, kOrigin2, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); // Check data stored in prefs. CheckExpirationsPref( @@ -383,7 +383,7 @@ TEST_F(PermissionLifetimeManagerTest, TwoPermissionsBigTimeDifference) { const base::Time expected_expiration_time = base::Time::Now() + *request->GetLifetime(); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_TRUE(timer().IsRunning()); EXPECT_EQ(timer().desired_run_time(), expected_expiration_time); @@ -392,7 +392,7 @@ TEST_F(PermissionLifetimeManagerTest, TwoPermissionsBigTimeDifference) { const base::Time expected_expiration_time2 = base::Time::Now() + *request2->GetLifetime(); manager()->PermissionDecided(*request2, kOrigin2, kOrigin2, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); // Timer should be restarted. EXPECT_EQ(timer().desired_run_time(), expected_expiration_time2); @@ -429,7 +429,7 @@ TEST_F(PermissionLifetimeManagerTest, RestoreAfterRestart) { const base::Time expected_expiration_time = base::Time::Now() + *request->GetLifetime(); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_TRUE(timer().IsRunning()); ResetManager(); @@ -469,7 +469,7 @@ TEST_F(PermissionLifetimeManagerTest, ExpiredRestoreAfterRestart) { auto request(CreateRequestAndAllowContentSetting( kOrigin, ContentSettingsType::NOTIFICATIONS, kLifetime)); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_TRUE(timer().IsRunning()); ResetManager(); @@ -493,12 +493,12 @@ TEST_F(PermissionLifetimeManagerTest, PartiallyExpiredRestoreAfterRestart) { const base::Time expected_expiration_time = base::Time::Now() + *request->GetLifetime(); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); auto request2(CreateRequestAndAllowContentSetting( kOrigin2, ContentSettingsType::NOTIFICATIONS, kLifetime)); manager()->PermissionDecided(*request2, kOrigin2, kOrigin2, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); ResetManager(); browser_task_environment_.FastForwardBy(kLifetime); @@ -536,7 +536,7 @@ TEST_F(PermissionLifetimeManagerTest, ExternalContentSettingChange) { auto request(CreateRequestAndAllowContentSetting( kOrigin, ContentSettingsType::GEOLOCATION, kLifetime)); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_TRUE(timer().IsRunning()); host_content_settings_map_->SetContentSettingDefaultScope( @@ -556,12 +556,12 @@ TEST_F(PermissionLifetimeManagerTest, ClearAllExpiredAfterRestart) { auto request(CreateRequestAndAllowContentSetting( kOrigin, ContentSettingsType::NOTIFICATIONS, kOneSecond)); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); auto request2(CreateRequestAndAllowContentSetting( kOrigin2, ContentSettingsType::NOTIFICATIONS, kLifetime)); manager()->PermissionDecided(*request2, kOrigin2, kOrigin2, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); ResetManager(); browser_task_environment_.FastForwardBy(kLifetime); @@ -616,7 +616,7 @@ TEST_F(PermissionLifetimeManagerWithOriginMonitorTest, SubscribeToPermissionOriginDestruction(kOrigin)) .WillOnce(testing::Return(kOrigin.host())); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_FALSE(timer().IsRunning()); // Check data stored in prefs. @@ -652,9 +652,9 @@ TEST_F(PermissionLifetimeManagerWithOriginMonitorTest, SubscribeToPermissionOriginDestruction(kOrigin2)) .WillOnce(testing::Return(kOrigin2.host())); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); manager()->PermissionDecided(*request2, kOrigin2, kOrigin2, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_FALSE(timer().IsRunning()); // Check data stored in prefs. @@ -692,9 +692,9 @@ TEST_F(PermissionLifetimeManagerWithOriginMonitorTest, SubscribeToPermissionOriginDestruction(kOrigin2)) .WillOnce(testing::Return(kOrigin2.host())); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); manager()->PermissionDecided(*request2, kOrigin2, kOrigin2, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); EXPECT_TRUE(timer().IsRunning()); // Check data stored in prefs. @@ -734,7 +734,7 @@ TEST_F(PermissionLifetimeManagerWithOriginMonitorTest, SubscribeToPermissionOriginDestruction(kOrigin)) .WillOnce(testing::Return(std::string())); manager()->PermissionDecided(*request, kOrigin, kOrigin, - PermissionDecision::kAllow, false); + PermissionDecision::kAllow); // Nothing should be stored in prefs. CheckExpirationsPref(FROM_HERE, "{}"); diff --git a/browser/widevine/widevine_permission_request.cc b/browser/widevine/widevine_permission_request.cc index cf1fb2e15c3..e22533902f2 100644 --- a/browser/widevine/widevine_permission_request.cc +++ b/browser/widevine/widevine_permission_request.cc @@ -66,7 +66,6 @@ std::u16string WidevinePermissionRequest::GetMessageTextFragment() const { void WidevinePermissionRequest::PermissionDecided( PermissionDecision decision, - bool is_one_time, bool is_final_decision, const permissions::PermissionRequestData& request_data) { // Permission granted diff --git a/browser/widevine/widevine_permission_request.h b/browser/widevine/widevine_permission_request.h index 524f9017599..94cb8c1973e 100644 --- a/browser/widevine/widevine_permission_request.h +++ b/browser/widevine/widevine_permission_request.h @@ -42,7 +42,6 @@ class WidevinePermissionRequest : public permissions::PermissionRequest { #endif void PermissionDecided( PermissionDecision decision, - bool is_one_time, bool is_final_decision, const permissions::PermissionRequestData& request_data); diff --git a/chromium_src/components/permissions/content_setting_permission_context_base.cc b/chromium_src/components/permissions/content_setting_permission_context_base.cc index 9b68b08ce58..c14d727d3ad 100644 --- a/chromium_src/components/permissions/content_setting_permission_context_base.cc +++ b/chromium_src/components/permissions/content_setting_permission_context_base.cc @@ -47,7 +47,6 @@ void ContentSettingPermissionContextBase::SetPermissionLifetimeManagerFactory( void ContentSettingPermissionContextBase::PermissionDecided( PermissionDecision decision, - bool is_one_time, bool is_final_decision, const PermissionRequestData& request_data) { if (permission_lifetime_manager_factory_) { @@ -60,7 +59,7 @@ void ContentSettingPermissionContextBase::PermissionDecided( permission_lifetime_manager_factory_.Run(browser_context_)) { permission_lifetime_manager->PermissionDecided( *permission_request, request_data.requesting_origin, - request_data.embedding_origin, decision, is_one_time); + request_data.embedding_origin, decision); } } const auto group_request_it = @@ -73,7 +72,7 @@ void ContentSettingPermissionContextBase::PermissionDecided( permission_lifetime_manager_factory_.Run(browser_context_)) { permission_lifetime_manager->PermissionDecided( *permission_request, request_data.requesting_origin, - request_data.embedding_origin, decision, is_one_time); + request_data.embedding_origin, decision); } } } @@ -81,7 +80,7 @@ void ContentSettingPermissionContextBase::PermissionDecided( if (!IsGroupedPermissionType(content_settings_type())) { ContentSettingPermissionContextBase_ChromiumImpl::PermissionDecided( - decision, is_one_time, is_final_decision, request_data); + decision, is_final_decision, request_data); return; } @@ -106,7 +105,7 @@ void ContentSettingPermissionContextBase::PermissionDecided( auto callback = grouped_request->second->GetNextCallback(); if (callback) { NotifyPermissionSet(request_data, std::move(callback), persist, decision, - is_one_time, is_final_decision); + is_final_decision); } } diff --git a/chromium_src/components/permissions/content_setting_permission_context_base.h b/chromium_src/components/permissions/content_setting_permission_context_base.h index 0b1c403ddd4..dc7c4e08a69 100644 --- a/chromium_src/components/permissions/content_setting_permission_context_base.h +++ b/chromium_src/components/permissions/content_setting_permission_context_base.h @@ -82,7 +82,6 @@ class ContentSettingPermissionContextBase }; void PermissionDecided(PermissionDecision decision, - bool is_one_time, bool is_final_decision, const PermissionRequestData& request_data) override; void CleanUpRequest(content::WebContents* web_contents, diff --git a/components/permissions/contexts/brave_wallet_permission_context.cc b/components/permissions/contexts/brave_wallet_permission_context.cc index 11dcaf3546c..8df7cfd0fcf 100644 --- a/components/permissions/contexts/brave_wallet_permission_context.cc +++ b/components/permissions/contexts/brave_wallet_permission_context.cc @@ -90,7 +90,6 @@ void BraveWalletPermissionContext::RequestPermission( url::Origin::Create(web_contents->GetLastCommittedURL()).GetURL(); NotifyPermissionSet(*request_data, std::move(callback), /*persist=*/false, PermissionDecision::kDeny, - /*is_one_time=*/false, /*is_final_decision=*/true); return; } diff --git a/components/permissions/permission_lifetime_manager.cc b/components/permissions/permission_lifetime_manager.cc index 4825128ff9b..a0d4fc39731 100644 --- a/components/permissions/permission_lifetime_manager.cc +++ b/components/permissions/permission_lifetime_manager.cc @@ -85,12 +85,10 @@ void PermissionLifetimeManager::PermissionDecided( const PermissionRequest& permission_request, const GURL& requesting_origin, const GURL& embedding_origin, - PermissionDecision decision, - bool is_one_time) { + PermissionDecision decision) { if (!permission_request.SupportsLifetime() || (decision != PermissionDecision::kAllow && - decision != PermissionDecision::kDeny) || - is_one_time) { + decision != PermissionDecision::kDeny)) { // Only interested in ALLOW/BLOCK and non one-time (Chromium // geolocation-specific) decisions. return; diff --git a/components/permissions/permission_lifetime_manager.h b/components/permissions/permission_lifetime_manager.h index aa54388b634..592e66b7ac2 100644 --- a/components/permissions/permission_lifetime_manager.h +++ b/components/permissions/permission_lifetime_manager.h @@ -56,8 +56,7 @@ class PermissionLifetimeManager : public KeyedService, void PermissionDecided(const PermissionRequest& permission_request, const GURL& requesting_origin, const GURL& embedding_origin, - PermissionDecision decision, - bool is_one_time); + PermissionDecision decision); // content_settings::Observer: void OnContentSettingChanged(const ContentSettingsPattern& primary_pattern,