[PEPC] Rework the priority queue to always put PEPC requests first

Define a 3rd type of priority for requests to corresponds to PEPC requests.

In order to simplify the complicated logic on the queue, reworked it
to always be front-to-back. `Pop` and `Peek` will always return the front request. The `Push` function will decide whether to push requests to the front or back instead.

Rework the PRM logic that decides the current request fate to always
allow PEPC requests to preempt the current request (if it's not also a
PEPC request).

Bug: 1462930
Change-Id: Icaa695be1e35beb2f4806ed5b3ae1d1c2dd3b894
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4915775
Reviewed-by: Thomas Nguyen <tungnh@chromium.org>
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1210402}

components/permissions/permission_request.cc

@@ -325,4 +325,10 @@ std::u16string PermissionRequest::GetPermissionNameTextFragment() const {
   return l10n_util::GetStringUTF16(message_id);
 }
 
+void PermissionRequest::SetEmbeddedPermissionElementInitiatedForTesting(
+    bool embedded_permission_element_initiated) {
+  data_.embedded_permission_element_initiated =
+      embedded_permission_element_initiated;
+}
+
 }  // namespace permissions

components/permissions/permission_request.h

@@ -168,6 +168,12 @@ class PermissionRequest {
   // identify the permission being requested.
   virtual std::u16string GetPermissionNameTextFragment() const;
 
+ protected:
+  // Sets whether this request is permission element initiated, for testing
+  // subclasses only.
+  void SetEmbeddedPermissionElementInitiatedForTesting(
+      bool embedded_permission_element_initiated);
+
  private:
   PermissionRequestData data_;
 

components/permissions/permission_request_manager.cc

@@ -283,8 +283,16 @@ void PermissionRequestManager::AddRequest(
 }
 
 bool PermissionRequestManager::ReprioritizeCurrentRequestIfNeeded() {
-  if (pending_permission_requests_.IsEmpty() || !IsRequestInProgress())
+  if (pending_permission_requests_.IsEmpty() || !IsRequestInProgress() ||
+      IsCurrentRequestEmbeddedPermissionElementInitiated()) {
     return true;
+  }
+
+  // Pop out all invalid requests in front of the queue.
+  while (!pending_permission_requests_.IsEmpty() &&
+         !ValidateRequest(pending_permission_requests_.Peek())) {
+    pending_permission_requests_.Pop();
+  }
 
   auto current_request_fate = CurrentRequestFate::kKeepCurrent;
 
@@ -298,12 +306,6 @@ bool PermissionRequestManager::ReprioritizeCurrentRequestIfNeeded() {
       if (ShouldCurrentRequestUseQuietUI()) {
         current_request_fate = CurrentRequestFate::kPreempt;
       } else {
-        // Pop out all invalid requests in front of the queue.
-        while (!pending_permission_requests_.IsEmpty() &&
-               !ValidateRequest(pending_permission_requests_.Peek())) {
-          pending_permission_requests_.Pop();
-        }
-
         // Here we also try to prioritise the requests. If there's a valid high
         // priority request (high acceptance rate request) in the pending queue,
         // preempt the current request. The valid high priority request, if
@@ -321,6 +323,12 @@ bool PermissionRequestManager::ReprioritizeCurrentRequestIfNeeded() {
     current_request_fate = CurrentRequestFate::kFinalize;
   }
 
+  if (current_request_fate == CurrentRequestFate::kKeepCurrent &&
+      pending_permission_requests_.Peek()
+          ->IsEmbeddedPermissionElementInitiated()) {
+    current_request_fate = CurrentRequestFate::kPreempt;
+  }
+
   switch (current_request_fate) {
     case CurrentRequestFate::kKeepCurrent:
       return true;
@@ -339,7 +347,7 @@ bool PermissionRequestManager::ReprioritizeCurrentRequestIfNeeded() {
 
       pending_permission_requests_.Pop();
       PreemptAndRequeueCurrentRequest();
-      pending_permission_requests_.Push(next_candidate);
+      pending_permission_requests_.PushFront(next_candidate);
       ScheduleDequeueRequestIfNeeded();
       return false;
     }
@@ -377,16 +385,15 @@ bool PermissionRequestManager::ValidateRequest(PermissionRequest* request,
 void PermissionRequestManager::QueueRequest(
     content::RenderFrameHost* source_frame,
     PermissionRequest* request) {
-  pending_permission_requests_.Push(request,
-                                    true /*reorder_based_on_priority*/);
+  pending_permission_requests_.Push(request);
   request_sources_map_.emplace(
       request, PermissionRequestSource({source_frame->GetGlobalId()}));
 }
 
 void PermissionRequestManager::PreemptAndRequeueCurrentRequest() {
   ResetViewStateForCurrentRequest();
   for (auto* current_request : requests_) {
-    pending_permission_requests_.Push(current_request);
+    pending_permission_requests_.PushFront(current_request);
   }
 
   // Because the order of the requests is changed, we should not preignore it.
@@ -818,9 +825,11 @@ void PermissionRequestManager::DequeueRequestIfNeeded() {
   // Mark the remaining pending requests as validated, so only the "new and has
   // not been validated" requests added to the queue could have effect to
   // priority order
-  for (auto* request : pending_permission_requests_) {
-    if (ValidateRequest(request, /* should_finalize */ false)) {
-      validated_requests_set_.insert(request);
+  for (const auto& request_list : pending_permission_requests_) {
+    for (auto* request : request_list) {
+      if (ValidateRequest(request, /* should_finalize */ false)) {
+        validated_requests_set_.insert(request);
+      }
     }
   }
 
@@ -1449,6 +1458,12 @@ void PermissionRequestManager::DoAutoResponseForTesting() {
   }
 }
 
+bool PermissionRequestManager::
+    IsCurrentRequestEmbeddedPermissionElementInitiated() const {
+  return IsRequestInProgress() &&
+         requests_[0]->IsEmbeddedPermissionElementInitiated();
+}
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(PermissionRequestManager);
 
 }  // namespace permissions

components/permissions/permission_request_manager.h

@@ -395,6 +395,10 @@ class PermissionRequestManager
 
   void PreIgnoreQuietPromptInternal();
 
+  // Returns true if there is a request in progress that is initiated by an
+  // embedded permission element.
+  bool IsCurrentRequestEmbeddedPermissionElementInitiated() const;
+
   // Factory to be used to create views when needed.
   PermissionPrompt::Factory view_factory_;
 

components/permissions/permission_request_queue.cc

@@ -5,83 +5,94 @@
 #include "components/permissions/permission_request_queue.h"
 
 #include "base/ranges/algorithm.h"
-#include "components/permissions/features.h"
+#include "components/permissions/permission_request.h"
 #include "components/permissions/permission_util.h"
 
 namespace permissions {
 
-PermissionRequestQueue::PermissionRequestQueue() = default;
+PermissionRequestQueue::PermissionRequestQueue()
+    : queued_requests_(static_cast<size_t>(Priority::kNum)) {}
 
 PermissionRequestQueue::~PermissionRequestQueue() = default;
 
 bool PermissionRequestQueue::IsEmpty() const {
-  return queued_requests_.empty();
+  return !Count();
 }
 
 size_t PermissionRequestQueue::Count() const {
-  return queued_requests_.size();
+  size_t count = 0;
+  for (const auto& request_list : queued_requests_) {
+    count += request_list.size();
+  }
+  return count;
 }
 
 size_t PermissionRequestQueue::Count(PermissionRequest* request) const {
-  return base::ranges::count(queued_requests_, request);
+  size_t count = 0;
+  for (const auto& request_list : queued_requests_) {
+    count += base::ranges::count(request_list, request);
+  }
+  return count;
 }
 
-void PermissionRequestQueue::PushInternal(PermissionRequest* request) {
-  queued_requests_.push_back(request);
-}
+void PermissionRequestQueue::Push(PermissionRequest* request) {
+  Priority priority = DetermineRequestPriority(request);
 
-void PermissionRequestQueue::Push(PermissionRequest* request,
-                                  bool reorder_based_on_priority) {
-  if (!reorder_based_on_priority) {
-    PushInternal(request);
+  // High priority requests are always pushed to the back since they don't use
+  // the chip.
+  if (priority == Priority::kHigh) {
+    PushBackInternal(request, priority);
     return;
   }
 
+  // If the platform does not support the chip, push to the back.
   if (!PermissionUtil::DoesPlatformSupportChip()) {
-    PushInternal(request);
+    PushBackInternal(request, priority);
     return;
   }
 
-  // There're situations we need to take the priority into consideration (eg:
-  // when the permission chip is enabled). In such cases,
-  // push the new request to front of queue if it has high priority. Otherwise,
-  // insert the request after the first low priority request.
-  // Note that, the queue processing order is FIFO, but we have to iterate the
-  // queue in reverse order (see |PushInternal|)
-  if (queued_requests_.empty() ||
-      !PermissionUtil::IsLowPriorityPermissionRequest(request)) {
-    PushInternal(request);
-    return;
-  }
+  // Otherwise push to the front since chip requests use FILO ordering.
+  PushFrontInternal(request, priority);
+}
 
-  PermissionRequestQueue::const_reverse_iterator iter =
-      queued_requests_.rbegin();
-  for (; iter != queued_requests_.rend() &&
-         !PermissionUtil::IsLowPriorityPermissionRequest(*iter);
-       ++iter) {
-  }
+void PermissionRequestQueue::PushFront(
+    permissions::PermissionRequest* request) {
+  Priority priority = DetermineRequestPriority(request);
+  PushFrontInternal(request, priority);
+}
 
-  queued_requests_.insert(iter.base(), request);
+void PermissionRequestQueue::PushBack(permissions::PermissionRequest* request) {
+  Priority priority = DetermineRequestPriority(request);
+  PushBackInternal(request, priority);
 }
 
 PermissionRequest* PermissionRequestQueue::Pop() {
-  PermissionRequest* next = Peek();
-  if (PermissionUtil::DoesPlatformSupportChip()) {
-    queued_requests_.pop_back();
-  } else {
-    queued_requests_.pop_front();
+  std::vector<base::circular_deque<PermissionRequest*>>::reverse_iterator it;
+  // Skip entries that contain empty queues.
+  for (it = queued_requests_.rbegin();
+       it != queued_requests_.rend() && it->empty(); ++it) {
   }
-  return next;
+  PermissionRequest* front = it->front();
+  it->pop_front();
+  return front;
 }
 
 PermissionRequest* PermissionRequestQueue::Peek() const {
-  return PermissionUtil::DoesPlatformSupportChip() ? queued_requests_.back()
-                                                   : queued_requests_.front();
+  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) {
+  }
+  return it->front();
 }
 
 PermissionRequest* PermissionRequestQueue::FindDuplicate(
     PermissionRequest* request) const {
-  for (PermissionRequest* queued_request : queued_requests_) {
+  auto priority = DetermineRequestPriority(request);
+  const auto& queued_request_list =
+      queued_requests_[static_cast<size_t>(priority)];
+  for (PermissionRequest* queued_request : queued_request_list) {
     if (request->IsDuplicateOf(queued_request)) {
       return queued_request;
     }
@@ -97,4 +108,32 @@ PermissionRequestQueue::const_iterator PermissionRequestQueue::end() const {
   return queued_requests_.end();
 }
 
+// static
+PermissionRequestQueue::Priority
+PermissionRequestQueue::DetermineRequestPriority(
+    permissions::PermissionRequest* request) {
+  if (request->IsEmbeddedPermissionElementInitiated()) {
+    return Priority::kHigh;
+  }
+
+  if (permissions::PermissionUtil::DoesPlatformSupportChip() &&
+      permissions::PermissionUtil::IsLowPriorityPermissionRequest(request)) {
+    return Priority::kLow;
+  }
+
+  return Priority::kMedium;
+}
+
+void PermissionRequestQueue::PushFrontInternal(
+    permissions::PermissionRequest* request,
+    Priority priority) {
+  queued_requests_[static_cast<size_t>(priority)].push_front(request);
+}
+
+void PermissionRequestQueue::PushBackInternal(
+    permissions::PermissionRequest* request,
+    Priority priority) {
+  queued_requests_[static_cast<size_t>(priority)].push_back(request);
+}
+
 }  // namespace permissions