Commit f4a2ba13 authored by Sunny Sachanandani's avatar Sunny Sachanandani Committed by Commit Bot

infobar: Do not animate bad flag and automation infobars

Animating infobars during startup is speculated to cause regressions and
noise in rendering benchmarks.  This CL disables animations for bad flag
and automation infobars by allowing the InfoBarDelegate to control if
the infobar animates or not.

Bug: 804324
Change-Id: I2c1067e122cf031ab151a1f51d9d11bc6bac60e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1808297Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697791}
parent 965898a0
...@@ -189,8 +189,7 @@ void PluginObserver::CreatePluginObserverInfoBar( ...@@ -189,8 +189,7 @@ void PluginObserver::CreatePluginObserverInfoBar(
infobars::InfoBarDelegate::PLUGIN_OBSERVER_INFOBAR_DELEGATE, infobars::InfoBarDelegate::PLUGIN_OBSERVER_INFOBAR_DELEGATE,
&kExtensionCrashedIcon, &kExtensionCrashedIcon,
l10n_util::GetStringFUTF16(IDS_PLUGIN_INITIALIZATION_ERROR_PROMPT, l10n_util::GetStringFUTF16(IDS_PLUGIN_INITIALIZATION_ERROR_PROMPT,
plugin_name), plugin_name));
true);
} }
void PluginObserver::BlockedOutdatedPlugin( void PluginObserver::BlockedOutdatedPlugin(
......
...@@ -32,8 +32,7 @@ void ChromeSelectFilePolicy::SelectFileDenied() { ...@@ -32,8 +32,7 @@ void ChromeSelectFilePolicy::SelectFileDenied() {
SimpleAlertInfoBarDelegate::Create( SimpleAlertInfoBarDelegate::Create(
InfoBarService::FromWebContents(source_contents_), InfoBarService::FromWebContents(source_contents_),
infobars::InfoBarDelegate::FILE_ACCESS_DISABLED_INFOBAR_DELEGATE, infobars::InfoBarDelegate::FILE_ACCESS_DISABLED_INFOBAR_DELEGATE,
nullptr, l10n_util::GetStringUTF16(IDS_FILE_SELECTION_DIALOG_INFOBAR), nullptr, l10n_util::GetStringUTF16(IDS_FILE_SELECTION_DIALOG_INFOBAR));
true);
} else { } else {
LOG(WARNING) << "File-selection dialogs are disabled but no WebContents " LOG(WARNING) << "File-selection dialogs are disabled but no WebContents "
<< "is given to display the InfoBar."; << "is given to display the InfoBar.";
......
...@@ -31,6 +31,14 @@ bool AutomationInfoBarDelegate::ShouldExpire( ...@@ -31,6 +31,14 @@ bool AutomationInfoBarDelegate::ShouldExpire(
return false; return false;
} }
bool AutomationInfoBarDelegate::ShouldAnimate() const {
// Animating the infobar also animates the content area size which can trigger
// a flood of page layout, compositing, texture reallocations, etc. Since
// this infobar is primarily used for automated testing do not animate the
// infobar to reduce noise in tests.
return false;
}
base::string16 AutomationInfoBarDelegate::GetMessageText() const { base::string16 AutomationInfoBarDelegate::GetMessageText() const {
return l10n_util::GetStringUTF16(IDS_CONTROLLED_BY_AUTOMATION); return l10n_util::GetStringUTF16(IDS_CONTROLLED_BY_AUTOMATION);
} }
......
...@@ -23,6 +23,7 @@ class AutomationInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -23,6 +23,7 @@ class AutomationInfoBarDelegate : public ConfirmInfoBarDelegate {
infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override; infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
bool ShouldExpire(const NavigationDetails& details) const override; bool ShouldExpire(const NavigationDetails& details) const override;
bool ShouldAnimate() const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
int GetButtons() const override; int GetButtons() const override;
......
...@@ -150,14 +150,19 @@ static const base::Feature* kBadFeatureFlagsInAboutFlags[] = { ...@@ -150,14 +150,19 @@ static const base::Feature* kBadFeatureFlagsInAboutFlags[] = {
#endif // OS_ANDROID #endif // OS_ANDROID
}; };
void ShowBadFeatureFlagsInfoBar(content::WebContents* web_contents, void ShowBadFlagsInfoBarHelper(content::WebContents* web_contents,
int message_id, int message_id,
const base::Feature* feature) { base::StringPiece flag) {
// Animating the infobar also animates the content area size which can trigger
// a flood of page layout, compositing, texture reallocations, etc. Do not
// animate the infobar to reduce noise in perf benchmarks because they pass
// --ignore-certificate-errors-spki-list. This infobar only appears at
// startup so the animation isn't visible to users anyway.
SimpleAlertInfoBarDelegate::Create( SimpleAlertInfoBarDelegate::Create(
InfoBarService::FromWebContents(web_contents), InfoBarService::FromWebContents(web_contents),
infobars::InfoBarDelegate::BAD_FLAGS_INFOBAR_DELEGATE, nullptr, infobars::InfoBarDelegate::BAD_FLAGS_INFOBAR_DELEGATE, nullptr,
l10n_util::GetStringFUTF16(message_id, base::UTF8ToUTF16(feature->name)), l10n_util::GetStringFUTF16(message_id, base::UTF8ToUTF16(flag)),
false); /*auto_expire=*/false, /*should_animate=*/false);
} }
} // namespace } // namespace
...@@ -176,8 +181,8 @@ void ShowBadFlagsPrompt(content::WebContents* web_contents) { ...@@ -176,8 +181,8 @@ void ShowBadFlagsPrompt(content::WebContents* web_contents) {
for (const base::Feature* feature : kBadFeatureFlagsInAboutFlags) { for (const base::Feature* feature : kBadFeatureFlagsInAboutFlags) {
if (base::FeatureList::IsEnabled(*feature)) { if (base::FeatureList::IsEnabled(*feature)) {
ShowBadFeatureFlagsInfoBar(web_contents, IDS_BAD_FEATURES_WARNING_MESSAGE, ShowBadFlagsInfoBarHelper(web_contents, IDS_BAD_FEATURES_WARNING_MESSAGE,
feature); feature->name);
return; return;
} }
} }
...@@ -190,13 +195,8 @@ void ShowBadFlagsInfoBar(content::WebContents* web_contents, ...@@ -190,13 +195,8 @@ void ShowBadFlagsInfoBar(content::WebContents* web_contents,
base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(flag); base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(flag);
if (!switch_value.empty()) if (!switch_value.empty())
switch_value = "=" + switch_value; switch_value = "=" + switch_value;
SimpleAlertInfoBarDelegate::Create( ShowBadFlagsInfoBarHelper(web_contents, message_id,
InfoBarService::FromWebContents(web_contents), std::string("--") + flag + switch_value);
infobars::InfoBarDelegate::BAD_FLAGS_INFOBAR_DELEGATE, nullptr,
l10n_util::GetStringFUTF16(
message_id,
base::UTF8ToUTF16(std::string("--") + flag + switch_value)),
false);
} }
void MaybeShowInvalidUserDataDirWarningDialog() { void MaybeShowInvalidUserDataDirWarningDialog() {
......
...@@ -24,8 +24,10 @@ InfoBar::InfoBar(std::unique_ptr<InfoBarDelegate> delegate) ...@@ -24,8 +24,10 @@ InfoBar::InfoBar(std::unique_ptr<InfoBarDelegate> delegate)
target_height_(0) { target_height_(0) {
DCHECK(delegate_ != nullptr); DCHECK(delegate_ != nullptr);
animation_.SetTweenType(gfx::Tween::LINEAR); animation_.SetTweenType(gfx::Tween::LINEAR);
if (!gfx::Animation::ShouldRenderRichAnimation()) if (!gfx::Animation::ShouldRenderRichAnimation() ||
!delegate_->ShouldAnimate()) {
animation_.SetSlideDuration(base::TimeDelta()); animation_.SetSlideDuration(base::TimeDelta());
}
delegate_->set_infobar(this); delegate_->set_infobar(this);
} }
......
...@@ -76,6 +76,10 @@ bool InfoBarDelegate::IsCloseable() const { ...@@ -76,6 +76,10 @@ bool InfoBarDelegate::IsCloseable() const {
return true; return true;
} }
bool InfoBarDelegate::ShouldAnimate() const {
return true;
}
ConfirmInfoBarDelegate* InfoBarDelegate::AsConfirmInfoBarDelegate() { ConfirmInfoBarDelegate* InfoBarDelegate::AsConfirmInfoBarDelegate() {
return nullptr; return nullptr;
} }
......
...@@ -225,6 +225,10 @@ class InfoBarDelegate { ...@@ -225,6 +225,10 @@ class InfoBarDelegate {
// Returns true if the InfoBar has a close button; true by default. // Returns true if the InfoBar has a close button; true by default.
virtual bool IsCloseable() const; virtual bool IsCloseable() const;
// Returns true if the InfoBar should animate when showing or hiding; true by
// default.
virtual bool ShouldAnimate() const;
// Type-checking downcast routines: // Type-checking downcast routines:
virtual ConfirmInfoBarDelegate* AsConfirmInfoBarDelegate(); virtual ConfirmInfoBarDelegate* AsConfirmInfoBarDelegate();
virtual HungRendererInfoBarDelegate* AsHungRendererInfoBarDelegate(); virtual HungRendererInfoBarDelegate* AsHungRendererInfoBarDelegate();
......
...@@ -16,24 +16,27 @@ void SimpleAlertInfoBarDelegate::Create( ...@@ -16,24 +16,27 @@ void SimpleAlertInfoBarDelegate::Create(
infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier, infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier,
const gfx::VectorIcon* vector_icon, const gfx::VectorIcon* vector_icon,
const base::string16& message, const base::string16& message,
bool auto_expire) { bool auto_expire,
bool should_animate) {
infobar_manager->AddInfoBar(infobar_manager->CreateConfirmInfoBar( infobar_manager->AddInfoBar(infobar_manager->CreateConfirmInfoBar(
std::unique_ptr<ConfirmInfoBarDelegate>(new SimpleAlertInfoBarDelegate( std::unique_ptr<ConfirmInfoBarDelegate>(new SimpleAlertInfoBarDelegate(
infobar_identifier, vector_icon, message, auto_expire)))); infobar_identifier, vector_icon, message, auto_expire,
should_animate))));
} }
SimpleAlertInfoBarDelegate::SimpleAlertInfoBarDelegate( SimpleAlertInfoBarDelegate::SimpleAlertInfoBarDelegate(
infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier, infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier,
const gfx::VectorIcon* vector_icon, const gfx::VectorIcon* vector_icon,
const base::string16& message, const base::string16& message,
bool auto_expire) bool auto_expire,
bool should_animate)
: infobar_identifier_(infobar_identifier), : infobar_identifier_(infobar_identifier),
vector_icon_(vector_icon), vector_icon_(vector_icon),
message_(message), message_(message),
auto_expire_(auto_expire) {} auto_expire_(auto_expire),
should_animate_(should_animate) {}
SimpleAlertInfoBarDelegate::~SimpleAlertInfoBarDelegate() { SimpleAlertInfoBarDelegate::~SimpleAlertInfoBarDelegate() = default;
}
infobars::InfoBarDelegate::InfoBarIdentifier infobars::InfoBarDelegate::InfoBarIdentifier
SimpleAlertInfoBarDelegate::GetIdentifier() const { SimpleAlertInfoBarDelegate::GetIdentifier() const {
...@@ -49,6 +52,10 @@ bool SimpleAlertInfoBarDelegate::ShouldExpire( ...@@ -49,6 +52,10 @@ bool SimpleAlertInfoBarDelegate::ShouldExpire(
return auto_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details); return auto_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details);
} }
bool SimpleAlertInfoBarDelegate::ShouldAnimate() const {
return should_animate_ && ConfirmInfoBarDelegate::ShouldAnimate();
}
base::string16 SimpleAlertInfoBarDelegate::GetMessageText() const { base::string16 SimpleAlertInfoBarDelegate::GetMessageText() const {
return message_; return message_;
} }
......
...@@ -28,20 +28,23 @@ class SimpleAlertInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -28,20 +28,23 @@ class SimpleAlertInfoBarDelegate : public ConfirmInfoBarDelegate {
infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier, infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier,
const gfx::VectorIcon* vector_icon, const gfx::VectorIcon* vector_icon,
const base::string16& message, const base::string16& message,
bool auto_expire); bool auto_expire = true,
bool should_animate = true);
private: private:
SimpleAlertInfoBarDelegate( SimpleAlertInfoBarDelegate(
infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier, infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier,
const gfx::VectorIcon* vector_icon, const gfx::VectorIcon* vector_icon,
const base::string16& message, const base::string16& message,
bool auto_expire); bool auto_expire,
bool should_animate);
~SimpleAlertInfoBarDelegate() override; ~SimpleAlertInfoBarDelegate() override;
// ConfirmInfoBarDelegate: // ConfirmInfoBarDelegate:
infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override; infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
const gfx::VectorIcon& GetVectorIcon() const override; const gfx::VectorIcon& GetVectorIcon() const override;
bool ShouldExpire(const NavigationDetails& details) const override; bool ShouldExpire(const NavigationDetails& details) const override;
bool ShouldAnimate() const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
int GetButtons() const override; int GetButtons() const override;
...@@ -49,6 +52,7 @@ class SimpleAlertInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -49,6 +52,7 @@ class SimpleAlertInfoBarDelegate : public ConfirmInfoBarDelegate {
const gfx::VectorIcon* vector_icon_; const gfx::VectorIcon* vector_icon_;
base::string16 message_; base::string16 message_;
bool auto_expire_; // Should it expire automatically on navigation? bool auto_expire_; // Should it expire automatically on navigation?
bool should_animate_;
DISALLOW_COPY_AND_ASSIGN(SimpleAlertInfoBarDelegate); DISALLOW_COPY_AND_ASSIGN(SimpleAlertInfoBarDelegate);
}; };
......
...@@ -119,8 +119,7 @@ PresentAddPassesDialogResult GetUmaResult( ...@@ -119,8 +119,7 @@ PresentAddPassesDialogResult GetUmaResult(
InfoBarManagerImpl::FromWebState(_webState), InfoBarManagerImpl::FromWebState(_webState),
infobars::InfoBarDelegate::SHOW_PASSKIT_ERROR_INFOBAR_DELEGATE_IOS, infobars::InfoBarDelegate::SHOW_PASSKIT_ERROR_INFOBAR_DELEGATE_IOS,
/*vector_icon=*/nullptr, /*vector_icon=*/nullptr,
l10n_util::GetStringUTF16(IDS_IOS_GENERIC_PASSKIT_ERROR), l10n_util::GetStringUTF16(IDS_IOS_GENERIC_PASSKIT_ERROR));
/*auto_expire=*/true);
// Infobar does not provide callback on dismissal. // Infobar does not provide callback on dismissal.
[self stop]; [self stop];
......
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