Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[cr138] Incorporate changes to PermissionRequest/PermissionRequestData

Browse Source 
PermissionRequestData is now used to pass various permission parameters around
and several functions/constructors were modified to accomodate that change.

Chromium change:
https://source.chromium.org/chromium/chromium/src/+/7f3e3ca1b6ceddc2e18a4fe86d2ce3ac936e68ba

commit 7f3e3ca1b6ceddc2e18a4fe86d2ce3ac936e68ba
Author: Florian Jacky <fjacky@google.com>
Date:   Mon May 5 09:19:20 2025 -0700

    [PermissionOptions] Simplify request path, provide resolvers, pass back request information

    This CL is part of a larger change to support permissions with options. A summary of changes in this CL can be found below, the full de>

    - Simplify interfaces to rely on PermissionRequestData wrapper and refactor PermissionContextBase and its subclasses accordingly.

    - Update PermissionRequestData to rely on permission resolvers.

    - Refactor PermissionRequest to rely on unique PermissionRequestData object and pass it to permission decided callbacks. Let Permission>

    Full design doc: http://go/multi-state-permissions-dd

    Change-Id: I81df3cbcedb9c7ef2340a0d79734adf77f336aae
    Bug: 394547183, 393053278
This commit is contained in:
Emerick Rogul
2025-06-11 18:56:27 +01:00
committed by Claudio DeSouza
parent b67eabb39e
commit 56f7ee1dc0
13 changed files with 89 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
browser/ai_chat/brave_open_ai_chat_permission_context_unittest.cc
+18 -8
View File
@@ -15,6 +15,7 @@
#include "components/permissions/permission_request_id.h"
#include "components/permissions/permission_request_manager.h"
#include "components/permissions/test/mock_permission_prompt_factory.h"
#include "content/public/browser/permission_descriptor_util.h"
#include "content/public/browser/permission_result.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
@@ -40,8 +41,8 @@ class BraveOpenAIChatPermissionContextTest
ContentSetting setting = ContentSetting::CONTENT_SETTING_DEFAULT;
base::RunLoop run_loop;
permission_context->RequestPermission(
PermissionRequestData(permission_context, id, /*user_gesture=*/true,
url),
std::make_unique<PermissionRequestData>(permission_context, id,
/*user_gesture=*/true, url),
base::BindLambdaForTesting([&](ContentSetting result) {
setting = result;
run_loop.Quit();
@@ -95,20 +96,29 @@ TEST_F(BraveOpenAIChatPermissionContextTest, NotAllowedInInsecureOrigins) {
EXPECT_EQ(content::PermissionStatus::DENIED,
permission_context
.GetPermissionStatus(nullptr /* render_frame_host */,
insecure_url, insecure_url)
.GetPermissionStatus(
content::PermissionDescriptorUtil::
CreatePermissionDescriptorForPermissionType(
blink::PermissionType::NOTIFICATIONS),
nullptr /* render_frame_host */, insecure_url, insecure_url)
.status);
EXPECT_EQ(content::PermissionStatus::DENIED,
permission_context
.GetPermissionStatus(nullptr /* render_frame_host */,
insecure_url, secure_url)
.GetPermissionStatus(
content::PermissionDescriptorUtil::
CreatePermissionDescriptorForPermissionType(
blink::PermissionType::NOTIFICATIONS),
nullptr /* render_frame_host */, insecure_url, secure_url)
.status);
EXPECT_EQ(content::PermissionStatus::ASK,
permission_context
.GetPermissionStatus(nullptr /* render_frame_host */,
secure_url, secure_url)
.GetPermissionStatus(
content::PermissionDescriptorUtil::
CreatePermissionDescriptorForPermissionType(
blink::PermissionType::NOTIFICATIONS),
nullptr /* render_frame_host */, secure_url, secure_url)
.status);
}
browser/geolocation/brave_geolocation_permission_context_delegate.cc
+2 -4
View File
@@ -23,9 +23,7 @@ BraveGeolocationPermissionContextDelegate::
~BraveGeolocationPermissionContextDelegate() = default;
bool BraveGeolocationPermissionContextDelegate::DecidePermission(
const permissions::PermissionRequestID& id,
const GURL& requesting_origin,
bool user_gesture,
const std::unique_ptr<permissions::PermissionRequestData>& request_data,
permissions::BrowserPermissionCallback* callback,
permissions::GeolocationPermissionContext* context) {
if (profile_->IsTor()) {
@@ -34,5 +32,5 @@ bool BraveGeolocationPermissionContextDelegate::DecidePermission(
}
return GeolocationPermissionContextDelegate::DecidePermission(
id, requesting_origin, user_gesture, callback, context);
std::move(request_data), callback, context);
}
browser/geolocation/brave_geolocation_permission_context_delegate.h
+1 -3
View File
@@ -23,9 +23,7 @@ class BraveGeolocationPermissionContextDelegate
~BraveGeolocationPermissionContextDelegate() override;
bool DecidePermission(
const permissions::PermissionRequestID& id,
const GURL& requesting_origin,
bool user_gesture,
const std::unique_ptr<permissions::PermissionRequestData>& request_data,
permissions::BrowserPermissionCallback* callback,
permissions::GeolocationPermissionContext* context) override;
browser/permissions/permission_lifetime_manager_unittest.cc
+5 -1
View File
@@ -25,6 +25,7 @@
#include "components/content_settings/core/browser/website_settings_registry.h"
#include "components/permissions/permission_request.h"
#include "components/permissions/request_type.h"
#include "components/permissions/resolvers/content_setting_permission_resolver.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
@@ -178,7 +179,10 @@ class PermissionLifetimeManagerTest : public testing::Test {
ExpectContentSetting(FROM_HERE, origin, content_type, content_setting);
auto request = std::make_unique<PermissionRequest>(
origin, ContentSettingsTypeToRequestType(content_type), true,
std::make_unique<permissions::PermissionRequestData>(
std::make_unique<permissions::ContentSettingPermissionResolver>(
ContentSettingsTypeToRequestType(content_type)),
/*user_gesture=*/true, origin),
PermissionDecidedCallback(), base::OnceClosure());
request->SetLifetime(lifetime);
return request;
browser/widevine/widevine_permission_request.cc
+11 -6
View File
@@ -13,6 +13,7 @@
#include "chrome/browser/lifetime/application_lifetime.h"
#include "chrome/browser/profiles/profile.h"
#include "components/permissions/request_type.h"
#include "components/permissions/resolvers/content_setting_permission_resolver.h"
#include "components/prefs/pref_service.h"
#include "components/url_formatter/elide_url.h"
#include "components/vector_icons/vector_icons.h"
@@ -27,9 +28,11 @@ WidevinePermissionRequest::WidevinePermissionRequest(
content::WebContents* web_contents,
bool for_restart)
: PermissionRequest(
web_contents->GetVisibleURL(),
permissions::RequestType::kWidevine,
/*has_gesture=*/false,
std::make_unique<permissions::PermissionRequestData>(
std::make_unique<permissions::ContentSettingPermissionResolver>(
permissions::RequestType::kWidevine),
false,
web_contents->GetVisibleURL()),
base::BindRepeating(&WidevinePermissionRequest::PermissionDecided,
base::Unretained(this)),
base::BindOnce(&WidevinePermissionRequest::DeleteRequest,
@@ -58,9 +61,11 @@ std::u16string WidevinePermissionRequest::GetMessageTextFragment() const {
}
#endif
void WidevinePermissionRequest::PermissionDecided(ContentSetting result,
bool is_one_time,
bool is_final_decision) {
void WidevinePermissionRequest::PermissionDecided(
ContentSetting result,
bool is_one_time,
bool is_final_decision,
const std::unique_ptr<permissions::PermissionRequestData>& request_data) {
// Permission granted
if (result == ContentSetting::CONTENT_SETTING_ALLOW) {
if (!for_restart_) {
browser/widevine/widevine_permission_request.h
+5 -3
View File
@@ -41,9 +41,11 @@ class WidevinePermissionRequest : public permissions::PermissionRequest {
#else
std::u16string GetMessageTextFragment() const override;
#endif
void PermissionDecided(ContentSetting result,
bool is_one_time,
bool is_final_decision);
void PermissionDecided(
ContentSetting result,
bool is_one_time,
bool is_final_decision,
const std::unique_ptr<permissions::PermissionRequestData>& request_data);
void DeleteRequest();
// It's safe to use this raw |web_contents_| because this request is deleted
chromium_src/chrome/browser/storage_access_api/storage_access_grant_permission_context.cc
+3 -4
View File
@@ -4,10 +4,9 @@
* You can obtain one at https://mozilla.org/MPL/2.0/. */
#define BRAVE_STORAGE_ACCESS_GRANT_PERMISSION_CONTEXT_CHECK_FOR_AUTO_GRANT_OR_AUTO_DENIAL \
NotifyPermissionSetInternal( \
request_data.id, request_data.requesting_origin, \
request_data.embedding_origin, std::move(callback), /*persist=*/true, \
CONTENT_SETTING_BLOCK, RequestOutcome::kDeniedByPrerequisites); \
NotifyPermissionSetInternal(std::move(request_data), std::move(callback), \
/*persist=*/true, CONTENT_SETTING_BLOCK, \
RequestOutcome::kDeniedByPrerequisites); \
return;
#include "src/chrome/browser/storage_access_api/storage_access_grant_permission_context.cc"
chromium_src/components/permissions/permission_context_base.cc
+22 -22
View File
@@ -44,14 +44,13 @@ void PermissionContextBase::SetPermissionLifetimeManagerFactory(
permission_lifetime_manager_factory_ = factory;
}
void PermissionContextBase::PermissionDecided(const PermissionRequestID& id,
const GURL& requesting_origin,
const GURL& embedding_origin,
ContentSetting content_setting,
bool is_one_time,
bool is_final_decision) {
void PermissionContextBase::PermissionDecided(
ContentSetting content_setting,
bool is_one_time,
bool is_final_decision,
const std::unique_ptr<PermissionRequestData>& request_data) {
if (permission_lifetime_manager_factory_) {
const auto request_it = pending_requests_.find(id.ToString());
const auto request_it = pending_requests_.find(request_data->id.ToString());
if (request_it != pending_requests_.end()) {
const PermissionRequest* permission_request =
request_it->second.first.get();
@@ -59,11 +58,12 @@ void PermissionContextBase::PermissionDecided(const PermissionRequestID& id,
if (auto* permission_lifetime_manager =
permission_lifetime_manager_factory_.Run(browser_context_)) {
permission_lifetime_manager->PermissionDecided(
*permission_request, requesting_origin, embedding_origin,
content_setting, is_one_time);
*permission_request, request_data->requesting_origin,
request_data->embedding_origin, content_setting, is_one_time);
}
}
const auto group_request_it = pending_grouped_requests_.find(id.ToString());
const auto group_request_it =
pending_grouped_requests_.find(request_data->id.ToString());
if (group_request_it != pending_grouped_requests_.end()) {
for (const auto& request : group_request_it->second->Requests()) {
const PermissionRequest* permission_request = request.first.get();
@@ -71,8 +71,8 @@ void PermissionContextBase::PermissionDecided(const PermissionRequestID& id,
if (auto* permission_lifetime_manager =
permission_lifetime_manager_factory_.Run(browser_context_)) {
permission_lifetime_manager->PermissionDecided(
*permission_request, requesting_origin, embedding_origin,
content_setting, is_one_time);
*permission_request, request_data->requesting_origin,
request_data->embedding_origin, content_setting, is_one_time);
}
}
}
@@ -80,20 +80,21 @@ void PermissionContextBase::PermissionDecided(const PermissionRequestID& id,
if (!IsGroupedPermissionType(content_settings_type())) {
PermissionContextBase_ChromiumImpl::PermissionDecided(
id, requesting_origin, embedding_origin, content_setting, is_one_time,
is_final_decision);
content_setting, is_one_time, is_final_decision,
std::move(request_data));
return;
}
DCHECK(content_setting == CONTENT_SETTING_ALLOW ||
content_setting == CONTENT_SETTING_BLOCK ||
content_setting == CONTENT_SETTING_DEFAULT);
UserMadePermissionDecision(id, requesting_origin, embedding_origin,
content_setting);
UserMadePermissionDecision(request_data->id, request_data->requesting_origin,
request_data->embedding_origin, content_setting);
bool persist = content_setting != CONTENT_SETTING_DEFAULT;
auto grouped_request = pending_grouped_requests_.find(id.ToString());
auto grouped_request =
pending_grouped_requests_.find(request_data->id.ToString());
DCHECK(grouped_request != pending_grouped_requests_.end());
DCHECK(grouped_request->second);
@@ -103,16 +104,15 @@ void PermissionContextBase::PermissionDecided(const PermissionRequestID& id,
auto callback = grouped_request->second->GetNextCallback();
if (callback) {
NotifyPermissionSet(id, requesting_origin, embedding_origin,
std::move(callback), persist, content_setting,
is_one_time, is_final_decision);
NotifyPermissionSet(request_data, std::move(callback), persist,
content_setting, is_one_time, is_final_decision);
}
}
void PermissionContextBase::DecidePermission(
permissions::PermissionRequestData request_data,
std::unique_ptr<permissions::PermissionRequestData> request_data,
BrowserPermissionCallback callback) {
auto id = request_data.id;
auto id = request_data->id;
PermissionContextBase_ChromiumImpl::DecidePermission(std::move(request_data),
std::move(callback));
chromium_src/components/permissions/permission_context_base.h
+8 -8
View File
@@ -49,8 +49,9 @@ class PermissionContextBase : public PermissionContextBase_ChromiumImpl {
const base::RepeatingCallback<
PermissionLifetimeManager*(content::BrowserContext*)>& factory);
void DecidePermission(permissions::PermissionRequestData request_data,
BrowserPermissionCallback callback) override;
void DecidePermission(
std::unique_ptr<permissions::PermissionRequestData> request_data,
BrowserPermissionCallback callback) override;
bool IsPendingGroupedRequestsEmptyForTesting();
@@ -87,12 +88,11 @@ class PermissionContextBase : public PermissionContextBase_ChromiumImpl {
size_t next_callback_index_ = 0;
};
void PermissionDecided(const PermissionRequestID& id,
const GURL& requesting_origin,
const GURL& embedding_origin,
ContentSetting content_setting,
bool is_one_time,
bool is_final_decision) override;
void PermissionDecided(
ContentSetting content_setting,
bool is_one_time,
bool is_final_decision,
const std::unique_ptr<PermissionRequestData>& request_data) override;
void CleanUpRequest(content::WebContents* web_contents,
const PermissionRequestID& id,
bool embedded_permission_element_initiated) override;
chromium_src/components/permissions/permission_request.cc
+1 -13
View File
@@ -99,19 +99,7 @@ const unsigned int IDS_VR_PERMISSION_FRAGMENT_OVERRIDE =
namespace permissions {
PermissionRequest::PermissionRequest(
const GURL& requesting_origin,
RequestType request_type,
bool has_gesture,
PermissionDecidedCallback permission_decided_callback,
base::OnceClosure delete_callback)
: PermissionRequest_ChromiumImpl(requesting_origin,
request_type,
has_gesture,
std::move(permission_decided_callback),
std::move(delete_callback)) {}
PermissionRequest::PermissionRequest(
PermissionRequestData request_data,
std::unique_ptr<PermissionRequestData> request_data,
PermissionDecidedCallback permission_decided_callback,
base::OnceClosure delete_callback,
bool uses_automatic_embargo)
chromium_src/components/permissions/permission_request.h
+2 -8
View File
@@ -29,16 +29,10 @@ namespace permissions {
class PermissionRequest : public PermissionRequest_ChromiumImpl {
public:
PermissionRequest(const GURL& requesting_origin,
RequestType request_type,
bool has_gesture,
PermissionDecidedCallback permission_decided_callback,
base::OnceClosure delete_callback);
PermissionRequest(PermissionRequestData request_data,
PermissionRequest(std::unique_ptr<PermissionRequestData> request_data,
PermissionDecidedCallback permission_decided_callback,
base::OnceClosure delete_callback,
bool uses_automatic_embargo);
bool uses_automatic_embargo = true);
PermissionRequest(const PermissionRequest&) = delete;
PermissionRequest& operator=(const PermissionRequest&) = delete;
components/permissions/contexts/brave_wallet_permission_context.cc
+10 -10
View File
@@ -65,11 +65,11 @@ bool BraveWalletPermissionContext::IsRestrictedToSecureOrigins() const {
}
void BraveWalletPermissionContext::RequestPermission(
PermissionRequestData request_data,
std::unique_ptr<PermissionRequestData> request_data,
BrowserPermissionCallback callback) {
const std::string id_str = request_data.id.ToString();
const std::string id_str = request_data->id.ToString();
url::Origin requesting_origin =
url::Origin::Create(request_data.requesting_origin);
url::Origin::Create(request_data->requesting_origin);
url::Origin origin;
permissions::RequestType type =
ContentSettingsTypeToRequestType(content_settings_type());
@@ -83,13 +83,12 @@ void BraveWalletPermissionContext::RequestPermission(
type, requesting_origin, &origin,
is_new_id ? &address_queue : nullptr)) {
content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(
request_data.id.global_render_frame_host_id());
request_data->id.global_render_frame_host_id());
content::WebContents* web_contents =
content::WebContents::FromRenderFrameHost(rfh);
GURL embedding_origin =
url::Origin::Create(web_contents->GetLastCommittedURL()).GetURL();
NotifyPermissionSet(request_data.id, requesting_origin.GetURL(),
embedding_origin, std::move(callback),
NotifyPermissionSet(std::move(request_data), std::move(callback),
/*persist=*/false, CONTENT_SETTING_BLOCK,
/*is_one_time=*/false,
/*is_final_decision=*/true);
@@ -114,12 +113,13 @@ void BraveWalletPermissionContext::RequestPermission(
if (addr_queue.empty()) {
request_address_queues_.erase(addr_queue_it);
}
auto data =
PermissionRequestData(this, request_data.id, request_data.user_gesture,
sub_request_origin->GetURL());
std::unique_ptr<PermissionRequestData> data =
std::make_unique<PermissionRequestData>(this, request_data->id,
request_data->user_gesture,
sub_request_origin->GetURL());
// This will prevent PermissionRequestManager from reprioritize the request
// queue.
data.embedded_permission_element_initiated = true;
data->embedded_permission_element_initiated = true;
PermissionContextBase::RequestPermission(std::move(data),
std::move(callback));
}
components/permissions/contexts/brave_wallet_permission_context.h
+1 -1
View File
@@ -45,7 +45,7 @@ class BraveWalletPermissionContext : public PermissionContextBase {
* will then consume one address from the saved list and call
* PermissionContextBase::RequestPermission with it.
*/
void RequestPermission(PermissionRequestData request_data,
void RequestPermission(std::unique_ptr<PermissionRequestData> request_data,
BrowserPermissionCallback callback) override;
static void RequestPermissions(