From d06bcaca1a000d55e80d9cbf0865eb32f7668ba1 Mon Sep 17 00:00:00 2001
From: "gcasto@chromium.org"
 <gcasto@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 27 Mar 2013 21:17:57 +0000
Subject: [PATCH] Fix the no password save issue for ajax login

The idea is that if a navigation is triggered by non-user gesture after a password form is submitted, then we assume that this navigation is related to the previous form submit and chrome should prompt for saving password if all criteria for provisionally saving password are met.

The link between the previous form submit and the coming navigation is weak, so its possible for chrome to inherit password forms from previous state when it should not. However most false cases would be aborted at PasswordManager::ProvisionallySavePassword, which does a list of sanity checks for saving password, such as if the form is already seen on the current page.

BUG=43219


Review URL: https://chromiumcodereview.appspot.com/12713007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191024 0039d316-1c4b-4281-b951-d872f2087c98
---
 .../password_manager_browsertest.cc                | 129 +++++++++++++++++++++
 chrome/chrome_tests.gypi                           |   1 +
 chrome/test/data/password/password_xhr_done.html   |   5 +
 chrome/test/data/password/password_xhr_submit.html |  27 +++++
 content/public/renderer/document_state.h           |   9 ++
 content/renderer/render_view_impl.cc               |  17 +++
 6 files changed, 188 insertions(+)
 create mode 100644 chrome/browser/password_manager/password_manager_browsertest.cc
 create mode 100644 chrome/test/data/password/password_xhr_done.html
 create mode 100644 chrome/test/data/password/password_xhr_submit.html

