[Password Manager] Fix to recognise failed login attempt for sites where content server pushes new …

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 "base/run_loop.h"
@@ skipping 1101 matching lines @@
   std::string submit =
       "document.getElementById('username').value = 'overwrite_me';"
       "document.getElementById('password').value = 'random';"
       "document.getElementById('non-form-button').click();";
   ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), submit));
   observer.Wait();
 
   EXPECT_TRUE(prompt_observer->IsShowingPrompt());
 }
 
+// Test that if login fails and content server pushes a different login form
+// with action URL having different scheme and/or case. Heuristic shall be able

[vabr (Chromium), 2014/08/21 14:54:12]

I'm actually not convinced that the action URL comparison should be case
insensitive. URL paths are case insensitive. Are there real examples, when
Chrome's password manager misunderstands a failed login because of a different
letter case?

[vabr (Chromium), 2014/08/21 14:54:12]

The test below does not test the different scheme, only the different case.
Please introduce a separate test to test the different scheme.

[Pritam Nikam, 2014/08/21 16:49:21]

I didn't find any real example though, this particular test-case is failing
keeping password_manager.cc untouched. Thats the reason I've added this case
insensitivity  check as well.

[Pritam Nikam, 2014/08/21 16:49:21]

Seems it's quite difficult as we need http and https servers running.

Current test class uses only HTTP test_server i,e, embedded_test_server().
Moreover, it does not have support for DNS resolution, it publishes a random
port and with 127.0.0.1 (localhost) every time we run browser_tests.

So even we have a HTTPS server running, port number will be different than that
of and test cases will fail even if we ignore schemes.

e.g. 
say separate_login_form.html action >>
http://127.0.0.1:1234:/password/done_and_separate_login_form.html

and done_and_separate_login_form.html action >>
https://127.0.0.1:7890:/password/done_and_separate_login_form.html

comparing urls even ignoring schemes will not match.

+// identify such cases and *shall not* prompt to save incorrect password.
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
+                       NoPromptForLoginFailedAndServerPushSeperateLoginForm) {
+  NavigateToFile("/password/seperate_login_form.html");

[vabr (Chromium), 2014/08/21 14:54:12]

typo: seperate -> separate
Please also correct in the file names, id names etc.

[Pritam Nikam, 2014/08/21 16:49:21]

Done.

+
+  NavigationObserver observer(WebContents());
+  scoped_ptr<PromptObserver> prompt_observer(
+      PromptObserver::Create(WebContents()));
+  std::string fill_and_submit =
+      "document.getElementById('username_seperate').value = 'temp';"
+      "document.getElementById('password_seperate').value = 'random';"
+      "document.getElementById('submit_seperate').click();";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
+  observer.Wait();
+
+  EXPECT_FALSE(prompt_observer->IsShowingPrompt());
+}