summaryrefslogtreecommitdiffstats
path: root/components/infobars
diff options
context:
space:
mode:
authorpkasting <pkasting@chromium.org>2015-07-02 00:59:09 -0700
committerCommit bot <commit-bot@chromium.org>2015-07-02 07:59:43 +0000
commitf9fec9942a46dfa0242b9a03ce515d9ace75ea7e (patch)
treeb8575204008757476f1cc3a8bd1a11414c738836 /components/infobars
parent681973dc83ad56b14f1ed606d28b588ce1ba19bd (diff)
downloadchromium_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.cc1
-rw-r--r--components/infobars/core/infobar.h6
-rw-r--r--components/infobars/core/infobar_delegate.cc11
-rw-r--r--components/infobars/core/infobar_delegate.h5
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);
};