Fix to enable the cached credentials login test

Fix to enable the cached credentials login test



Login with cached credentials

Change-Id: I89c0cce73102348349fafdd003ad7a373f8fe3e8

SVN changes up to revision 157305

Change-Id: Ib94ce981eeab2a40ab1dfb6296a7dd631852131f

BUG=32583

[beeps, 2012-09-18 17:59:14]

https://codereview.chromium.org/10944014/diff/2001/chrome/test/pyautolib/pyau...
File chrome/test/pyautolib/pyauto.py (right):

https://codereview.chromium.org/10944014/diff/2001/chrome/test/pyautolib/pyau...
chrome/test/pyautolib/pyauto.py:4697: def LoginWithCachedCredentials(self,
username, password):
Mimics def Login except for the actual hook it calls

[craigdh, 2012-09-18 18:16:35]

https://codereview.chromium.org/10944014/diff/2001/chrome/browser/automation/...
File chrome/browser/automation/testing_automation_provider_chromeos.cc (right):

https://codereview.chromium.org/10944014/diff/2001/chrome/browser/automation/...
chrome/browser/automation/testing_automation_provider_chromeos.cc:382:
webui_login_display->Login(username, password);
This looks identical to SubmitLoginForm except for this line. I suggest making
them one command that takes a use_cached_credentials boolean option to choose
the login behavior. This would also allow removal of the duplicated (non-trival)
Python code.

https://codereview.chromium.org/10944014/diff/2001/chrome/test/pyautolib/pyau...
File chrome/test/pyautolib/pyauto.py (right):

https://codereview.chromium.org/10944014/diff/2001/chrome/test/pyautolib/pyau...
chrome/test/pyautolib/pyauto.py:4697: def LoginWithCachedCredentials(self,
username, password):
On 2012/09/18 17:59:14, beeps wrote:
> Mimics def Login except for the actual hook it calls

Following my suggestion in testing_automation_provider_chromeos.cc, don't
duplicate this code. Instead give Login a use_cached_credentials option
(defaults to False).

https://codereview.chromium.org/10944014/diff/2001/chrome/test/pyautolib/pyau...
chrome/test/pyautolib/pyauto.py:4718: # GAIA authentication.
Please remove this todo, I already did this but missed removing the todo.

[beeps, 2012-09-19 16:29:02]

https://codereview.chromium.org/10944014/diff/2001/chrome/browser/automation/...
File chrome/browser/automation/testing_automation_provider_chromeos.cc (right):

https://codereview.chromium.org/10944014/diff/2001/chrome/browser/automation/...
chrome/browser/automation/testing_automation_provider_chromeos.cc:382:
webui_login_display->Login(username, password);
On 2012/09/18 18:16:35, craigdh wrote:
> This looks identical to SubmitLoginForm except for this line. I suggest making
> them one command that takes a use_cached_credentials boolean option to choose
> the login behavior. This would also allow removal of the duplicated
(non-trival)
> Python code.

Done.

https://codereview.chromium.org/10944014/diff/2001/chrome/test/pyautolib/pyau...
File chrome/test/pyautolib/pyauto.py (right):

https://codereview.chromium.org/10944014/diff/2001/chrome/test/pyautolib/pyau...
chrome/test/pyautolib/pyauto.py:4697: def LoginWithCachedCredentials(self,
username, password):
On 2012/09/18 18:16:35, craigdh wrote:
> On 2012/09/18 17:59:14, beeps wrote:
> > Mimics def Login except for the actual hook it calls
> 
> Following my suggestion in testing_automation_provider_chromeos.cc, don't
> duplicate this code. Instead give Login a use_cached_credentials option
> (defaults to False).

Done.

https://codereview.chromium.org/10944014/diff/2001/chrome/test/pyautolib/pyau...
chrome/test/pyautolib/pyauto.py:4718: # GAIA authentication.
On 2012/09/18 18:16:35, craigdh wrote:
> Please remove this todo, I already did this but missed removing the todo.

Done.

[craigdh, 2012-09-19 16:58:47]

https://codereview.chromium.org/10944014/diff/9001/chrome/browser/automation/...
File chrome/browser/automation/testing_automation_provider_chromeos.cc (right):

