Change permissions processing order

Make new permission requests to override current pending requests
and then restore pending requests in stack order.

Bug: 1019129

Change-Id: Ia7503432cceb27e4db40708c63e3e442e026595e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418445
Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826839}

chrome/browser/about_flags.cc

@@ -107,6 +107,7 @@
 #include "components/paint_preview/features/features.h"
 #include "components/password_manager/core/common/password_manager_features.h"
 #include "components/payments/core/features.h"
+#include "components/permissions/features.h"
 #include "components/policy/core/common/features.h"
 #include "components/previews/core/previews_features.h"
 #include "components/previews/core/previews_switches.h"
@@ -6258,7 +6259,7 @@ const FeatureEntry kFeatureEntries[] = {
 
     {"permission-chip", flag_descriptions::kPermissionChipName,
      flag_descriptions::kPermissionChipDescription, kOsDesktop,
-     FEATURE_VALUE_TYPE(features::kPermissionChip)},
+     FEATURE_VALUE_TYPE(permissions::features::kPermissionChip)},
 
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
     {"dice-web-signin-interception",

chrome/browser/ui/ui_features.cc

@@ -38,11 +38,6 @@ const base::Feature kNewProfilePicker{"NewProfilePicker",
 const base::Feature kNewTabstripAnimation{"NewTabstripAnimation",
                                           base::FEATURE_ENABLED_BY_DEFAULT};
 
-// Enables an experimental permission prompt that uses a chip in the location
-// bar.
-const base::Feature kPermissionChip{"PermissionChip",
-                                    base::FEATURE_DISABLED_BY_DEFAULT};
-
 // Enables a more prominent active tab title in dark mode to aid with
 // accessibility.
 const base::Feature kProminentDarkModeActiveTabTitle{

chrome/browser/ui/ui_features.h

@@ -33,8 +33,6 @@ extern const base::Feature kNewProfilePicker;
 
 extern const base::Feature kNewTabstripAnimation;
 
-extern const base::Feature kPermissionChip;
-
 extern const base::Feature kProminentDarkModeActiveTabTitle;
 
 extern const base::Feature kScrollableTabStrip;

chrome/browser/ui/views/bubble_anchor_util_views.cc

@@ -5,12 +5,12 @@
 #include "chrome/browser/ui/views/bubble_anchor_util_views.h"
 
 #include "build/build_config.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/frame/app_menu_button.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/location_bar/location_icon_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "components/permissions/features.h"
 
 // This file contains the bubble_anchor_util implementation for a Views
 // browser window (BrowserView).
@@ -49,7 +49,7 @@ AnchorConfiguration GetPageInfoAnchorConfiguration(Browser* browser,
 
 AnchorConfiguration GetPermissionPromptBubbleAnchorConfiguration(
     Browser* browser) {
-  if (base::FeatureList::IsEnabled(features::kPermissionChip)) {
+  if (base::FeatureList::IsEnabled(permissions::features::kPermissionChip)) {
     BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
     return {browser_view->GetLocationBarView(),
             browser_view->GetLocationBarView()->permission_chip()->button(),

chrome/browser/ui/views/location_bar/permission_chip_browsertest.cc

@@ -7,20 +7,20 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/test/test_browser_dialog.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/location_bar/permission_chip.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "chrome/test/permissions/permission_request_manager_test_api.h"
+#include "components/permissions/features.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 
 class PermissionChipBrowserTest : public UiBrowserTest {
  public:
   PermissionChipBrowserTest() {
-    feature_list_.InitAndEnableFeature(features::kPermissionChip);
+    feature_list_.InitAndEnableFeature(permissions::features::kPermissionChip);
   }
 
   PermissionChipBrowserTest(const PermissionChipBrowserTest&) = delete;

chrome/browser/ui/views/location_bar/permission_chip_prompt_browsertest.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/ui/views/location_bar/permission_chip.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/test/permissions/permission_request_manager_test_api.h"
+#include "components/permissions/features.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "ui/events/base_event_utils.h"
@@ -21,7 +22,7 @@
 class PermissionChipPromptBrowserTest : public DialogBrowserTest {
  public:
   PermissionChipPromptBrowserTest() {
-    feature_list_.InitAndEnableFeature(features::kPermissionChip);
+    feature_list_.InitAndEnableFeature(permissions::features::kPermissionChip);
   }
 
   PermissionChipPromptBrowserTest(const PermissionChipPromptBrowserTest&) =

chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.cc

@@ -15,12 +15,12 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/bubble_anchor_util_views.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/title_origin_label.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/permissions/features.h"
 #include "components/permissions/permission_request.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/elide_url.h"
@@ -70,7 +70,7 @@ PermissionPromptBubbleView::PermissionPromptBubbleView(
   // If the permission chip feature is enabled, the chip is indicating the
   // pending permission request and so the bubble can be opened and closed
   // repeatedly.
-  if (!base::FeatureList::IsEnabled(features::kPermissionChip)) {
+  if (!base::FeatureList::IsEnabled(permissions::features::kPermissionChip)) {
     set_close_on_deactivate(false);
     DialogDelegate::SetCloseCallback(
         base::BindOnce(&PermissionPromptBubbleView::ClosingPermission,

chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc

@@ -10,9 +10,9 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/permission_bubble/permission_prompt.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.h"
+#include "components/permissions/features.h"
 #include "components/permissions/notification_permission_ui_selector.h"
 #include "components/permissions/permission_request.h"
 #include "components/permissions/permission_request_manager.h"
@@ -56,8 +56,8 @@ PermissionPromptImpl::PermissionPromptImpl(Browser* browser,
     LocationBarView* lbv = GetLocationBarView();
     std::vector<permissions::PermissionRequest*> requests =
         delegate->Requests();
-    if (base::FeatureList::IsEnabled(features::kPermissionChip) && lbv &&
-        std::all_of(requests.begin(), requests.end(), [](auto* request) {
+    if (base::FeatureList::IsEnabled(permissions::features::kPermissionChip) &&
+        lbv && std::all_of(requests.begin(), requests.end(), [](auto* request) {
           return request->GetChipText().has_value();
         })) {
       permission_chip_ = lbv->permission_chip();

components/permissions/features.cc

@@ -24,5 +24,10 @@ const base::Feature kBlockRepeatedNotificationPermissionPrompts{
     "BlockRepeatedNotificationPermissionPrompts",
     base::FEATURE_ENABLED_BY_DEFAULT};
 
+// Enables an experimental permission prompt that uses a chip in the location
+// bar.
+const base::Feature kPermissionChip{"PermissionChip",
+                                    base::FEATURE_DISABLED_BY_DEFAULT};
+
 }  // namespace features
 }  // namespace permissions

components/permissions/features.h

@@ -13,6 +13,7 @@ namespace features {
 extern const base::Feature kBlockPromptsIfDismissedOften;
 extern const base::Feature kBlockPromptsIfIgnoredOften;
 extern const base::Feature kBlockRepeatedNotificationPermissionPrompts;
+extern const base::Feature kPermissionChip;
 
 }  // namespace features
 }  // namespace permissions

components/permissions/permission_request_manager.h

@@ -197,9 +197,12 @@ class PermissionRequestManager
   // Delete the view object
   void DeleteBubble();
 
+  // Finalize request.
+  void ResetViewStateForCurrentRequest();
+
   // Delete the view object, finalize requests, asynchronously show a queued
   // request if present.
-  void FinalizeBubble(PermissionAction permission_action);
+  void FinalizeCurrentRequests(PermissionAction permission_action);
 
   // Cancel all pending or active requests and destroy the PermissionPrompt if
   // one exists. This is called if the WebContents is destroyed or navigates its
@@ -233,8 +236,6 @@ class PermissionRequestManager
 
   void DoAutoResponseForTesting();
 
-  int CountQueuedPermissionRequests(PermissionRequest* request);
-
   // Factory to be used to create views when needed.
   PermissionPrompt::Factory view_factory_;
 
@@ -251,20 +252,29 @@ class PermissionRequestManager
   // tab is visible.
   std::vector<PermissionRequest*> requests_;
 
-  struct RequestAndSource {
+  struct PermissionRequestSource {
     int render_process_id;
     int render_frame_id;
-    PermissionRequest* request;
 
     bool IsSourceFrameInactiveAndDisallowReactivation() const;
   };
 
-  base::circular_deque<RequestAndSource> queued_requests_;
+  base::circular_deque<PermissionRequest*> queued_requests_;
+
+  PermissionRequest* PeekNextQueuedRequest();
+
+  PermissionRequest* PopNextQueuedRequest();
+
   // Maps from the first request of a kind to subsequent requests that were
   // duped against it.
   std::unordered_multimap<PermissionRequest*, PermissionRequest*>
       duplicate_requests_;
 
+  // Maps each PermissionRequest currently in |requests_| or |queued_requests_|
+  // to which RenderFrameHost it originated from. Note that no date is stored
+  // for |duplicate_requests_|.
+  std::map<PermissionRequest*, PermissionRequestSource> request_sources_map_;
+
   base::ObserverList<Observer>::Unchecked observer_list_;
   AutoResponseType auto_response_for_test_;