Persist "Save password" infobars across redirects

chrome/browser/password_manager/password_manager_browsertest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ skipping 28 matching lines @@
 
 // Observer that waits for navigation to complete and for the password infobar
 // to be shown.
 class NavigationObserver : public content::NotificationObserver,
                            public content::WebContentsObserver {
  public:
   explicit NavigationObserver(content::WebContents* web_contents)
       : content::WebContentsObserver(web_contents),
         message_loop_runner_(new content::MessageLoopRunner),
         infobar_shown_(false),
+        infobar_removed_(false),
+        auto_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() {}
 
   // Normally Wait() will not return until a main frame navigation occurs.
   // If a path is set, Wait() will return after this path has been seen,
   // regardless of the frame that navigated. Useful for multi-frame pages.
   void SetPathToWaitFor(const std::string& path) {
     wait_for_path_ = path;
   }
 
   // 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 (auto_accept_infobar_)

[Ilya Sherman, 2014/01/08 23:34:53]

nit: This needs curlies.

[vabr (Chromium), 2014/01/09 11:39:38]

Done.

+          infobar_service_->infobar_at(0)
+              ->delegate()
+              ->AsConfirmInfoBarDelegate()
+              ->Accept();
+        infobar_shown_ = true;
+        break;
+      case chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED:
+        infobar_removed_ = true;
+        break;
+    }

[Ilya Sherman, 2014/01/08 23:34:53]

nit: Pretty sure you need to also have a default case in order to appease clang.

[vabr (Chromium), 2014/01/09 11:39:38]

Clang seemed to be fine with this, but I still added default: NOTREACHED for
documentation sake.

Also, I changed breaks to returns, because there is meant to be no common
processing after the switch statement.

   }
 
   // content::WebContentsObserver:
   virtual void DidFinishLoad(
       int64 frame_id,
       const GURL& validated_url,
       bool is_main_frame,
       content::RenderViewHost* render_view_host) OVERRIDE {
     if (!wait_for_path_.empty()) {
       if (validated_url.path() == wait_for_path_)
         message_loop_runner_->Quit();
     } else if (is_main_frame) {
       message_loop_runner_->Quit();
     }
   }
 
   bool infobar_shown() const { return infobar_shown_; }
+  bool infobar_removed() const { return infobar_removed_; }
+
+  void DisableAutoAcceptingInfobars() { auto_accept_infobar_ = false; }

[Ilya Sherman, 2014/01/08 23:34:53]

nit: I'd name this "disable_foo", where "foo" is the variable name (see also my
comment below).

[vabr (Chromium), 2014/01/09 11:39:38]

Done.

 
   void Wait() {
     message_loop_runner_->Run();
   }
 
  private:
   std::string wait_for_path_;
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
   bool infobar_shown_;
+  bool infobar_removed_;
+  bool auto_accept_infobar_;

[Ilya Sherman, 2014/01/08 23:34:53]

nit: Please document this member var.

[Ilya Sherman, 2014/01/08 23:34:53]

Optional nit: Perhaps a name like "should_automatically_accept_infobar_" would
be a bit clearer?

[vabr (Chromium), 2014/01/09 11:39:38]

Done.

[vabr (Chromium), 2014/01/09 11:39:38]

Done.

   content::NotificationRegistrar registrar_;
   InfoBarService* infobar_service_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationObserver);
 };
 
 }  // namespace
 
 
 // PasswordManagerBrowserTest -------------------------------------------------
@@ skipping 68 matching lines @@
   NavigationObserver observer(WebContents());
   std::string fill_and_submit =
       "document.getElementById('username_field').value = 'temp';"
       "document.getElementById('password_field').value = 'random';"
       "document.getElementById('input_submit_button').click()";
   ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
   observer.Wait();
   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.DisableAutoAcceptingInfobars();
+  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");
 
   // Fill a form and submit using <button> that calls submit() on the form.
   // This should work regardless of the type of element, as long as submit() is
   // called.
   NavigationObserver observer(WebContents());
   std::string fill_and_submit =
       "document.getElementById('username_field').value = 'temp';"
@@ skipping 274 matching lines @@
       "document.getElementById('username_field').value = 'temp';"
       "document.getElementById('password_field').value = 'random';"
       "document.getElementById('input_submit_button').click();"
       "window.location.href = 'done.html';";
 
   ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), save_and_remove));
   ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), navigate_frame));
   observer.Wait();
   // The only thing we check here is that there is no use-after-free reported.
 }