Add handling of pending requests when switching to fullscreen

When there is a pending request and chip UI is used, after switching to
fullscreen mode bubble UI should be used if pending request is new (chip
haven't been collapsed yet). This prevents case when permission was
requested and shorty after that user switches to fullscreen mode and
cannot see pending request anymore.

When switching from fullscreen mode, if request should be shown with
chip UI, switch back to it with bubble opened. This provides visual
consistency and in regular mode with chip UI bubble is easily dismissable.

Bug: 1148332
Change-Id: I52886d95842db45ec72997127f25010585d97670
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2540527
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@{#831042}

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

@@ -136,8 +136,11 @@ void PermissionChip::Show(permissions::PermissionPrompt::Delegate* delegate) {
   SetVisible(true);
   // TODO(olesiamarukhno): Add tests for animation logic.
   animation_->Reset();
-  if (!delegate_->WasCurrentRequestAlreadyDisplayed())
+  is_collapsed_ = true;
+  if (!delegate_->WasCurrentRequestAlreadyDisplayed()) {
     animation_->Show();
+    is_collapsed_ = false;
+  }
   requested_time_ = base::TimeTicks::Now();
   PreferredSizeChanged();
 }
@@ -188,13 +191,10 @@ void PermissionChip::OnWidgetDestroying(views::Widget* widget) {
   widget->RemoveObserver(this);
   prompt_bubble_ = nullptr;
   animation_->Hide();
+  is_collapsed_ = true;
 }
 
-bool PermissionChip::IsBubbleShowing() const {
-  return prompt_bubble_ != nullptr;
-}
-
-void PermissionChip::ChipButtonPressed() {
+void PermissionChip::OpenBubble() {
   // The prompt bubble is either not opened yet or already closed on
   // deactivation.
   DCHECK(!prompt_bubble_);
@@ -203,6 +203,14 @@ void PermissionChip::ChipButtonPressed() {
       browser_, delegate_, requested_time_, PermissionPromptStyle::kChip);
   prompt_bubble_->Show();
   prompt_bubble_->GetWidget()->AddObserver(this);
+}
+
+bool PermissionChip::IsBubbleShowing() const {
+  return prompt_bubble_ != nullptr;
+}
+
+void PermissionChip::ChipButtonPressed() {
+  OpenBubble();
   // Restart the timer after user clicks on the chip to open the bubble.
   StartCollapseTimer();
   if (!already_recorded_interaction_) {
@@ -217,6 +225,7 @@ void PermissionChip::Collapse() {
     StartCollapseTimer();
   } else {
     animation_->Hide();
+    is_collapsed_ = true;
   }
 }
 

chrome/browser/ui/views/location_bar/permission_chip.h

@@ -41,8 +41,10 @@ class PermissionChip : public views::View,
 
   void Show(permissions::PermissionPrompt::Delegate* delegate);
   void Hide();
+  void OpenBubble();
 
   views::Button* button() { return chip_button_; }
+  bool is_collapsed() { return is_collapsed_; }
 
   // views::AnimationDelegateViews:
   void AnimationEnded(const gfx::Animation* animation) override;
@@ -90,6 +92,10 @@ class PermissionChip : public views::View,
 
   // If uma metric was already recorded on the button click.
   bool already_recorded_interaction_ = false;
+
+  // If chip is collapsed. In the collapsed state, only an icon is visible,
+  // without text.
+  bool is_collapsed_ = false;
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_PERMISSION_CHIP_H_

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

@@ -96,16 +96,7 @@ PermissionPromptBubbleView::PermissionPromptBubbleView(
   SetCancelCallback(base::BindOnce(&PermissionPromptBubbleView::DenyPermission,
                                    base::Unretained(this)));
 
-  // If bubble hanging off the padlock icon, with no chip showing, it shouldn't
-  // close on deactivate and it should stick until user makes a decision.
-  // Otherwise, the chip is indicating the pending permission request and so the
-  // bubble can be opened and closed repeatedly.
-  if (prompt_style == PermissionPromptStyle::kBubbleOnly) {
-    set_close_on_deactivate(false);
-    DialogDelegate::SetCloseCallback(
-        base::BindOnce(&PermissionPromptBubbleView::ClosingPermission,
-                       base::Unretained(this)));
-  }
+  SetPromptStyle(prompt_style);
 
   SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::Orientation::kVertical, gfx::Insets(),
@@ -217,6 +208,23 @@ void PermissionPromptBubbleView::UpdateAnchorPosition() {
   SetArrow(configuration.bubble_arrow);
 }
 
+void PermissionPromptBubbleView::SetPromptStyle(
+    PermissionPromptStyle prompt_style) {
+  // If bubble hanging off the padlock icon, with no chip showing, it shouldn't
+  // close on deactivate and it should stick until user makes a decision.
+  // Otherwise, the chip is indicating the pending permission request and so the
+  // bubble can be opened and closed repeatedly.
+  if (prompt_style == PermissionPromptStyle::kBubbleOnly) {
+    set_close_on_deactivate(false);
+    DialogDelegate::SetCloseCallback(
+        base::BindOnce(&PermissionPromptBubbleView::ClosingPermission,
+                       base::Unretained(this)));
+  } else {
+    set_close_on_deactivate(true);
+    DialogDelegate::SetCloseCallback(base::OnceClosure());
+  }
+}
+
 void PermissionPromptBubbleView::AddedToWidget() {
   if (name_or_origin_.is_origin) {
     // There is a risk of URL spoofing from origins that are too wide to fit in

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

@@ -29,6 +29,8 @@ class PermissionPromptBubbleView : public views::BubbleDialogDelegateView {
   // bubble_anchor_util::GetPageInfoAnchorConfiguration.
   void UpdateAnchorPosition();
 
+  void SetPromptStyle(PermissionPromptStyle prompt_style);
+
   // views::BubbleDialogDelegateView:
   void AddedToWidget() override;
   bool ShouldShowCloseButton() const override;

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

@@ -47,16 +47,8 @@ PermissionPromptImpl::PermissionPromptImpl(Browser* browser,
     content_settings::UpdateLocationBarUiForWebContents(web_contents_);
   } else {
     LocationBarView* lbv = GetLocationBarView();
-    std::vector<permissions::PermissionRequest*> requests =
-        delegate->Requests();
-    const bool should_use_chip_ui = std::all_of(
-        requests.begin(), requests.end(),
-        [](auto* request) { return request->GetChipText().has_value(); });
-    if (base::FeatureList::IsEnabled(permissions::features::kPermissionChip) &&
-        lbv && lbv->IsDrawn() && should_use_chip_ui) {
-      permission_chip_ = lbv->permission_chip();
-      permission_chip_->Show(delegate);
-      prompt_style_ = PermissionPromptStyle::kChip;
+    if (lbv && lbv->IsDrawn() && ShouldCurrentRequestUseChipUI()) {
+      ShowChipUI();
     } else {
       ShowBubble();
     }
@@ -70,25 +62,60 @@ void PermissionPromptImpl::OnWidgetClosing(views::Widget* widget) {
 }
 
 PermissionPromptImpl::~PermissionPromptImpl() {
-  if (prompt_bubble_)
-    prompt_bubble_->GetWidget()->Close();
-
-  if (prompt_style_ == PermissionPromptStyle::kQuiet) {
-    // Hides the quiet prompt.
-    content_settings::UpdateLocationBarUiForWebContents(web_contents_);
+  switch (prompt_style_) {
+    case PermissionPromptStyle::kBubbleOnly:
+      DCHECK(!permission_chip_);
+      if (prompt_bubble_)
+        prompt_bubble_->GetWidget()->Close();
+      break;
+    case PermissionPromptStyle::kChip:
+      DCHECK(!prompt_bubble_);
+      DCHECK(permission_chip_);
+      permission_chip_->Hide();
+      permission_chip_ = nullptr;
+      break;
+    case PermissionPromptStyle::kQuiet:
+      DCHECK(!prompt_bubble_);
+      DCHECK(!permission_chip_);
+      content_settings::UpdateLocationBarUiForWebContents(web_contents_);
+      break;
   }
 
-  if (prompt_style_ == PermissionPromptStyle::kChip && permission_chip_) {
-    permission_chip_->Hide();
-  }
   CHECK(!IsInObserverList());
 }
 
 void PermissionPromptImpl::UpdateAnchorPosition() {
-  // TODO(olesiamarukhno): Figure out if we need to switch UI styles (chip to
-  // bubble and vice versa) here.
-  if (prompt_bubble_)
-    prompt_bubble_->UpdateAnchorPosition();
+  LocationBarView* lbv = GetLocationBarView();
+  const bool is_location_bar_drawn = lbv && lbv->IsDrawn();
+  switch (prompt_style_) {
+    case PermissionPromptStyle::kBubbleOnly:
+      DCHECK(prompt_bubble_);
+      DCHECK(!permission_chip_);
+      if (ShouldCurrentRequestUseChipUI() && is_location_bar_drawn) {
+        // Change prompt style to chip to avoid dismissing request while
+        // switching UI style.
+        prompt_bubble_->SetPromptStyle(PermissionPromptStyle::kChip);
+        prompt_bubble_->GetWidget()->Close();
+        ShowChipUI();
+        permission_chip_->OpenBubble();
+      } else {
+        prompt_bubble_->UpdateAnchorPosition();
+      }
+      break;
+    case PermissionPromptStyle::kChip:
+      DCHECK(!prompt_bubble_);
+      DCHECK(permission_chip_);
+      // If there is fresh pending request shown as chip UI and location bar
+      // isn't visible anymore, show bubble UI instead.
+      if (!permission_chip_->is_collapsed() && !is_location_bar_drawn) {
+        permission_chip_->Hide();
+        permission_chip_ = nullptr;
+        ShowBubble();
+      }
+      break;
+    case PermissionPromptStyle::kQuiet:
+      break;
+  }
 }
 
 LocationBarView* PermissionPromptImpl::GetLocationBarView() {
@@ -96,6 +123,15 @@ LocationBarView* PermissionPromptImpl::GetLocationBarView() {
   return browser_view ? browser_view->GetLocationBarView() : nullptr;
 }
 
+void PermissionPromptImpl::ShowChipUI() {
+  LocationBarView* lbv = GetLocationBarView();
+  DCHECK(lbv);
+
+  permission_chip_ = lbv->permission_chip();
+  permission_chip_->Show(delegate_);
+  prompt_style_ = PermissionPromptStyle::kChip;
+}
+
 void PermissionPromptImpl::ShowBubble() {
   prompt_style_ = PermissionPromptStyle::kBubbleOnly;
   prompt_bubble_ = new PermissionPromptBubbleView(
@@ -104,6 +140,16 @@ void PermissionPromptImpl::ShowBubble() {
   prompt_bubble_->GetWidget()->AddObserver(this);
 }
 
+bool PermissionPromptImpl::ShouldCurrentRequestUseChipUI() {
+  if (!base::FeatureList::IsEnabled(permissions::features::kPermissionChip))
+    return false;
+
+  std::vector<permissions::PermissionRequest*> requests = delegate_->Requests();
+  return std::all_of(requests.begin(), requests.end(), [](auto* request) {
+    return request->GetChipText().has_value();
+  });
+}
+
 permissions::PermissionPrompt::TabSwitchingBehavior
 PermissionPromptImpl::GetTabSwitchingBehavior() {
   return permissions::PermissionPrompt::TabSwitchingBehavior::

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

@@ -50,6 +50,10 @@ class PermissionPromptImpl : public permissions::PermissionPrompt,
 
   void ShowBubble();
 
+  void ShowChipUI();
+
+  bool ShouldCurrentRequestUseChipUI();
+
   // The popup bubble. Not owned by this class; it will delete itself when a
   // decision is made.
   PermissionPromptBubbleView* prompt_bubble_;