From f9fec9942a46dfa0242b9a03ce515d9ace75ea7e Mon Sep 17 00:00:00 2001
From: pkasting <pkasting@chromium.org>
Date: Thu, 2 Jul 2015 00:59:09 -0700
Subject: 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}
---
 components/infobars/core/infobar.cc          |  1 +
 components/infobars/core/infobar.h           |  6 +++---
 components/infobars/core/infobar_delegate.cc | 11 +++++++++--
 components/infobars/core/infobar_delegate.h  |  5 +++++
 4 files changed, 18 insertions(+), 5 deletions(-)

(limited to 'components/infobars')

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);
 };
 
-- 
cgit v1.1