https://codereview.chromium.org/10944014/diff/9001/chrome/browser/automation/...
chrome/browser/automation/testing_automation_provider_chromeos.cc:367:
!args->GetBoolean("use cached credentials", &use_cached_credentials)) {
Use underscores. These are normally treated like variable names, so no spaces.

[beeps, 2012-09-19 17:14:39]

https://codereview.chromium.org/10944014/diff/9001/chrome/browser/automation/...
File chrome/browser/automation/testing_automation_provider_chromeos.cc (right):

https://codereview.chromium.org/10944014/diff/9001/chrome/browser/automation/...
chrome/browser/automation/testing_automation_provider_chromeos.cc:367:
!args->GetBoolean("use cached credentials", &use_cached_credentials)) {
On 2012/09/19 16:58:47, craigdh wrote:
> Use underscores. These are normally treated like variable names, so no spaces.

Done.

[craigdh, 2012-09-19 17:21:22]

lgtm, though you will need OWNERs approval for browser/automation (jam) and
test/pyautolib (nirnimesh).

[Nirnimesh, 2012-09-21 18:46:42]

http://codereview.chromium.org/10944014/diff/3007/chrome/browser/automation/t...
File chrome/browser/automation/testing_automation_provider_chromeos.cc (right):

http://codereview.chromium.org/10944014/diff/3007/chrome/browser/automation/t...
chrome/browser/automation/testing_automation_provider_chromeos.cc:386:
webui_login_display->Login(username, password);
are you sure this is what is being done at the user pod screen?

http://codereview.chromium.org/10944014/diff/3007/chrome/test/functional/chro...
File chrome/test/functional/chromeos_login.py (right):

http://codereview.chromium.org/10944014/diff/3007/chrome/test/functional/chro...
chrome/test/functional/chromeos_login.py:320: class
ChromeosLoginCachedCredentialsUserPod(ChromeosLogin):
why?

http://codereview.chromium.org/10944014/diff/3007/chrome/test/functional/chro...
chrome/test/functional/chromeos_login.py:365:
self.pyauto.Login(credentials['username'], credentials['password'], True)
prefer named args.
username=credentials['username'],
password=...,
use_cached_credentials=True

http://codereview.chromium.org/10944014/diff/3007/chrome/test/pyautolib/pyaut...
File chrome/test/pyautolib/pyauto.py (right):

http://codereview.chromium.org/10944014/diff/3007/chrome/test/pyautolib/pyaut...
chrome/test/pyautolib/pyauto.py:4613: this case the GetNextEvent automation
command will always time out.
Add Args: section
Explain what use_cached_credentials means

[Nirnimesh, 2012-09-21 18:47:40]

Please fix the CL description too

[beeps, 2012-09-26 18:31:09]

http://codereview.chromium.org/10944014/diff/3007/chrome/browser/automation/t...
File chrome/browser/automation/testing_automation_provider_chromeos.cc (right):

http://codereview.chromium.org/10944014/diff/3007/chrome/browser/automation/t...
chrome/browser/automation/testing_automation_provider_chromeos.cc:386:
webui_login_display->Login(username, password);
tested it with bogus password and it fails
On 2012/09/21 18:46:43, Nirnimesh wrote:
> are you sure this is what is being done at the user pod screen?

http://codereview.chromium.org/10944014/diff/3007/chrome/test/functional/chro...
File chrome/test/functional/chromeos_login.py (right):

http://codereview.chromium.org/10944014/diff/3007/chrome/test/functional/chro...
chrome/test/functional/chromeos_login.py:320: class
ChromeosLoginCachedCredentialsUserPod(ChromeosLogin):
wanted to reuse testGoodLogin, have duplicated it instead in this revision.
On 2012/09/21 18:46:43, Nirnimesh wrote:
> why?

http://codereview.chromium.org/10944014/diff/3007/chrome/test/functional/chro...
chrome/test/functional/chromeos_login.py:365:
self.pyauto.Login(credentials['username'], credentials['password'], True)
On 2012/09/21 18:46:43, Nirnimesh wrote:
> prefer named args.
> username=credentials['username'],
> password=...,
> use_cached_credentials=True

Done.

http://codereview.chromium.org/10944014/diff/3007/chrome/test/pyautolib/pyaut...
File chrome/test/pyautolib/pyauto.py (right):

http://codereview.chromium.org/10944014/diff/3007/chrome/test/pyautolib/pyaut...
chrome/test/pyautolib/pyauto.py:4613: this case the GetNextEvent automation
command will always time out.
On 2012/09/21 18:46:43, Nirnimesh wrote:
> Add Args: section
> Explain what use_cached_credentials means

Done.

[Nirnimesh, 2012-09-26 18:44:32]

LGTM