diff --git a/chrome/browser/password_manager/password_manager_browsertest.cc b/chrome/browser/password_manager/password_manager_browsertest.cc
new file mode 100644
index 000000000000..e42034115396
--- /dev/null
+++ b/chrome/browser/password_manager/password_manager_browsertest.cc
@@ -0,0 +1,129 @@
+// 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 "chrome/browser/infobars/confirm_infobar_delegate.h"
+#include "chrome/browser/infobars/infobar_service.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "ui/base/keycodes/keyboard_codes.h"
+
+namespace {
+
+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),
+        info_bar_shown_(false),
+        infobar_service_(InfoBarService::FromWebContents(web_contents)) {
+    registrar_.Add(this,
+                   chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_ADDED,
+                   content::Source<InfoBarService>(infobar_service_));
+  }
+
+  virtual ~NavigationObserver() {}
+
+  void Wait() {
+    message_loop_runner_->Run();
+  }
+
+  bool InfoBarWasShown() {
+    return info_bar_shown_;
+  }
+
+  // content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE {
+    // Accept in the infobar.
+    InfoBarDelegate* infobar = infobar_service_->GetInfoBarDelegateAt(0);
+    ConfirmInfoBarDelegate* confirm_infobar =
+        infobar->AsConfirmInfoBarDelegate();
+    confirm_infobar->Accept();
+    info_bar_shown_ = true;
+  }
+
+  // content::WebContentsObserver
+  virtual void DidFinishLoad(int64 frame_id,
+                             const GURL& validated_url,
+                             bool is_main_frame,
+                             content::RenderViewHost* render_view_host) {
+    message_loop_runner_->Quit();
+  }
+
+ private:
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+  bool info_bar_shown_;
+  content::NotificationRegistrar registrar_;
+  InfoBarService* infobar_service_;
+};
+
+}  // namespace
+
+class PasswordManagerBrowserTest : public InProcessBrowserTest {
+ protected:
+  content::WebContents* WebContents() {
+    return browser()->tab_strip_model()->GetActiveWebContents();
+  }
+
+  content::RenderViewHost* RenderViewHost() {
+    return WebContents()->GetRenderViewHost();
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, PromptForXHRSubmit) {
+  ASSERT_TRUE(test_server()->Start());
+
+  GURL url = test_server()->GetURL("files/password/password_xhr_submit.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Verify that we show the save password prompt if a form returns false
+  // in its onsubmit handler but instead logs in/navigates via XHR.
+  // Note that calling 'submit()' on a form with javascript doesn't call
+  // the onsubmit handler, so we click the submit button instead.
+  NavigationObserver observer(WebContents());
+  std::string fill_and_submit =
+      "document.getElementById('username_field').value = 'temp';"
+      "document.getElementById('password_field').value = 'random';"
+      "document.getElementById('submit_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
+  observer.Wait();
+  EXPECT_TRUE(observer.InfoBarWasShown());
+}
+
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, NoPromptForOtherXHR) {
+  ASSERT_TRUE(test_server()->Start());
+
+  GURL url = test_server()->GetURL("files/password/password_xhr_submit.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Verify that if random XHR navigation occurs, we don't try and save the
+  // password.
+  //
+  // We may want to change this functionality in the future to account for
+  // cases where the element that users click on isn't a submit button.
+  NavigationObserver observer(WebContents());
+  std::string fill_and_navigate =
+      "document.getElementById('username_field').value = 'temp';"
+      "document.getElementById('password_field').value = 'random';"
+      "send_xhr()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_navigate));
+  observer.Wait();
+  EXPECT_FALSE(observer.InfoBarWasShown());
+}
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 1b7fd422f930..7e97ceee1ff6 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1359,6 +1359,7 @@
         'browser/net/websocket_browsertest.cc',
         'browser/notifications/message_center_notifications_browsertest.cc',
         'browser/page_cycler/page_cycler_browsertest.cc',
+        'browser/password_manager/password_manager_browsertest.cc',
         'browser/performance_monitor/performance_monitor_browsertest.cc',
         'browser/policy/cloud/cloud_policy_browsertest.cc',
         'browser/policy/cloud/cloud_policy_manager_browsertest.cc',
diff --git a/chrome/test/data/password/password_xhr_done.html b/chrome/test/data/password/password_xhr_done.html
new file mode 100644
index 000000000000..99e103fd2bcc
--- /dev/null
+++ b/chrome/test/data/password/password_xhr_done.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+XHR navigation complete.
+</body>
+</html>
diff --git a/chrome/test/data/password/password_xhr_submit.html b/chrome/test/data/password/password_xhr_submit.html
new file mode 100644
index 000000000000..5a3d60f388f7
--- /dev/null
+++ b/chrome/test/data/password/password_xhr_submit.html
@@ -0,0 +1,27 @@
+<html>
+<head>
+<script>
+
+function state_changed(xhr) {
+  if (xhr.readyState == 4)
+    window.location.href = "password_xhr_done.html";
+}
+
+function send_xhr() {
+  var xhr = new XMLHttpRequest();
+  xhr.onreadystatechange = function() { state_changed(xhr); };
+  xhr.open("GET", "password_xhr_submit.html", true);
+  xhr.send(null);
+}
+
+</script>
+</head>
+<body>
+<form action="password_xhr_submit.html" onsubmit="send_xhr(); return false;"
+      id="testform">
+  <input type="text" id="username_field" name="username_field">
+  <input type="password" id="password_field" name="password_field">
+  <input type="submit" id="submit_button" name="submit_button">
+</form>
+</body>
+</html>
diff --git a/content/public/renderer/document_state.h b/content/public/renderer/document_state.h
index 3168776091b2..d13be7f4b726 100644
--- a/content/public/renderer/document_state.h
+++ b/content/public/renderer/document_state.h
@@ -169,6 +169,15 @@ class DocumentState : public WebKit::WebDataSource::ExtraData {
     searchable_form_encoding_ = encoding;
   }
 
+  // If set, contains the PasswordForm that we believe triggered the current
+  // navigation (there is some ambiguity in the case of javascript initiated
+  // navigations). This information is used by the PasswordManager to determine
+  // if the user should be prompted to save their password.
+  //
+  // Note that setting this field doesn't affect where the data is sent or what
+  // origin we associate it with, only whether we prompt the user to save it.
+  // That is, a false positive is a usability issue (e.g. may try to save a
+  // mis-typed password) not a security issue.
   PasswordForm* password_form_data() const {
     return password_form_data_.get();
   }
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 103160696c37..b1e6bbd83650 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -3357,6 +3357,23 @@ void RenderViewImpl::didStartProvisionalLoad(WebFrame* frame) {
     navigation_gesture_ = WebUserGestureIndicator::isProcessingUserGesture() ?
         NavigationGestureUser : NavigationGestureAuto;
 
+    // If the navigation is not triggered by a user gesture, e.g. by some ajax
+    // callback, then inherit the submitted password form from the previous
+    // state. This fixes the no password save issue for ajax login, tracked in
+    // [http://crbug/43219]. Note that there are still some sites that this
+    // fails for because they use some element other than a submit button to
+    // trigger submission.
+    if (navigation_gesture_ == NavigationGestureAuto) {
+      DocumentState* old_document_state = DocumentState::FromDataSource(
+          frame->dataSource());
+      const content::PasswordForm* old_password_form =
+          old_document_state->password_form_data();
+      if (old_password_form) {
+        document_state->set_password_form_data(
+            make_scoped_ptr(new content::PasswordForm(*old_password_form)));
+      }
+    }
+
     // Make sure redirect tracking state is clear for the new load.
     completed_client_redirect_src_ = Referrer();
   } else if (frame->parent()->isLoading()) {
-- 
2.11.4.GIT