Check non-empty precondition before peek and pop permissions queue

(cherry picked from commit 944ef12)

Bug: 1497867
Change-Id: Ie9a32813eb45bd8ff707572d66264bcbd8d46b15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4993484
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1217735}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4994308
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/6099@{#14}
Cr-Branched-From: e6ee450-refs/heads/main@{#1217362}

components/permissions/permission_request_manager.cc

@@ -283,7 +283,7 @@ void PermissionRequestManager::AddRequest(
 }
 
 bool PermissionRequestManager::ReprioritizeCurrentRequestIfNeeded() {
-  if (pending_permission_requests_.IsEmpty() || !IsRequestInProgress() ||
+  if (!IsRequestInProgress() ||
       IsCurrentRequestEmbeddedPermissionElementInitiated()) {
     return true;
   }
@@ -294,6 +294,10 @@ bool PermissionRequestManager::ReprioritizeCurrentRequestIfNeeded() {
     pending_permission_requests_.Pop();
   }
 
+  if (pending_permission_requests_.IsEmpty()) {
+    return true;
+  }
+
   auto current_request_fate = CurrentRequestFate::kKeepCurrent;
 
   if (PermissionUtil::DoesPlatformSupportChip()) {
@@ -324,6 +328,7 @@ bool PermissionRequestManager::ReprioritizeCurrentRequestIfNeeded() {
   }
 
   if (current_request_fate == CurrentRequestFate::kKeepCurrent &&
+      !pending_permission_requests_.IsEmpty() &&
       pending_permission_requests_.Peek()
           ->IsEmbeddedPermissionElementInitiated()) {
     current_request_fate = CurrentRequestFate::kPreempt;
@@ -333,7 +338,7 @@ bool PermissionRequestManager::ReprioritizeCurrentRequestIfNeeded() {
     case CurrentRequestFate::kKeepCurrent:
       return true;
     case CurrentRequestFate::kPreempt: {
-      DCHECK(!pending_permission_requests_.IsEmpty());
+      CHECK(!pending_permission_requests_.IsEmpty());
       auto* next_candidate = pending_permission_requests_.Peek();
 
       // Consider a case of infinite loop here (eg: 2 low priority requests can

components/permissions/permission_request_queue.cc

@@ -16,15 +16,7 @@ PermissionRequestQueue::PermissionRequestQueue()
 PermissionRequestQueue::~PermissionRequestQueue() = default;
 
 bool PermissionRequestQueue::IsEmpty() const {
-  return !Count();
-}
-
-size_t PermissionRequestQueue::Count() const {
-  size_t count = 0;
-  for (const auto& request_list : queued_requests_) {
-    count += request_list.size();
-  }
-  return count;
+  return !size_;
 }
 
 size_t PermissionRequestQueue::Count(PermissionRequest* request) const {
@@ -68,22 +60,27 @@ void PermissionRequestQueue::PushBack(permissions::PermissionRequest* request) {
 
 PermissionRequest* PermissionRequestQueue::Pop() {
   std::vector<base::circular_deque<PermissionRequest*>>::reverse_iterator it;
+  CHECK(!IsEmpty());
   // Skip entries that contain empty queues.
   for (it = queued_requests_.rbegin();
        it != queued_requests_.rend() && it->empty(); ++it) {
   }
+  CHECK(it != queued_requests_.rend());
   PermissionRequest* front = it->front();
   it->pop_front();
+  --size_;
   return front;
 }
 
 PermissionRequest* PermissionRequestQueue::Peek() const {
+  CHECK(!IsEmpty());
   std::vector<base::circular_deque<PermissionRequest*>>::const_reverse_iterator
       it;
   // Skip entries that contain empty queues.
   for (it = queued_requests_.rbegin();
        it != queued_requests_.rend() && it->empty(); ++it) {
   }
+  CHECK(it != queued_requests_.rend());
   return it->front();
 }
 
@@ -128,12 +125,14 @@ void PermissionRequestQueue::PushFrontInternal(
     permissions::PermissionRequest* request,
     Priority priority) {
   queued_requests_[static_cast<size_t>(priority)].push_front(request);
+  ++size_;
 }
 
 void PermissionRequestQueue::PushBackInternal(
     permissions::PermissionRequest* request,
     Priority priority) {
   queued_requests_[static_cast<size_t>(priority)].push_back(request);
+  ++size_;
 }
 
 }  // namespace permissions

components/permissions/permission_request_queue.h

@@ -41,8 +41,8 @@ class PermissionRequestQueue {
   ~PermissionRequestQueue();
 
   bool IsEmpty() const;
-  size_t Count() const;
   size_t Count(PermissionRequest* request) const;
+  size_t size() const { return size_; }
 
   // Push a new request into queue. This function will decide based on request
   // priority and platform whether to call |PushBack| or |PushFront|.
@@ -92,6 +92,8 @@ class PermissionRequestQueue {
   // priorities have strictly ascending, contignous values from lowest to
   // highest.
   std::vector<base::circular_deque<PermissionRequest*>> queued_requests_;
+
+  size_t size_{0};
 };
 
 }  // namespace permissions

components/permissions/permission_request_queue_unittest.cc

@@ -49,22 +49,22 @@ class PermissionRequestQueueTest : public ::testing::Test {
 };
 
 TEST_F(PermissionRequestQueueTest, CountNumberOfRequestsInQueue) {
-  EXPECT_EQ(0ul, permission_request_queue_.Count());
+  EXPECT_EQ(0ul, permission_request_queue_.size());
 
   permission_request_queue_.Push(&request_normal1_);
   permission_request_queue_.Push(&request_normal2_);
-  EXPECT_EQ(2ul, permission_request_queue_.Count());
+  EXPECT_EQ(2ul, permission_request_queue_.size());
 
   permission_request_queue_.Pop();
-  EXPECT_EQ(1ul, permission_request_queue_.Count());
+  EXPECT_EQ(1ul, permission_request_queue_.size());
 }
 
 TEST_F(PermissionRequestQueueTest, CountDuplicateRequests) {
-  EXPECT_EQ(0ul, permission_request_queue_.Count());
+  EXPECT_EQ(0ul, permission_request_queue_.size());
 
   permission_request_queue_.Push(&request_normal1_);
   permission_request_queue_.Push(&request_normal1_);
-  EXPECT_EQ(2ul, permission_request_queue_.Count());
+  EXPECT_EQ(2ul, permission_request_queue_.size());
 }
 
 TEST_F(PermissionRequestQueueTest, CountNumberOfRequestOccurencesInQueue) {