diff options
author | pkasting <pkasting@chromium.org> | 2015-07-02 00:59:09 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-07-02 07:59:43 +0000 |
commit | f9fec9942a46dfa0242b9a03ce515d9ace75ea7e (patch) | |
tree | b8575204008757476f1cc3a8bd1a11414c738836 /components/infobars | |
parent | 681973dc83ad56b14f1ed606d28b588ce1ba19bd (diff) | |
download | chromium_src-f9fec9942a46dfa0242b9a03ce515d9ace75ea7e.zip chromium_src-f9fec9942a46dfa0242b9a03ce515d9ace75ea7e.tar.gz chromium_src-f9fec9942a46dfa0242b9a03ce515d9ace75ea7e.tar.bz2 |
Don't let navigation dismiss infobars added during that navigation.
Turns out my reasoning on https://codereview.chromium.org/1142153002/ was
incorrect, and the unique ID check there was doing something after all.
Specifically, if you add an infobar while a navigation is pending, then when the
navigation commits, it will remove the infobar again (unless this check is
re-added).
This patch restores that check.
Note that the behavior now is still not quite what it was before this whole
cleanup began; we're now a bit more conservative on various infobars that used
to override ShouldExpireInternal(), in that we now run this ID check when we
didn't then. I think that's fine, arguably more correct.
BUG=492610
TEST=Run chrome, force it to crash, relaunch with --disable-session-crashed-bubble, ensure you get a "session crashed" infobar and it is not auto-dismissed
TBR=droger
Review URL: https://codereview.chromium.org/1215053007
Cr-Commit-Position: refs/heads/master@{#337187}
Diffstat (limited to 'components/infobars')
-rw-r--r-- | components/infobars/core/infobar.cc | 1 | ||||
-rw-r--r-- | components/infobars/core/infobar.h | 6 | ||||
-rw-r--r-- | components/infobars/core/infobar_delegate.cc | 11 | ||||
-rw-r--r-- | components/infobars/core/infobar_delegate.h | 5 |
4 files changed, 18 insertions, 5 deletions
diff --git a/components/infobars/core/infobar.cc b/components/infobars/core/infobar.cc index 91c9273..29c1995 100644 --- a/components/infobars/core/infobar.cc +++ b/components/infobars/core/infobar.cc @@ -56,6 +56,7 @@ SkColor InfoBar::GetBottomColor(InfoBarDelegate::Type infobar_type) { void InfoBar::SetOwner(InfoBarManager* owner) { DCHECK(!owner_); owner_ = owner; + delegate_->set_nav_entry_id(owner->GetActiveEntryID()); PlatformSpecificSetOwner(); } diff --git a/components/infobars/core/infobar.h b/components/infobars/core/infobar.h index 28a1543..f5edd12 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 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 also sets the nav entry ID 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. 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 d9f7655..de0ae13 100644 --- a/components/infobars/core/infobar_delegate.cc +++ b/components/infobars/core/infobar_delegate.cc @@ -41,7 +41,14 @@ bool InfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const { } bool InfoBarDelegate::ShouldExpire(const NavigationDetails& details) const { - return details.is_navigation_to_different_page && !details.did_replace_entry; + return details.is_navigation_to_different_page && + !details.did_replace_entry && + // This next condition ensures a navigation that passes the above + // conditions doesn't dismiss infobars added while that navigation was + // already in process. We carve out an exception for reloads since we + // want reloads to dismiss infobars, but they will have unchanged entry + // IDs. + ((nav_entry_id_ != details.entry_id) || details.is_reload); } void InfoBarDelegate::InfoBarDismissed() { @@ -100,7 +107,7 @@ InfoBarDelegate::AsTranslateInfoBarDelegate() { return nullptr; } -InfoBarDelegate::InfoBarDelegate() { +InfoBarDelegate::InfoBarDelegate() : nav_entry_id_(0) { } } // namespace infobars diff --git a/components/infobars/core/infobar_delegate.h b/components/infobars/core/infobar_delegate.h index 4c1e234..9342292 100644 --- a/components/infobars/core/infobar_delegate.h +++ b/components/infobars/core/infobar_delegate.h @@ -63,6 +63,7 @@ class InfoBarDelegate { bool is_navigation_to_different_page; // True if the entry replaced the existing one. bool did_replace_entry; + bool is_reload; bool is_redirect; }; @@ -125,6 +126,7 @@ class InfoBarDelegate { virtual translate::TranslateInfoBarDelegate* AsTranslateInfoBarDelegate(); void set_infobar(InfoBar* infobar) { infobar_ = infobar; } + void set_nav_entry_id(int nav_entry_id) { nav_entry_id_ = nav_entry_id; } protected: InfoBarDelegate(); @@ -135,6 +137,9 @@ class InfoBarDelegate { // The InfoBar associated with us. InfoBar* infobar_; + // The ID of the active navigation entry at the time we became owned. + int nav_entry_id_; + DISALLOW_COPY_AND_ASSIGN(InfoBarDelegate); }; |