Commit 34cc3c8d authored by pkasting's avatar pkasting Committed by Commit bot

Simplify infobar expiry.

Some of the changes below may have a functional effect, but I don't _think_ they will, if they do it should be minor, and I don't know another way of finding out than trying to make this change and seeing what happens.  (Besides simplification, this change is also intended to unblock https://codereview.chromium.org/1139823006/ , which doesn't want to affect infobar behavior but without this change might do so.)

* Moved the check for "!details.did_replace_entry" from ConfirmInfoBarDelegate up to the base class.  Very few people subclass the base class anyway, and for those who do, there's no obvious reason not to do this.  I actually suggested doing this in a review several years ago but it never happened.

* Removed InfoBarDelegate::ShouldExpireInternal(), which didn't seem to be doing much useful.  I think the unique ID check was basically doing the same thing as the "is_navigation_to_different_page" bit that was already checked in ShouldExpire() (which didn't exist when the unique IDs were added long ago).  So if we got to ShouldExpireInternal() at all, that would be true regardless, and thus that helper would always return true.  (Incidentally, reloads _are_ treated as "navigations to a different page", so we still dismiss infobars by default on reload.  If this wasn't true we'd have already been broken anyway, since ShouldExpire() would have early-returned false.)

* Removed ProtectedMediaIdentifierInfoBarDelegate::ShouldExpireInternal().  I discussed this with kkimlabs@, the original author, and neither of us can understand why it was necessary to begin with.  We both think this should be removed.

* Removed the wonky TranslateInfoBarDelegate::ShouldExpire().  Long ago, this used to check a bit called "is_auto", because it was trying to allow the infobar to be dismissed even for automatic navigations, whereas other infobars wanted to only be dismissed for user-driven ones.  However, the "is_auto" bit was later removed, and the replacement code didn't really work the same way, and besides this, the conditional didn't really make sense, since "!details.is_main_frame" implies "!details.is_navigation_to_different_page".  So we basically had a function that would call InfoBarDelegate::ShouldExpireInternal() for all main-frame navigations.  But since, as I said above, I think the unique ID check there was functionally the same as checking for a "navigation to a different page"... I think all this ended up just doing the same thing as the original base function.  And I couldn't check because the translate infobar no longer exists for views anyway.  So whatever.

* Removed copy-and-pasted code that wasn't needed.

BUG=none
TEST=none

Review URL: https://codereview.chromium.org/1142153002

Cr-Commit-Position: refs/heads/master@{#330817}
parent 9316d3b9
...@@ -170,8 +170,8 @@ class ExtensionDevToolsInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -170,8 +170,8 @@ class ExtensionDevToolsInfoBarDelegate : public ConfirmInfoBarDelegate {
// ConfirmInfoBarDelegate: // ConfirmInfoBarDelegate:
Type GetInfoBarType() const override; Type GetInfoBarType() const override;
bool ShouldExpire(const NavigationDetails& details) const override;
void InfoBarDismissed() override; void InfoBarDismissed() override;
bool ShouldExpireInternal(const NavigationDetails& details) const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
int GetButtons() const override; int GetButtons() const override;
bool Cancel() override; bool Cancel() override;
...@@ -214,16 +214,16 @@ ExtensionDevToolsInfoBarDelegate::GetInfoBarType() const { ...@@ -214,16 +214,16 @@ ExtensionDevToolsInfoBarDelegate::GetInfoBarType() const {
return WARNING_TYPE; return WARNING_TYPE;
} }
bool ExtensionDevToolsInfoBarDelegate::ShouldExpire(
const NavigationDetails& details) const {
return false;
}
void ExtensionDevToolsInfoBarDelegate::InfoBarDismissed() { void ExtensionDevToolsInfoBarDelegate::InfoBarDismissed() {
if (client_host_) if (client_host_)
client_host_->MarkAsDismissed(); client_host_->MarkAsDismissed();
} }
bool ExtensionDevToolsInfoBarDelegate::ShouldExpireInternal(
const NavigationDetails& details) const {
return false;
}
base::string16 ExtensionDevToolsInfoBarDelegate::GetMessageText() const { base::string16 ExtensionDevToolsInfoBarDelegate::GetMessageText() const {
return l10n_util::GetStringFUTF16(IDS_DEV_TOOLS_INFOBAR_LABEL, return l10n_util::GetStringFUTF16(IDS_DEV_TOOLS_INFOBAR_LABEL,
base::UTF8ToUTF16(client_name_)); base::UTF8ToUTF16(client_name_));
......
...@@ -4,18 +4,10 @@ ...@@ -4,18 +4,10 @@
#include "chrome/browser/geolocation/geolocation_infobar_delegate.h" #include "chrome/browser/geolocation/geolocation_infobar_delegate.h"
#include "base/metrics/histogram.h"
#include "chrome/browser/content_settings/permission_queue_controller.h"
#include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/infobars/infobar_service.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "chrome/grit/locale_settings.h"
#include "components/google/core/browser/google_util.h"
#include "components/infobars/core/infobar.h" #include "components/infobars/core/infobar.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "grit/generated_resources.h" #include "grit/generated_resources.h"
#include "grit/locale_settings.h"
#include "grit/theme_resources.h" #include "grit/theme_resources.h"
#include "net/base/net_util.h" #include "net/base/net_util.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -28,22 +20,15 @@ infobars::InfoBar* GeolocationInfoBarDelegate::Create( ...@@ -28,22 +20,15 @@ infobars::InfoBar* GeolocationInfoBarDelegate::Create(
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
const std::string& display_languages) { const std::string& display_languages) {
const content::NavigationEntry* committed_entry =
infobar_service->web_contents()->GetController().GetLastCommittedEntry();
GeolocationInfoBarDelegate* const delegate = new GeolocationInfoBarDelegate(
controller, id, requesting_frame,
committed_entry ? committed_entry->GetUniqueID() : 0,
display_languages);
return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar( return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar(
scoped_ptr<ConfirmInfoBarDelegate>(delegate))); scoped_ptr<ConfirmInfoBarDelegate>(new GeolocationInfoBarDelegate(
controller, id, requesting_frame, display_languages))));
} }
GeolocationInfoBarDelegate::GeolocationInfoBarDelegate( GeolocationInfoBarDelegate::GeolocationInfoBarDelegate(
PermissionQueueController* controller, PermissionQueueController* controller,
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
int contents_unique_id,
const std::string& display_languages) const std::string& display_languages)
: PermissionInfobarDelegate(controller, id, requesting_frame, : PermissionInfobarDelegate(controller, id, requesting_frame,
CONTENT_SETTINGS_TYPE_GEOLOCATION), CONTENT_SETTINGS_TYPE_GEOLOCATION),
......
...@@ -27,7 +27,6 @@ class GeolocationInfoBarDelegate : public PermissionInfobarDelegate { ...@@ -27,7 +27,6 @@ class GeolocationInfoBarDelegate : public PermissionInfobarDelegate {
GeolocationInfoBarDelegate(PermissionQueueController* controller, GeolocationInfoBarDelegate(PermissionQueueController* controller,
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
int contents_unique_id,
const std::string& display_languages); const std::string& display_languages);
~GeolocationInfoBarDelegate() override; ~GeolocationInfoBarDelegate() override;
......
...@@ -4,14 +4,9 @@ ...@@ -4,14 +4,9 @@
#include "chrome/browser/media/midi_permission_infobar_delegate.h" #include "chrome/browser/media/midi_permission_infobar_delegate.h"
#include "chrome/browser/content_settings/permission_queue_controller.h"
#include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/infobars/infobar_service.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "chrome/grit/locale_settings.h"
#include "components/content_settings/core/common/permission_request_id.h"
#include "components/infobars/core/infobar.h" #include "components/infobars/core/infobar.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "grit/theme_resources.h" #include "grit/theme_resources.h"
#include "net/base/net_util.h" #include "net/base/net_util.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -24,20 +19,15 @@ infobars::InfoBar* MidiPermissionInfoBarDelegate::Create( ...@@ -24,20 +19,15 @@ infobars::InfoBar* MidiPermissionInfoBarDelegate::Create(
const GURL& requesting_frame, const GURL& requesting_frame,
const std::string& display_languages, const std::string& display_languages,
ContentSettingsType type) { ContentSettingsType type) {
const content::NavigationEntry* committed_entry =
infobar_service->web_contents()->GetController().GetLastCommittedEntry();
return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar( return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar(
scoped_ptr<ConfirmInfoBarDelegate>(new MidiPermissionInfoBarDelegate( scoped_ptr<ConfirmInfoBarDelegate>(new MidiPermissionInfoBarDelegate(
controller, id, requesting_frame, controller, id, requesting_frame, display_languages, type))));
committed_entry ? committed_entry->GetUniqueID() : 0,
display_languages, type))));
} }
MidiPermissionInfoBarDelegate::MidiPermissionInfoBarDelegate( MidiPermissionInfoBarDelegate::MidiPermissionInfoBarDelegate(
PermissionQueueController* controller, PermissionQueueController* controller,
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
int contents_unique_id,
const std::string& display_languages, const std::string& display_languages,
ContentSettingsType type) ContentSettingsType type)
: PermissionInfobarDelegate(controller, id, requesting_frame, type), : PermissionInfobarDelegate(controller, id, requesting_frame, type),
......
...@@ -31,7 +31,6 @@ class MidiPermissionInfoBarDelegate : public PermissionInfobarDelegate { ...@@ -31,7 +31,6 @@ class MidiPermissionInfoBarDelegate : public PermissionInfobarDelegate {
MidiPermissionInfoBarDelegate(PermissionQueueController* controller, MidiPermissionInfoBarDelegate(PermissionQueueController* controller,
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
int contents_unique_id,
const std::string& display_languages, const std::string& display_languages,
ContentSettingsType type); ContentSettingsType type);
~MidiPermissionInfoBarDelegate() override; ~MidiPermissionInfoBarDelegate() override;
......
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "components/infobars/core/infobar.h" #include "components/infobars/core/infobar.h"
#include "content/public/browser/navigation_entry.h"
#include "grit/components_strings.h" #include "grit/components_strings.h"
#include "grit/theme_resources.h" #include "grit/theme_resources.h"
#include "net/base/net_util.h" #include "net/base/net_util.h"
...@@ -22,14 +21,10 @@ infobars::InfoBar* ProtectedMediaIdentifierInfoBarDelegate::Create( ...@@ -22,14 +21,10 @@ infobars::InfoBar* ProtectedMediaIdentifierInfoBarDelegate::Create(
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
const std::string& display_languages) { const std::string& display_languages) {
const content::NavigationEntry* committed_entry =
infobar_service->web_contents()->GetController().GetLastCommittedEntry();
return infobar_service->AddInfoBar( return infobar_service->AddInfoBar(
infobar_service->CreateConfirmInfoBar(scoped_ptr<ConfirmInfoBarDelegate>( infobar_service->CreateConfirmInfoBar(scoped_ptr<ConfirmInfoBarDelegate>(
new ProtectedMediaIdentifierInfoBarDelegate( new ProtectedMediaIdentifierInfoBarDelegate(
controller, id, requesting_frame, controller, id, requesting_frame, display_languages))));
committed_entry ? committed_entry->GetUniqueID() : 0,
display_languages))));
} }
...@@ -38,13 +33,11 @@ ProtectedMediaIdentifierInfoBarDelegate:: ...@@ -38,13 +33,11 @@ ProtectedMediaIdentifierInfoBarDelegate::
PermissionQueueController* controller, PermissionQueueController* controller,
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
int contents_unique_id,
const std::string& display_languages) const std::string& display_languages)
: ConfirmInfoBarDelegate(), : ConfirmInfoBarDelegate(),
controller_(controller), controller_(controller),
id_(id), id_(id),
requesting_frame_(requesting_frame), requesting_frame_(requesting_frame),
contents_unique_id_(contents_unique_id),
display_languages_(display_languages) { display_languages_(display_languages) {
} }
...@@ -80,14 +73,6 @@ void ProtectedMediaIdentifierInfoBarDelegate::InfoBarDismissed() { ...@@ -80,14 +73,6 @@ void ProtectedMediaIdentifierInfoBarDelegate::InfoBarDismissed() {
SetPermission(false, false); SetPermission(false, false);
} }
bool ProtectedMediaIdentifierInfoBarDelegate::ShouldExpireInternal(
const NavigationDetails& details) const {
// This implementation matches InfoBarDelegate::ShouldExpireInternal(), but
// uses the unique ID we set in the constructor instead of that stored in the
// base class.
return (contents_unique_id_ != details.entry_id) || details.is_reload;
}
base::string16 ProtectedMediaIdentifierInfoBarDelegate::GetMessageText() const { base::string16 ProtectedMediaIdentifierInfoBarDelegate::GetMessageText() const {
return l10n_util::GetStringFUTF16( return l10n_util::GetStringFUTF16(
IDS_PROTECTED_MEDIA_IDENTIFIER_INFOBAR_QUESTION, IDS_PROTECTED_MEDIA_IDENTIFIER_INFOBAR_QUESTION,
...@@ -112,10 +97,10 @@ base::string16 ProtectedMediaIdentifierInfoBarDelegate::GetLinkText() const { ...@@ -112,10 +97,10 @@ base::string16 ProtectedMediaIdentifierInfoBarDelegate::GetLinkText() const {
bool ProtectedMediaIdentifierInfoBarDelegate::LinkClicked( bool ProtectedMediaIdentifierInfoBarDelegate::LinkClicked(
WindowOpenDisposition disposition) { WindowOpenDisposition disposition) {
const GURL learn_more_url(chrome::kEnhancedPlaybackNotificationLearnMoreURL);
InfoBarService::WebContentsFromInfoBar(infobar()) InfoBarService::WebContentsFromInfoBar(infobar())
->OpenURL(content::OpenURLParams( ->OpenURL(content::OpenURLParams(
learn_more_url, content::Referrer(), GURL(chrome::kEnhancedPlaybackNotificationLearnMoreURL),
content::Referrer(),
(disposition == CURRENT_TAB) ? NEW_FOREGROUND_TAB : disposition, (disposition == CURRENT_TAB) ? NEW_FOREGROUND_TAB : disposition,
ui::PAGE_TRANSITION_LINK, false)); ui::PAGE_TRANSITION_LINK, false));
return false; // Do not dismiss the info bar. return false; // Do not dismiss the info bar.
......
...@@ -32,7 +32,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -32,7 +32,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate {
ProtectedMediaIdentifierInfoBarDelegate(PermissionQueueController* controller, ProtectedMediaIdentifierInfoBarDelegate(PermissionQueueController* controller,
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
int contents_unique_id,
const std::string& display_languages); const std::string& display_languages);
~ProtectedMediaIdentifierInfoBarDelegate() override; ~ProtectedMediaIdentifierInfoBarDelegate() override;
...@@ -44,7 +43,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -44,7 +43,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate {
Type GetInfoBarType() const override; Type GetInfoBarType() const override;
int GetIconID() const override; int GetIconID() const override;
void InfoBarDismissed() override; void InfoBarDismissed() override;
bool ShouldExpireInternal(const NavigationDetails& details) const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
base::string16 GetButtonLabel(InfoBarButton button) const override; base::string16 GetButtonLabel(InfoBarButton button) const override;
bool Accept() override; bool Accept() override;
...@@ -55,7 +53,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -55,7 +53,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate {
PermissionQueueController* controller_; PermissionQueueController* controller_;
const PermissionRequestID id_; const PermissionRequestID id_;
GURL requesting_frame_; GURL requesting_frame_;
int contents_unique_id_;
std::string display_languages_; std::string display_languages_;
DISALLOW_COPY_AND_ASSIGN(ProtectedMediaIdentifierInfoBarDelegate); DISALLOW_COPY_AND_ASSIGN(ProtectedMediaIdentifierInfoBarDelegate);
......
...@@ -4,15 +4,9 @@ ...@@ -4,15 +4,9 @@
#include "chrome/browser/notifications/desktop_notification_infobar_delegate.h" #include "chrome/browser/notifications/desktop_notification_infobar_delegate.h"
#include "chrome/browser/content_settings/permission_queue_controller.h"
#include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/infobars/infobar_service.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "chrome/grit/locale_settings.h"
#include "components/content_settings/core/common/content_settings_types.h"
#include "components/content_settings/core/common/permission_request_id.h"
#include "components/infobars/core/infobar.h" #include "components/infobars/core/infobar.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "grit/theme_resources.h" #include "grit/theme_resources.h"
#include "net/base/net_util.h" #include "net/base/net_util.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -24,20 +18,15 @@ infobars::InfoBar* DesktopNotificationInfoBarDelegate::Create( ...@@ -24,20 +18,15 @@ infobars::InfoBar* DesktopNotificationInfoBarDelegate::Create(
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
const std::string& display_languages) { const std::string& display_languages) {
const content::NavigationEntry* committed_entry =
infobar_service->web_contents()->GetController().GetLastCommittedEntry();
return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar( return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar(
scoped_ptr<ConfirmInfoBarDelegate>(new DesktopNotificationInfoBarDelegate( scoped_ptr<ConfirmInfoBarDelegate>(new DesktopNotificationInfoBarDelegate(
controller, id, requesting_frame, controller, id, requesting_frame, display_languages))));
committed_entry ? committed_entry->GetUniqueID() : 0,
display_languages))));
} }
DesktopNotificationInfoBarDelegate::DesktopNotificationInfoBarDelegate( DesktopNotificationInfoBarDelegate::DesktopNotificationInfoBarDelegate(
PermissionQueueController* controller, PermissionQueueController* controller,
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
int contents_unique_id,
const std::string& display_languages) const std::string& display_languages)
: PermissionInfobarDelegate(controller, id, requesting_frame, : PermissionInfobarDelegate(controller, id, requesting_frame,
CONTENT_SETTINGS_TYPE_NOTIFICATIONS), CONTENT_SETTINGS_TYPE_NOTIFICATIONS),
......
...@@ -23,7 +23,6 @@ class DesktopNotificationInfoBarDelegate : public PermissionInfobarDelegate { ...@@ -23,7 +23,6 @@ class DesktopNotificationInfoBarDelegate : public PermissionInfobarDelegate {
DesktopNotificationInfoBarDelegate(PermissionQueueController* controller, DesktopNotificationInfoBarDelegate(PermissionQueueController* controller,
const PermissionRequestID& id, const PermissionRequestID& id,
const GURL& requesting_frame, const GURL& requesting_frame,
int contents_unique_id,
const std::string& display_languages); const std::string& display_languages);
~DesktopNotificationInfoBarDelegate() override; ~DesktopNotificationInfoBarDelegate() override;
......
...@@ -137,8 +137,7 @@ int SavePasswordInfoBarDelegate::GetIconID() const { ...@@ -137,8 +137,7 @@ int SavePasswordInfoBarDelegate::GetIconID() const {
bool SavePasswordInfoBarDelegate::ShouldExpire( bool SavePasswordInfoBarDelegate::ShouldExpire(
const NavigationDetails& details) const { const NavigationDetails& details) const {
return !details.is_redirect && return !details.is_redirect && ConfirmInfoBarDelegate::ShouldExpire(details);
infobars::InfoBarDelegate::ShouldExpire(details);
} }
void SavePasswordInfoBarDelegate::InfoBarDismissed() { void SavePasswordInfoBarDelegate::InfoBarDismissed() {
......
...@@ -52,7 +52,7 @@ class KeystonePromotionInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -52,7 +52,7 @@ class KeystonePromotionInfoBarDelegate : public ConfirmInfoBarDelegate {
// ConfirmInfoBarDelegate // ConfirmInfoBarDelegate
int GetIconID() const override; int GetIconID() const override;
bool ShouldExpireInternal(const NavigationDetails& details) const override; bool ShouldExpire(const NavigationDetails& details) const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
base::string16 GetButtonLabel(InfoBarButton button) const override; base::string16 GetButtonLabel(InfoBarButton button) const override;
bool Accept() override; bool Accept() override;
...@@ -108,9 +108,9 @@ int KeystonePromotionInfoBarDelegate::GetIconID() const { ...@@ -108,9 +108,9 @@ int KeystonePromotionInfoBarDelegate::GetIconID() const {
return IDR_PRODUCT_LOGO_32; return IDR_PRODUCT_LOGO_32;
} }
bool KeystonePromotionInfoBarDelegate::ShouldExpireInternal( bool KeystonePromotionInfoBarDelegate::ShouldExpire(
const NavigationDetails& details) const { const NavigationDetails& details) const {
return can_expire_; return can_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details);
} }
base::string16 KeystonePromotionInfoBarDelegate::GetMessageText() const { base::string16 KeystonePromotionInfoBarDelegate::GetMessageText() const {
......
...@@ -48,7 +48,7 @@ class AutolaunchInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -48,7 +48,7 @@ class AutolaunchInfoBarDelegate : public ConfirmInfoBarDelegate {
// ConfirmInfoBarDelegate: // ConfirmInfoBarDelegate:
int GetIconID() const override; int GetIconID() const override;
bool ShouldExpireInternal(const NavigationDetails& details) const override; bool ShouldExpire(const NavigationDetails& details) const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
base::string16 GetButtonLabel(InfoBarButton button) const override; base::string16 GetButtonLabel(InfoBarButton button) const override;
bool Accept() override; bool Accept() override;
...@@ -100,9 +100,9 @@ int AutolaunchInfoBarDelegate::GetIconID() const { ...@@ -100,9 +100,9 @@ int AutolaunchInfoBarDelegate::GetIconID() const {
return IDR_PRODUCT_LOGO_32; return IDR_PRODUCT_LOGO_32;
} }
bool AutolaunchInfoBarDelegate::ShouldExpireInternal( bool AutolaunchInfoBarDelegate::ShouldExpire(
const NavigationDetails& details) const { const NavigationDetails& details) const {
return should_expire_; return should_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details);
} }
base::string16 AutolaunchInfoBarDelegate::GetMessageText() const { base::string16 AutolaunchInfoBarDelegate::GetMessageText() const {
......
...@@ -74,7 +74,7 @@ class DefaultBrowserInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -74,7 +74,7 @@ class DefaultBrowserInfoBarDelegate : public ConfirmInfoBarDelegate {
// ConfirmInfoBarDelegate: // ConfirmInfoBarDelegate:
int GetIconID() const override; int GetIconID() const override;
bool ShouldExpireInternal(const NavigationDetails& details) const override; bool ShouldExpire(const NavigationDetails& details) const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
base::string16 GetButtonLabel(InfoBarButton button) const override; base::string16 GetButtonLabel(InfoBarButton button) const override;
bool OKButtonTriggersUACPrompt() const override; bool OKButtonTriggersUACPrompt() const override;
...@@ -136,9 +136,9 @@ int DefaultBrowserInfoBarDelegate::GetIconID() const { ...@@ -136,9 +136,9 @@ int DefaultBrowserInfoBarDelegate::GetIconID() const {
return IDR_PRODUCT_LOGO_32; return IDR_PRODUCT_LOGO_32;
} }
bool DefaultBrowserInfoBarDelegate::ShouldExpireInternal( bool DefaultBrowserInfoBarDelegate::ShouldExpire(
const NavigationDetails& details) const { const NavigationDetails& details) const {
return should_expire_; return should_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details);
} }
base::string16 DefaultBrowserInfoBarDelegate::GetMessageText() const { base::string16 DefaultBrowserInfoBarDelegate::GetMessageText() const {
......
...@@ -60,11 +60,7 @@ int AutofillCCInfoBarDelegate::GetIconID() const { ...@@ -60,11 +60,7 @@ int AutofillCCInfoBarDelegate::GetIconID() const {
return IDR_INFOBAR_AUTOFILL_CC; return IDR_INFOBAR_AUTOFILL_CC;
} }
void AutofillCCInfoBarDelegate::InfoBarDismissed() { bool AutofillCCInfoBarDelegate::ShouldExpire(
LogUserAction(AutofillMetrics::INFOBAR_DENIED);
}
bool AutofillCCInfoBarDelegate::ShouldExpireInternal(
const NavigationDetails& details) const { const NavigationDetails& details) const {
// The user has submitted a form, causing the page to navigate elsewhere. We // The user has submitted a form, causing the page to navigate elsewhere. We
// don't want the infobar to be expired at this point, because the user won't // don't want the infobar to be expired at this point, because the user won't
...@@ -72,6 +68,10 @@ bool AutofillCCInfoBarDelegate::ShouldExpireInternal( ...@@ -72,6 +68,10 @@ bool AutofillCCInfoBarDelegate::ShouldExpireInternal(
return false; return false;
} }
void AutofillCCInfoBarDelegate::InfoBarDismissed() {
LogUserAction(AutofillMetrics::INFOBAR_DENIED);
}
base::string16 AutofillCCInfoBarDelegate::GetMessageText() const { base::string16 AutofillCCInfoBarDelegate::GetMessageText() const {
return l10n_util::GetStringUTF16(IDS_AUTOFILL_CC_INFOBAR_TEXT); return l10n_util::GetStringUTF16(IDS_AUTOFILL_CC_INFOBAR_TEXT);
} }
......
...@@ -51,8 +51,8 @@ class AutofillCCInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -51,8 +51,8 @@ class AutofillCCInfoBarDelegate : public ConfirmInfoBarDelegate {
// ConfirmInfoBarDelegate: // ConfirmInfoBarDelegate:
Type GetInfoBarType() const override; Type GetInfoBarType() const override;
int GetIconID() const override; int GetIconID() const override;
bool ShouldExpire(const NavigationDetails& details) const override;
void InfoBarDismissed() override; void InfoBarDismissed() override;
bool ShouldExpireInternal(const NavigationDetails& details) const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
base::string16 GetButtonLabel(InfoBarButton button) const override; base::string16 GetButtonLabel(InfoBarButton button) const override;
bool Accept() override; bool Accept() override;
......
...@@ -53,12 +53,6 @@ ConfirmInfoBarDelegate::ConfirmInfoBarDelegate() ...@@ -53,12 +53,6 @@ ConfirmInfoBarDelegate::ConfirmInfoBarDelegate()
: InfoBarDelegate() { : InfoBarDelegate() {
} }
bool ConfirmInfoBarDelegate::ShouldExpireInternal(
const NavigationDetails& details) const {
return !details.did_replace_entry &&
InfoBarDelegate::ShouldExpireInternal(details);
}
bool ConfirmInfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const { bool ConfirmInfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const {
ConfirmInfoBarDelegate* confirm_delegate = ConfirmInfoBarDelegate* confirm_delegate =
delegate->AsConfirmInfoBarDelegate(); delegate->AsConfirmInfoBarDelegate();
......
...@@ -68,8 +68,6 @@ class ConfirmInfoBarDelegate : public infobars::InfoBarDelegate { ...@@ -68,8 +68,6 @@ class ConfirmInfoBarDelegate : public infobars::InfoBarDelegate {
protected: protected:
ConfirmInfoBarDelegate(); ConfirmInfoBarDelegate();
bool ShouldExpireInternal(const NavigationDetails& details) const override;
private: private:
// InfoBarDelegate: // InfoBarDelegate:
bool EqualsDelegate(infobars::InfoBarDelegate* delegate) const override; bool EqualsDelegate(infobars::InfoBarDelegate* delegate) const override;
......
...@@ -56,7 +56,6 @@ SkColor InfoBar::GetBottomColor(InfoBarDelegate::Type infobar_type) { ...@@ -56,7 +56,6 @@ SkColor InfoBar::GetBottomColor(InfoBarDelegate::Type infobar_type) {
void InfoBar::SetOwner(InfoBarManager* owner) { void InfoBar::SetOwner(InfoBarManager* owner) {
DCHECK(!owner_); DCHECK(!owner_);
owner_ = owner; owner_ = owner;
delegate_->StoreActiveEntryUniqueID();
PlatformSpecificSetOwner(); PlatformSpecificSetOwner();
} }
......
...@@ -50,9 +50,9 @@ class InfoBar : public gfx::AnimationDelegate { ...@@ -50,9 +50,9 @@ class InfoBar : public gfx::AnimationDelegate {
InfoBarDelegate* delegate() const { return delegate_.get(); } InfoBarDelegate* delegate() const { return delegate_.get(); }
void set_container(InfoBarContainer* container) { container_ = container; } void set_container(InfoBarContainer* container) { container_ = container; }
// Sets |owner_|. This also calls StoreActiveEntryUniqueID() on |delegate_|. // Sets |owner_|. This must only be called once as there's no way to extract
// This must only be called once as there's no way to extract an infobar from // an infobar from its owner without deleting it, for reparenting in another
// its owner without deleting it, for reparenting in another tab. // tab.
void SetOwner(InfoBarManager* owner); void SetOwner(InfoBarManager* owner);
// Makes the infobar visible. If |animate| is true, the infobar is then // Makes the infobar visible. If |animate| is true, the infobar is then
......
...@@ -41,10 +41,7 @@ bool InfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const { ...@@ -41,10 +41,7 @@ bool InfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const {
} }
bool InfoBarDelegate::ShouldExpire(const NavigationDetails& details) const { bool InfoBarDelegate::ShouldExpire(const NavigationDetails& details) const {
if (!details.is_navigation_to_different_page) return details.is_navigation_to_different_page && !details.did_replace_entry;
return false;
return ShouldExpireInternal(details);
} }
void InfoBarDelegate::InfoBarDismissed() { void InfoBarDelegate::InfoBarDismissed() {
...@@ -99,18 +96,7 @@ InfoBarDelegate::AsTranslateInfoBarDelegate() { ...@@ -99,18 +96,7 @@ InfoBarDelegate::AsTranslateInfoBarDelegate() {
return nullptr; return nullptr;
} }
void InfoBarDelegate::StoreActiveEntryUniqueID() { InfoBarDelegate::InfoBarDelegate() {
contents_unique_id_ = infobar()->owner()->GetActiveEntryID();
}
InfoBarDelegate::InfoBarDelegate() : contents_unique_id_(0) {
}
bool InfoBarDelegate::ShouldExpireInternal(
const NavigationDetails& details) const {
// NOTE: If you change this, be sure to check and adjust the behavior of
// anyone who overrides this as necessary!
return (contents_unique_id_ != details.entry_id) || details.is_reload;
} }
} // namespace infobars } // namespace infobars
...@@ -127,24 +127,12 @@ class InfoBarDelegate { ...@@ -127,24 +127,12 @@ class InfoBarDelegate {
void set_infobar(InfoBar* infobar) { infobar_ = infobar; } void set_infobar(InfoBar* infobar) { infobar_ = infobar; }
// Store the unique id for the active entry, to be used later upon navigation
// to determine if this InfoBarDelegate should be expired.
void StoreActiveEntryUniqueID();
protected: protected:
InfoBarDelegate(); InfoBarDelegate();
// Returns true if the navigation is to a new URL or a reload occured.
virtual bool ShouldExpireInternal(const NavigationDetails& details) const;
int contents_unique_id() const { return contents_unique_id_; }
InfoBar* infobar() { return infobar_; } InfoBar* infobar() { return infobar_; }
private: private:
// The unique id of the active NavigationEntry of the WebContents that we were
// opened for. Used to help expire on navigations.
int contents_unique_id_;
// The InfoBar associated with us. // The InfoBar associated with us.
InfoBar* infobar_; InfoBar* infobar_;
......
...@@ -36,9 +36,9 @@ int SimpleAlertInfoBarDelegate::GetIconID() const { ...@@ -36,9 +36,9 @@ int SimpleAlertInfoBarDelegate::GetIconID() const {
return icon_id_; return icon_id_;
} }
bool SimpleAlertInfoBarDelegate::ShouldExpireInternal( bool SimpleAlertInfoBarDelegate::ShouldExpire(
const NavigationDetails& details) const { const NavigationDetails& details) const {
return auto_expire_ && ConfirmInfoBarDelegate::ShouldExpireInternal(details); return auto_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details);
} }
base::string16 SimpleAlertInfoBarDelegate::GetMessageText() const { base::string16 SimpleAlertInfoBarDelegate::GetMessageText() const {
......
...@@ -31,7 +31,7 @@ class SimpleAlertInfoBarDelegate : public ConfirmInfoBarDelegate { ...@@ -31,7 +31,7 @@ class SimpleAlertInfoBarDelegate : public ConfirmInfoBarDelegate {
// ConfirmInfoBarDelegate: // ConfirmInfoBarDelegate:
int GetIconID() const override; int GetIconID() const override;
bool ShouldExpireInternal(const NavigationDetails& details) const override; bool ShouldExpire(const NavigationDetails& details) const override;
base::string16 GetMessageText() const override; base::string16 GetMessageText() const override;
int GetButtons() const override; int GetButtons() const override;
......
...@@ -359,16 +359,6 @@ int TranslateInfoBarDelegate::GetIconID() const { ...@@ -359,16 +359,6 @@ int TranslateInfoBarDelegate::GetIconID() const {
return translate_manager_->translate_client()->GetInfobarIconID(); return translate_manager_->translate_client()->GetInfobarIconID();
} }
bool TranslateInfoBarDelegate::ShouldExpire(
const NavigationDetails& details) const {
// Note: we allow closing this infobar even if the main frame navigation
// was programmatic and not initiated by the user - crbug.com/70261 .
if (!details.is_navigation_to_different_page && !details.is_main_frame)
return false;
return infobars::InfoBarDelegate::ShouldExpireInternal(details);
}
void TranslateInfoBarDelegate::InfoBarDismissed() { void TranslateInfoBarDelegate::InfoBarDismissed() {
if (step_ != translate::TRANSLATE_STEP_BEFORE_TRANSLATE) if (step_ != translate::TRANSLATE_STEP_BEFORE_TRANSLATE)
return; return;
......
...@@ -199,7 +199,6 @@ class TranslateInfoBarDelegate : public infobars::InfoBarDelegate { ...@@ -199,7 +199,6 @@ class TranslateInfoBarDelegate : public infobars::InfoBarDelegate {
// InfoBarDelegate: // InfoBarDelegate:
Type GetInfoBarType() const override; Type GetInfoBarType() const override;
int GetIconID() const override; int GetIconID() const override;
bool ShouldExpire(const NavigationDetails& details) const override;
void InfoBarDismissed() override; void InfoBarDismissed() override;
TranslateInfoBarDelegate* AsTranslateInfoBarDelegate() override; TranslateInfoBarDelegate* AsTranslateInfoBarDelegate() override;
......
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