diff --git a/browser/permissions/permission_manager_browsertest.cc b/browser/permissions/permission_manager_browsertest.cc index 85baa1747971..4e217b0b4b43 100644 --- a/browser/permissions/permission_manager_browsertest.cc +++ b/browser/permissions/permission_manager_browsertest.cc @@ -8,6 +8,7 @@ #include "base/functional/bind.h" #include "base/memory/raw_ptr.h" #include "base/ranges/algorithm.h" +#include "base/test/mock_callback.h" #include "base/test/scoped_feature_list.h" #include "brave/components/brave_wallet/browser/permission_utils.h" #include "brave/components/brave_wallet/common/features.h" @@ -38,6 +39,8 @@ #include "url/gurl.h" #include "url/origin.h" +using testing::ElementsAreArray; + namespace permissions { namespace { @@ -169,9 +172,16 @@ IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest, RequestPermissions) { auto observer = std::make_unique( permission_request_manager); + base::MockCallback&)>> + callback; + EXPECT_CALL(callback, + Run(ElementsAreArray({blink::mojom::PermissionStatus::ASK, + blink::mojom::PermissionStatus::ASK}))) + .Times(1); permission_manager()->RequestPermissionsForOrigin( permissions, web_contents()->GetPrimaryMainFrame(), origin.GetURL(), - true, base::DoNothing()); + true, callback.Get()); content::RunAllTasksUntilIdle(); @@ -193,6 +203,7 @@ IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest, RequestPermissions) { // Test dismissing request. permissions::BraveWalletPermissionContext::Cancel(web_contents()); + testing::Mock::VerifyAndClearExpectations(&callback); EXPECT_TRUE(observer->IsRequestsFinalized()) << "case: " << i; EXPECT_TRUE(!observer->IsShowingBubble()) << "case: " << i; EXPECT_TRUE(IsPendingGroupedRequestsEmpty(cases[i].type)) << "case: " << i; @@ -206,9 +217,14 @@ IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest, RequestPermissions) { } observer->Reset(); + EXPECT_CALL( + callback, + Run(ElementsAreArray({blink::mojom::PermissionStatus::ASK, + blink::mojom::PermissionStatus::GRANTED}))) + .Times(1); permission_manager()->RequestPermissionsForOrigin( permissions, web_contents()->GetPrimaryMainFrame(), origin.GetURL(), - true, base::DoNothing()); + true, callback.Get()); content::RunAllTasksUntilIdle(); EXPECT_TRUE(permission_request_manager->IsRequestInProgress()) @@ -232,6 +248,7 @@ IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest, RequestPermissions) { std::vector{addresses[1]}, brave_wallet::mojom::PermissionLifetimeOption::kForever, web_contents()); + testing::Mock::VerifyAndClearExpectations(&callback); std::vector expected_settings( {ContentSetting::CONTENT_SETTING_ASK, ContentSetting::CONTENT_SETTING_ALLOW}); diff --git a/chromium_src/components/permissions/permission_request_manager.cc b/chromium_src/components/permissions/permission_request_manager.cc index 36e4ef7a9407..84a068610e43 100644 --- a/chromium_src/components/permissions/permission_request_manager.cc +++ b/chromium_src/components/permissions/permission_request_manager.cc @@ -5,6 +5,7 @@ #include +#include "base/containers/contains.h" #include "brave/components/brave_wallet/browser/permission_utils.h" #include "src/components/permissions/permission_request_manager.cc" #include "url/origin.h" @@ -34,7 +35,8 @@ bool PermissionRequestManager::ShouldGroupRequests(PermissionRequest* a, // Accept/Deny/Cancel each sub-request, total size of all passed in requests // should be equal to current requests_size because we will finalizing all -// current requests in the end. +// current requests in the end. The request callbacks will be called in FIFO +// order. void PermissionRequestManager::AcceptDenyCancel( const std::vector& accepted_requests, const std::vector& denied_requests, @@ -46,17 +48,20 @@ void PermissionRequestManager::AcceptDenyCancel( DCHECK((accepted_requests.size() + denied_requests.size() + cancelled_requests.size()) == requests_.size()); - for (PermissionRequest* request : accepted_requests) { - PermissionGrantedIncludingDuplicates(request, - /*is_one_time=*/false); - } - - for (PermissionRequest* request : denied_requests) { - PermissionDeniedIncludingDuplicates(request); - } - - for (PermissionRequest* request : cancelled_requests) { - CancelledIncludingDuplicates(request); + // We need to process requests in reverse order because + // PermissionRequestQueue impelementation of Push and Pop are paired with + // base::circular_dequeue's (push_back, pop_back) and (push_front, pop_front) + // Once pending_permission_requests_ is popped to the vector request_, the + // order is fixed because the reorder takes place in + // pending_permission_requests_. + for (auto it = requests_.crbegin(); it != requests_.crend(); ++it) { + if (base::Contains(accepted_requests, *it)) { + PermissionGrantedIncludingDuplicates(*it, /*is_one_time=*/false); + } else if (base::Contains(denied_requests, *it)) { + PermissionDeniedIncludingDuplicates(*it); + } else { + CancelledIncludingDuplicates(*it); + } } // Finalize permission with granted if some sub-requests are accepted. If