Commit 70546721 authored by Olesia Marukhno's avatar Olesia Marukhno Committed by Commit Bot

Fix chip animation for same request in another tab

If you request a permission, then open another tab and request the same
permission, the chip appears without animating and the origin in the
omnibox disappears. This patch fixes this behaviour.

If request was already shown to user and user switches back to a tab
with a pending request, request stays collaped and won't reanimate.

Bug: 1019129
Change-Id: I9d46f1fbe479de584bcce988d7a20deb4e7e76a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2445774Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com>
Cr-Commit-Position: refs/heads/master@{#814635}
parent a1a098cf
......@@ -33,6 +33,10 @@ GURL TestPermissionBubbleViewDelegate::GetEmbeddingOrigin() const {
return GURL("https://embedder.example.com");
}
bool TestPermissionBubbleViewDelegate::WasCurrentRequestAlreadyDisplayed() {
return false;
}
PermissionBubbleBrowserTest::PermissionBubbleBrowserTest() = default;
PermissionBubbleBrowserTest::~PermissionBubbleBrowserTest() = default;
......
......@@ -40,6 +40,8 @@ class TestPermissionBubbleViewDelegate
void Deny() override {}
void Closing() override {}
bool WasCurrentRequestAlreadyDisplayed() override;
void set_requests(std::vector<permissions::PermissionRequest*> requests) {
requests_ = requests;
}
......
......@@ -133,8 +133,12 @@ void PermissionChip::Show(permissions::PermissionPrompt::Delegate* delegate) {
UpdatePermissionIconAndTextColor();
SetVisible(true);
animation_->Show();
// TODO(olesiamarukhno): Add tests for animation logic.
animation_->Reset();
if (!delegate_->WasCurrentRequestAlreadyDisplayed())
animation_->Show();
requested_time_ = base::TimeTicks::Now();
PreferredSizeChanged();
}
void PermissionChip::Hide() {
......
......@@ -52,6 +52,8 @@ class TestDelegate : public permissions::PermissionPrompt::Delegate {
void Deny() override {}
void Closing() override {}
bool WasCurrentRequestAlreadyDisplayed() override { return false; }
private:
std::vector<std::unique_ptr<permissions::PermissionRequest>> requests_;
std::vector<permissions::PermissionRequest*> raw_requests_;
......
......@@ -58,6 +58,9 @@ class PermissionPrompt {
virtual void Accept() = 0;
virtual void Deny() = 0;
virtual void Closing() = 0;
// Whether the current request has been shown to the user at least once.
virtual bool WasCurrentRequestAlreadyDisplayed() = 0;
};
typedef base::Callback<
......
......@@ -403,6 +403,10 @@ void PermissionRequestManager::Closing() {
FinalizeBubble(PermissionAction::DISMISSED);
}
bool PermissionRequestManager::WasCurrentRequestAlreadyDisplayed() {
return current_request_already_displayed_;
}
PermissionRequestManager::PermissionRequestManager(
content::WebContents* web_contents)
: content::WebContentsObserver(web_contents),
......@@ -498,7 +502,7 @@ void PermissionRequestManager::ShowBubble() {
if (!view_)
return;
if (!current_request_view_shown_to_user_) {
if (!current_request_already_displayed_) {
PermissionUmaUtil::PermissionPromptShown(requests_);
if (ShouldCurrentRequestUseQuietUI()) {
......@@ -528,7 +532,7 @@ void PermissionRequestManager::ShowBubble() {
}
}
}
current_request_view_shown_to_user_ = true;
current_request_already_displayed_ = true;
NotifyBubbleAdded();
// If in testing mode, automatically respond to the bubble that was shown.
......@@ -601,7 +605,7 @@ void PermissionRequestManager::FinalizeBubble(
if (notification_permission_ui_selector_)
notification_permission_ui_selector_->Cancel();
current_request_view_shown_to_user_ = false;
current_request_already_displayed_ = false;
current_request_ui_to_use_.reset();
if (view_)
......
......@@ -134,6 +134,7 @@ class PermissionRequestManager
void Accept() override;
void Deny() override;
void Closing() override;
bool WasCurrentRequestAlreadyDisplayed() override;
void set_web_contents_supports_permission_requests(
bool web_contents_supports_permission_requests) {
......@@ -259,7 +260,7 @@ class PermissionRequestManager
// Whether the view for the current |requests_| has been shown to the user at
// least once.
bool current_request_view_shown_to_user_ = false;
bool current_request_already_displayed_ = false;
// Whether to use the normal or quiet UI to display the current permission
// |requests_|, and whether to show warnings. This will be nullopt if we are
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment