diff options
author | pkasting <pkasting@chromium.org> | 2015-05-20 15:27:42 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-05-20 22:28:05 +0000 |
commit | 34cc3c8d2b014a6413709ba88249eb2bd3c933a7 (patch) | |
tree | e34843cfef41c41de2683f851e8688e8a25c24f5 | |
parent | 9316d3b983530169e6c50d61fa87eeebecd8dcfd (diff) | |
download | chromium_src-34cc3c8d2b014a6413709ba88249eb2bd3c933a7.zip chromium_src-34cc3c8d2b014a6413709ba88249eb2bd3c933a7.tar.gz chromium_src-34cc3c8d2b014a6413709ba88249eb2bd3c933a7.tar.bz2 |
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}
25 files changed, 37 insertions, 141 deletions
diff --git a/chrome/browser/extensions/api/debugger/debugger_api.cc b/chrome/browser/extensions/api/debugger/debugger_api.cc index 27623d9..7d38197 100644 --- a/chrome/browser/extensions/api/debugger/debugger_api.cc +++ b/chrome/browser/extensions/api/debugger/debugger_api.cc @@ -170,8 +170,8 @@ class ExtensionDevToolsInfoBarDelegate : public ConfirmInfoBarDelegate { // ConfirmInfoBarDelegate: Type GetInfoBarType() const override; + bool ShouldExpire(const NavigationDetails& details) const override; void InfoBarDismissed() override; - bool ShouldExpireInternal(const NavigationDetails& details) const override; base::string16 GetMessageText() const override; int GetButtons() const override; bool Cancel() override; @@ -214,16 +214,16 @@ ExtensionDevToolsInfoBarDelegate::GetInfoBarType() const { return WARNING_TYPE; } +bool ExtensionDevToolsInfoBarDelegate::ShouldExpire( + const NavigationDetails& details) const { + return false; +} + void ExtensionDevToolsInfoBarDelegate::InfoBarDismissed() { if (client_host_) client_host_->MarkAsDismissed(); } -bool ExtensionDevToolsInfoBarDelegate::ShouldExpireInternal( - const NavigationDetails& details) const { - return false; -} - base::string16 ExtensionDevToolsInfoBarDelegate::GetMessageText() const { return l10n_util::GetStringFUTF16(IDS_DEV_TOOLS_INFOBAR_LABEL, base::UTF8ToUTF16(client_name_)); diff --git a/chrome/browser/geolocation/geolocation_infobar_delegate.cc b/chrome/browser/geolocation/geolocation_infobar_delegate.cc index b69acf0..1b6eb2d 100644 --- a/chrome/browser/geolocation/geolocation_infobar_delegate.cc +++ b/chrome/browser/geolocation/geolocation_infobar_delegate.cc @@ -4,18 +4,10 @@ #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/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 "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/locale_settings.h" #include "grit/theme_resources.h" #include "net/base/net_util.h" #include "ui/base/l10n/l10n_util.h" @@ -28,22 +20,15 @@ infobars::InfoBar* GeolocationInfoBarDelegate::Create( const PermissionRequestID& id, const GURL& requesting_frame, 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( - scoped_ptr<ConfirmInfoBarDelegate>(delegate))); + scoped_ptr<ConfirmInfoBarDelegate>(new GeolocationInfoBarDelegate( + controller, id, requesting_frame, display_languages)))); } GeolocationInfoBarDelegate::GeolocationInfoBarDelegate( PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - int contents_unique_id, const std::string& display_languages) : PermissionInfobarDelegate(controller, id, requesting_frame, CONTENT_SETTINGS_TYPE_GEOLOCATION), diff --git a/chrome/browser/geolocation/geolocation_infobar_delegate.h b/chrome/browser/geolocation/geolocation_infobar_delegate.h index b131024..1d4616a 100644 --- a/chrome/browser/geolocation/geolocation_infobar_delegate.h +++ b/chrome/browser/geolocation/geolocation_infobar_delegate.h @@ -27,7 +27,6 @@ class GeolocationInfoBarDelegate : public PermissionInfobarDelegate { GeolocationInfoBarDelegate(PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - int contents_unique_id, const std::string& display_languages); ~GeolocationInfoBarDelegate() override; diff --git a/chrome/browser/media/midi_permission_infobar_delegate.cc b/chrome/browser/media/midi_permission_infobar_delegate.cc index a90cdf6..5a590cd 100644 --- a/chrome/browser/media/midi_permission_infobar_delegate.cc +++ b/chrome/browser/media/midi_permission_infobar_delegate.cc @@ -4,14 +4,9 @@ #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/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 "content/public/browser/navigation_entry.h" -#include "content/public/browser/web_contents.h" #include "grit/theme_resources.h" #include "net/base/net_util.h" #include "ui/base/l10n/l10n_util.h" @@ -24,20 +19,15 @@ infobars::InfoBar* MidiPermissionInfoBarDelegate::Create( const GURL& requesting_frame, const std::string& display_languages, ContentSettingsType type) { - const content::NavigationEntry* committed_entry = - infobar_service->web_contents()->GetController().GetLastCommittedEntry(); return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar( scoped_ptr<ConfirmInfoBarDelegate>(new MidiPermissionInfoBarDelegate( - controller, id, requesting_frame, - committed_entry ? committed_entry->GetUniqueID() : 0, - display_languages, type)))); + controller, id, requesting_frame, display_languages, type)))); } MidiPermissionInfoBarDelegate::MidiPermissionInfoBarDelegate( PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - int contents_unique_id, const std::string& display_languages, ContentSettingsType type) : PermissionInfobarDelegate(controller, id, requesting_frame, type), diff --git a/chrome/browser/media/midi_permission_infobar_delegate.h b/chrome/browser/media/midi_permission_infobar_delegate.h index f6ffd15..befb181 100644 --- a/chrome/browser/media/midi_permission_infobar_delegate.h +++ b/chrome/browser/media/midi_permission_infobar_delegate.h @@ -31,7 +31,6 @@ class MidiPermissionInfoBarDelegate : public PermissionInfobarDelegate { MidiPermissionInfoBarDelegate(PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - int contents_unique_id, const std::string& display_languages, ContentSettingsType type); ~MidiPermissionInfoBarDelegate() override; diff --git a/chrome/browser/media/protected_media_identifier_infobar_delegate.cc b/chrome/browser/media/protected_media_identifier_infobar_delegate.cc index deb24c1..c155968 100644 --- a/chrome/browser/media/protected_media_identifier_infobar_delegate.cc +++ b/chrome/browser/media/protected_media_identifier_infobar_delegate.cc @@ -9,7 +9,6 @@ #include "chrome/common/url_constants.h" #include "chrome/grit/generated_resources.h" #include "components/infobars/core/infobar.h" -#include "content/public/browser/navigation_entry.h" #include "grit/components_strings.h" #include "grit/theme_resources.h" #include "net/base/net_util.h" @@ -22,14 +21,10 @@ infobars::InfoBar* ProtectedMediaIdentifierInfoBarDelegate::Create( const PermissionRequestID& id, const GURL& requesting_frame, const std::string& display_languages) { - const content::NavigationEntry* committed_entry = - infobar_service->web_contents()->GetController().GetLastCommittedEntry(); return infobar_service->AddInfoBar( infobar_service->CreateConfirmInfoBar(scoped_ptr<ConfirmInfoBarDelegate>( new ProtectedMediaIdentifierInfoBarDelegate( - controller, id, requesting_frame, - committed_entry ? committed_entry->GetUniqueID() : 0, - display_languages)))); + controller, id, requesting_frame, display_languages)))); } @@ -38,13 +33,11 @@ ProtectedMediaIdentifierInfoBarDelegate:: PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - int contents_unique_id, const std::string& display_languages) : ConfirmInfoBarDelegate(), controller_(controller), id_(id), requesting_frame_(requesting_frame), - contents_unique_id_(contents_unique_id), display_languages_(display_languages) { } @@ -80,14 +73,6 @@ void ProtectedMediaIdentifierInfoBarDelegate::InfoBarDismissed() { 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 { return l10n_util::GetStringFUTF16( IDS_PROTECTED_MEDIA_IDENTIFIER_INFOBAR_QUESTION, @@ -112,10 +97,10 @@ base::string16 ProtectedMediaIdentifierInfoBarDelegate::GetLinkText() const { bool ProtectedMediaIdentifierInfoBarDelegate::LinkClicked( WindowOpenDisposition disposition) { - const GURL learn_more_url(chrome::kEnhancedPlaybackNotificationLearnMoreURL); InfoBarService::WebContentsFromInfoBar(infobar()) ->OpenURL(content::OpenURLParams( - learn_more_url, content::Referrer(), + GURL(chrome::kEnhancedPlaybackNotificationLearnMoreURL), + content::Referrer(), (disposition == CURRENT_TAB) ? NEW_FOREGROUND_TAB : disposition, ui::PAGE_TRANSITION_LINK, false)); return false; // Do not dismiss the info bar. diff --git a/chrome/browser/media/protected_media_identifier_infobar_delegate.h b/chrome/browser/media/protected_media_identifier_infobar_delegate.h index 700c717..1681da8 100644 --- a/chrome/browser/media/protected_media_identifier_infobar_delegate.h +++ b/chrome/browser/media/protected_media_identifier_infobar_delegate.h @@ -32,7 +32,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate { ProtectedMediaIdentifierInfoBarDelegate(PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - int contents_unique_id, const std::string& display_languages); ~ProtectedMediaIdentifierInfoBarDelegate() override; @@ -44,7 +43,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate { Type GetInfoBarType() const override; int GetIconID() const override; void InfoBarDismissed() override; - bool ShouldExpireInternal(const NavigationDetails& details) const override; base::string16 GetMessageText() const override; base::string16 GetButtonLabel(InfoBarButton button) const override; bool Accept() override; @@ -55,7 +53,6 @@ class ProtectedMediaIdentifierInfoBarDelegate : public ConfirmInfoBarDelegate { PermissionQueueController* controller_; const PermissionRequestID id_; GURL requesting_frame_; - int contents_unique_id_; std::string display_languages_; DISALLOW_COPY_AND_ASSIGN(ProtectedMediaIdentifierInfoBarDelegate); diff --git a/chrome/browser/notifications/desktop_notification_infobar_delegate.cc b/chrome/browser/notifications/desktop_notification_infobar_delegate.cc index eb34214..a08b999 100644 --- a/chrome/browser/notifications/desktop_notification_infobar_delegate.cc +++ b/chrome/browser/notifications/desktop_notification_infobar_delegate.cc @@ -4,15 +4,9 @@ #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/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 "content/public/browser/navigation_entry.h" -#include "content/public/browser/web_contents.h" #include "grit/theme_resources.h" #include "net/base/net_util.h" #include "ui/base/l10n/l10n_util.h" @@ -24,20 +18,15 @@ infobars::InfoBar* DesktopNotificationInfoBarDelegate::Create( const PermissionRequestID& id, const GURL& requesting_frame, const std::string& display_languages) { - const content::NavigationEntry* committed_entry = - infobar_service->web_contents()->GetController().GetLastCommittedEntry(); return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar( scoped_ptr<ConfirmInfoBarDelegate>(new DesktopNotificationInfoBarDelegate( - controller, id, requesting_frame, - committed_entry ? committed_entry->GetUniqueID() : 0, - display_languages)))); + controller, id, requesting_frame, display_languages)))); } DesktopNotificationInfoBarDelegate::DesktopNotificationInfoBarDelegate( PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - int contents_unique_id, const std::string& display_languages) : PermissionInfobarDelegate(controller, id, requesting_frame, CONTENT_SETTINGS_TYPE_NOTIFICATIONS), diff --git a/chrome/browser/notifications/desktop_notification_infobar_delegate.h b/chrome/browser/notifications/desktop_notification_infobar_delegate.h index 52b36ae..8f918f6 100644 --- a/chrome/browser/notifications/desktop_notification_infobar_delegate.h +++ b/chrome/browser/notifications/desktop_notification_infobar_delegate.h @@ -23,7 +23,6 @@ class DesktopNotificationInfoBarDelegate : public PermissionInfobarDelegate { DesktopNotificationInfoBarDelegate(PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - int contents_unique_id, const std::string& display_languages); ~DesktopNotificationInfoBarDelegate() override; diff --git a/chrome/browser/password_manager/save_password_infobar_delegate.cc b/chrome/browser/password_manager/save_password_infobar_delegate.cc index 9e020a1..ef259a6 100644 --- a/chrome/browser/password_manager/save_password_infobar_delegate.cc +++ b/chrome/browser/password_manager/save_password_infobar_delegate.cc @@ -137,8 +137,7 @@ int SavePasswordInfoBarDelegate::GetIconID() const { bool SavePasswordInfoBarDelegate::ShouldExpire( const NavigationDetails& details) const { - return !details.is_redirect && - infobars::InfoBarDelegate::ShouldExpire(details); + return !details.is_redirect && ConfirmInfoBarDelegate::ShouldExpire(details); } void SavePasswordInfoBarDelegate::InfoBarDismissed() { diff --git a/chrome/browser/ui/cocoa/keystone_infobar_delegate.mm b/chrome/browser/ui/cocoa/keystone_infobar_delegate.mm index ca95f75..02a3434 100644 --- a/chrome/browser/ui/cocoa/keystone_infobar_delegate.mm +++ b/chrome/browser/ui/cocoa/keystone_infobar_delegate.mm @@ -52,7 +52,7 @@ class KeystonePromotionInfoBarDelegate : public ConfirmInfoBarDelegate { // ConfirmInfoBarDelegate 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 GetButtonLabel(InfoBarButton button) const override; bool Accept() override; @@ -108,9 +108,9 @@ int KeystonePromotionInfoBarDelegate::GetIconID() const { return IDR_PRODUCT_LOGO_32; } -bool KeystonePromotionInfoBarDelegate::ShouldExpireInternal( +bool KeystonePromotionInfoBarDelegate::ShouldExpire( const NavigationDetails& details) const { - return can_expire_; + return can_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details); } base::string16 KeystonePromotionInfoBarDelegate::GetMessageText() const { diff --git a/chrome/browser/ui/startup/autolaunch_prompt_win.cc b/chrome/browser/ui/startup/autolaunch_prompt_win.cc index 7f3559a..071fb75 100644 --- a/chrome/browser/ui/startup/autolaunch_prompt_win.cc +++ b/chrome/browser/ui/startup/autolaunch_prompt_win.cc @@ -48,7 +48,7 @@ class AutolaunchInfoBarDelegate : public ConfirmInfoBarDelegate { // ConfirmInfoBarDelegate: 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 GetButtonLabel(InfoBarButton button) const override; bool Accept() override; @@ -100,9 +100,9 @@ int AutolaunchInfoBarDelegate::GetIconID() const { return IDR_PRODUCT_LOGO_32; } -bool AutolaunchInfoBarDelegate::ShouldExpireInternal( +bool AutolaunchInfoBarDelegate::ShouldExpire( const NavigationDetails& details) const { - return should_expire_; + return should_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details); } base::string16 AutolaunchInfoBarDelegate::GetMessageText() const { diff --git a/chrome/browser/ui/startup/default_browser_prompt.cc b/chrome/browser/ui/startup/default_browser_prompt.cc index 5853399..86a592ba8 100644 --- a/chrome/browser/ui/startup/default_browser_prompt.cc +++ b/chrome/browser/ui/startup/default_browser_prompt.cc @@ -74,7 +74,7 @@ class DefaultBrowserInfoBarDelegate : public ConfirmInfoBarDelegate { // ConfirmInfoBarDelegate: 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 GetButtonLabel(InfoBarButton button) const override; bool OKButtonTriggersUACPrompt() const override; @@ -136,9 +136,9 @@ int DefaultBrowserInfoBarDelegate::GetIconID() const { return IDR_PRODUCT_LOGO_32; } -bool DefaultBrowserInfoBarDelegate::ShouldExpireInternal( +bool DefaultBrowserInfoBarDelegate::ShouldExpire( const NavigationDetails& details) const { - return should_expire_; + return should_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details); } base::string16 DefaultBrowserInfoBarDelegate::GetMessageText() const { diff --git a/components/autofill/core/browser/autofill_cc_infobar_delegate.cc b/components/autofill/core/browser/autofill_cc_infobar_delegate.cc index 50cab56..7dc13a7 100644 --- a/components/autofill/core/browser/autofill_cc_infobar_delegate.cc +++ b/components/autofill/core/browser/autofill_cc_infobar_delegate.cc @@ -60,11 +60,7 @@ int AutofillCCInfoBarDelegate::GetIconID() const { return IDR_INFOBAR_AUTOFILL_CC; } -void AutofillCCInfoBarDelegate::InfoBarDismissed() { - LogUserAction(AutofillMetrics::INFOBAR_DENIED); -} - -bool AutofillCCInfoBarDelegate::ShouldExpireInternal( +bool AutofillCCInfoBarDelegate::ShouldExpire( const NavigationDetails& details) const { // 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 @@ -72,6 +68,10 @@ bool AutofillCCInfoBarDelegate::ShouldExpireInternal( return false; } +void AutofillCCInfoBarDelegate::InfoBarDismissed() { + LogUserAction(AutofillMetrics::INFOBAR_DENIED); +} + base::string16 AutofillCCInfoBarDelegate::GetMessageText() const { return l10n_util::GetStringUTF16(IDS_AUTOFILL_CC_INFOBAR_TEXT); } diff --git a/components/autofill/core/browser/autofill_cc_infobar_delegate.h b/components/autofill/core/browser/autofill_cc_infobar_delegate.h index c5a685a..911e8ba 100644 --- a/components/autofill/core/browser/autofill_cc_infobar_delegate.h +++ b/components/autofill/core/browser/autofill_cc_infobar_delegate.h @@ -51,8 +51,8 @@ class AutofillCCInfoBarDelegate : public ConfirmInfoBarDelegate { // ConfirmInfoBarDelegate: Type GetInfoBarType() const override; int GetIconID() const override; + bool ShouldExpire(const NavigationDetails& details) const override; void InfoBarDismissed() override; - bool ShouldExpireInternal(const NavigationDetails& details) const override; base::string16 GetMessageText() const override; base::string16 GetButtonLabel(InfoBarButton button) const override; bool Accept() override; diff --git a/components/infobars/core/confirm_infobar_delegate.cc b/components/infobars/core/confirm_infobar_delegate.cc index 9c00449a5..daef8c1 100644 --- a/components/infobars/core/confirm_infobar_delegate.cc +++ b/components/infobars/core/confirm_infobar_delegate.cc @@ -53,12 +53,6 @@ ConfirmInfoBarDelegate::ConfirmInfoBarDelegate() : InfoBarDelegate() { } -bool ConfirmInfoBarDelegate::ShouldExpireInternal( - const NavigationDetails& details) const { - return !details.did_replace_entry && - InfoBarDelegate::ShouldExpireInternal(details); -} - bool ConfirmInfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const { ConfirmInfoBarDelegate* confirm_delegate = delegate->AsConfirmInfoBarDelegate(); diff --git a/components/infobars/core/confirm_infobar_delegate.h b/components/infobars/core/confirm_infobar_delegate.h index 0f35f8c..d36ea22 100644 --- a/components/infobars/core/confirm_infobar_delegate.h +++ b/components/infobars/core/confirm_infobar_delegate.h @@ -68,8 +68,6 @@ class ConfirmInfoBarDelegate : public infobars::InfoBarDelegate { protected: ConfirmInfoBarDelegate(); - bool ShouldExpireInternal(const NavigationDetails& details) const override; - private: // InfoBarDelegate: bool EqualsDelegate(infobars::InfoBarDelegate* delegate) const override; diff --git a/components/infobars/core/infobar.cc b/components/infobars/core/infobar.cc index 6e7aca7..91c9273 100644 --- a/components/infobars/core/infobar.cc +++ b/components/infobars/core/infobar.cc @@ -56,7 +56,6 @@ SkColor InfoBar::GetBottomColor(InfoBarDelegate::Type infobar_type) { void InfoBar::SetOwner(InfoBarManager* owner) { DCHECK(!owner_); owner_ = owner; - delegate_->StoreActiveEntryUniqueID(); PlatformSpecificSetOwner(); } diff --git a/components/infobars/core/infobar.h b/components/infobars/core/infobar.h index ebced9f..28a1543 100644 --- a/components/infobars/core/infobar.h +++ b/components/infobars/core/infobar.h @@ -50,9 +50,9 @@ class InfoBar : public gfx::AnimationDelegate { InfoBarDelegate* delegate() const { return delegate_.get(); } void set_container(InfoBarContainer* container) { container_ = container; } - // Sets |owner_|. This also calls StoreActiveEntryUniqueID() on |delegate_|. - // This must only be called once as there's no way to extract an infobar from - // its owner without deleting it, for reparenting in another tab. + // Sets |owner_|. This must only be called once as there's no way to extract + // an infobar from its owner without deleting it, for reparenting in another + // tab. void SetOwner(InfoBarManager* owner); // Makes the infobar visible. If |animate| is true, the infobar is then diff --git a/components/infobars/core/infobar_delegate.cc b/components/infobars/core/infobar_delegate.cc index 84cd0e1..37fbdf7 100644 --- a/components/infobars/core/infobar_delegate.cc +++ b/components/infobars/core/infobar_delegate.cc @@ -41,10 +41,7 @@ bool InfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const { } bool InfoBarDelegate::ShouldExpire(const NavigationDetails& details) const { - if (!details.is_navigation_to_different_page) - return false; - - return ShouldExpireInternal(details); + return details.is_navigation_to_different_page && !details.did_replace_entry; } void InfoBarDelegate::InfoBarDismissed() { @@ -99,18 +96,7 @@ InfoBarDelegate::AsTranslateInfoBarDelegate() { return nullptr; } -void InfoBarDelegate::StoreActiveEntryUniqueID() { - 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; +InfoBarDelegate::InfoBarDelegate() { } } // namespace infobars diff --git a/components/infobars/core/infobar_delegate.h b/components/infobars/core/infobar_delegate.h index bd7455f..5d5ff37 100644 --- a/components/infobars/core/infobar_delegate.h +++ b/components/infobars/core/infobar_delegate.h @@ -127,24 +127,12 @@ class InfoBarDelegate { 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: 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_; } 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. InfoBar* infobar_; diff --git a/components/infobars/core/simple_alert_infobar_delegate.cc b/components/infobars/core/simple_alert_infobar_delegate.cc index fa0188e..56fbfce 100644 --- a/components/infobars/core/simple_alert_infobar_delegate.cc +++ b/components/infobars/core/simple_alert_infobar_delegate.cc @@ -36,9 +36,9 @@ int SimpleAlertInfoBarDelegate::GetIconID() const { return icon_id_; } -bool SimpleAlertInfoBarDelegate::ShouldExpireInternal( +bool SimpleAlertInfoBarDelegate::ShouldExpire( const NavigationDetails& details) const { - return auto_expire_ && ConfirmInfoBarDelegate::ShouldExpireInternal(details); + return auto_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details); } base::string16 SimpleAlertInfoBarDelegate::GetMessageText() const { diff --git a/components/infobars/core/simple_alert_infobar_delegate.h b/components/infobars/core/simple_alert_infobar_delegate.h index 64dc03a..671bc15 100644 --- a/components/infobars/core/simple_alert_infobar_delegate.h +++ b/components/infobars/core/simple_alert_infobar_delegate.h @@ -31,7 +31,7 @@ class SimpleAlertInfoBarDelegate : public ConfirmInfoBarDelegate { // ConfirmInfoBarDelegate: int GetIconID() const override; - bool ShouldExpireInternal(const NavigationDetails& details) const override; + bool ShouldExpire(const NavigationDetails& details) const override; base::string16 GetMessageText() const override; int GetButtons() const override; diff --git a/components/translate/core/browser/translate_infobar_delegate.cc b/components/translate/core/browser/translate_infobar_delegate.cc index 9552463..8006c06f 100644 --- a/components/translate/core/browser/translate_infobar_delegate.cc +++ b/components/translate/core/browser/translate_infobar_delegate.cc @@ -359,16 +359,6 @@ int TranslateInfoBarDelegate::GetIconID() const { 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() { if (step_ != translate::TRANSLATE_STEP_BEFORE_TRANSLATE) return; diff --git a/components/translate/core/browser/translate_infobar_delegate.h b/components/translate/core/browser/translate_infobar_delegate.h index dca1979..ec3ae32 100644 --- a/components/translate/core/browser/translate_infobar_delegate.h +++ b/components/translate/core/browser/translate_infobar_delegate.h @@ -199,7 +199,6 @@ class TranslateInfoBarDelegate : public infobars::InfoBarDelegate { // InfoBarDelegate: Type GetInfoBarType() const override; int GetIconID() const override; - bool ShouldExpire(const NavigationDetails& details) const override; void InfoBarDismissed() override; TranslateInfoBarDelegate* AsTranslateInfoBarDelegate() override; |