Persist "Save password" infobars across redirects

As described in the bug, on some pages we observe a chain of redirections between the sign-in and the final landing form, during which the "save password" infobar is prematurely destroyed.

This CL overrides the ShouldExpire method for "save password" infobars to survive page transitions which are redirects.

BUG=26186

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243889 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/password_manager/password_manager_browsertest.cc

@@ -46,10 +46,15 @@ class NavigationObserver : public content::NotificationObserver,
       : content::WebContentsObserver(web_contents),
         message_loop_runner_(new content::MessageLoopRunner),
         infobar_shown_(false),
+        infobar_removed_(false),
+        should_automatically_accept_infobar_(true),
         infobar_service_(InfoBarService::FromWebContents(web_contents)) {
     registrar_.Add(this,
                    chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_ADDED,
                    content::Source<InfoBarService>(infobar_service_));
+    registrar_.Add(this,
+                   chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
+                   content::Source<InfoBarService>(infobar_service_));
   }
 
   virtual ~NavigationObserver() {}
@@ -65,9 +70,23 @@ class NavigationObserver : public content::NotificationObserver,
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE {
-    infobar_service_->infobar_at(0)->delegate()->AsConfirmInfoBarDelegate()->
-        Accept();
-    infobar_shown_ = true;
+    switch (type) {
+      case chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_ADDED:
+        if (should_automatically_accept_infobar_) {
+          infobar_service_->infobar_at(0)
+              ->delegate()
+              ->AsConfirmInfoBarDelegate()
+              ->Accept();
+        }
+        infobar_shown_ = true;
+        return;
+      case chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED:
+        infobar_removed_ = true;
+        return;
+      default:
+        NOTREACHED();
+        return;
+    }
   }
 
   // content::WebContentsObserver:
@@ -85,6 +104,11 @@ class NavigationObserver : public content::NotificationObserver,
   }
 
   bool infobar_shown() const { return infobar_shown_; }
+  bool infobar_removed() const { return infobar_removed_; }
+
+  void disable_should_automatically_accept_infobar() {
+    should_automatically_accept_infobar_ = false;
+  }
 
   void Wait() {
     message_loop_runner_->Run();
@@ -94,6 +118,10 @@ class NavigationObserver : public content::NotificationObserver,
   std::string wait_for_path_;
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
   bool infobar_shown_;
+  bool infobar_removed_;
+  // If |should_automatically_accept_infobar_| is true, then whenever the test
+  // sees an infobar added, it will click its accepting button. Default = true.
+  bool should_automatically_accept_infobar_;
   content::NotificationRegistrar registrar_;
   InfoBarService* infobar_service_;
 
@@ -182,6 +210,29 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
   EXPECT_TRUE(observer.infobar_shown());
 }
 
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, Redirects) {
+  NavigateToFile("/password/password_form.html");
+
+  // Fill a form and submit through a <input type="submit"> button. The form
+  // points to a redirection page.
+  NavigationObserver observer(WebContents());
+  std::string fill_and_submit =
+      "document.getElementById('username_redirect').value = 'temp';"
+      "document.getElementById('password_redirect').value = 'random';"
+      "document.getElementById('submit_redirect').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
+  observer.disable_should_automatically_accept_infobar();
+  observer.Wait();
+  EXPECT_TRUE(observer.infobar_shown());
+
+  // The redirection page now redirects via Javascript. We check that the
+  // infobar stays.
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(),
+                                     "window.location.href = 'done.html';"));
+  observer.Wait();
+  EXPECT_FALSE(observer.infobar_removed());
+}
+
 IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
                        PromptForSubmitUsingJavaScript) {
   NavigateToFile("/password/password_form.html");

chrome/browser/password_manager/password_manager_delegate_impl.cc

@@ -20,9 +20,11 @@
 #include "components/autofill/content/common/autofill_messages.h"
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/common/password_form.h"
+#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/page_transition_types.h"
 #include "content/public/common/ssl_status.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "grit/chromium_strings.h"
@@ -65,6 +67,10 @@ class SavePasswordInfoBarDelegate : public ConfirmInfoBarDelegate {
                               const std::string& uma_histogram_suffix);
   virtual ~SavePasswordInfoBarDelegate();
 
+  // InfoBarDelegate
+  virtual bool ShouldExpire(const content::LoadCommittedDetails& details)
+      const OVERRIDE;
+
   // ConfirmInfoBarDelegate
   virtual int GetIconID() const OVERRIDE;
   virtual Type GetInfoBarType() const OVERRIDE;
@@ -154,6 +160,13 @@ SavePasswordInfoBarDelegate::~SavePasswordInfoBarDelegate() {
   }
 }
 
+bool SavePasswordInfoBarDelegate::ShouldExpire(
+    const content::LoadCommittedDetails& details) const {
+  bool is_not_redirect = !(details.entry->GetTransitionType() &
+                           content::PAGE_TRANSITION_IS_REDIRECT_MASK);
+  return is_not_redirect && InfoBarDelegate::ShouldExpire(details);
+}
+
 int SavePasswordInfoBarDelegate::GetIconID() const {
   return IDR_INFOBAR_SAVE_PASSWORD;
 }

chrome/test/data/password/password_form.html

@@ -23,6 +23,12 @@
   <input type="submit" name="input_submit_button_no_id">
 </form>
 
+<form method="POST" action="redirect.html" id="redirectform">
+  <input type="text" id="username_redirect" name="username_redirect">
+  <input type="password" id="password_redirect" name="password_redirect">
+  <input type="submit" id="submit_redirect" name="submit_redirect">
+</form>
+
 <!--
 Don't add anything inside this form, and don't change the order of the
 elements. Because the elements have no "id" or "name" attributes, the test

chrome/test/data/password/redirect.html

+<html>
+<body>
+This is a non-empty redirection page.
+</body>
+</html>