[cros] Fetch cookies after online authentication is successful.

[cros] Fetch cookies after online authentication is successful.

BUG=chromium-os:9814
TEST=Manual. Checked that web properties load w/o auth request for new user/existing user/existing with cookies cleared/existing with changed password.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=69016

[Nikita (slow), 2010-12-03 14:04:23]

I haven't tested this change properly yet. There are 2 issues:

- on TOT sync seems not working properly i.e. not reporting its successful
initialization in settings
- even with --parallel-auth, and using WiFi constantly get
"pending_requests=false" in log

[Chris Masone, 2010-12-03 15:39:13]

On 2010/12/03 14:04:23, Nikita Kostylev wrote:
> I haven't tested this change properly yet. There are 2 issues:
> 
> - on TOT sync seems not working properly i.e. not reporting its successful
> initialization in settings
> - even with --parallel-auth, and using WiFi constantly get
> "pending_requests=false" in log

If online login completes before offline login, that'd happen.  We put in a DNS
pre-warm for www.google.com a couple weeks ago, and so I could certainly believe
that on a good network connection you're seeing ClientLogin take <700ms (about
how long offline takes)

Will review the code shortly.

[Chris Masone, 2010-12-03 16:35:56]

On 2010/12/03 15:39:13, Chris Masone wrote:
> On 2010/12/03 14:04:23, Nikita Kostylev wrote:
> > I haven't tested this change properly yet. There are 2 issues:
> > 
> > - on TOT sync seems not working properly i.e. not reporting its successful
> > initialization in settings
> > - even with --parallel-auth, and using WiFi constantly get
> > "pending_requests=false" in log
> 
> If online login completes before offline login, that'd happen.  We put in a
DNS
> pre-warm for http://www.google.com a couple weeks ago, and so I could
certainly believe
> that on a good network connection you're seeing ClientLogin take <700ms (about
> how long offline takes)
> 
> Will review the code shortly.

The code looks good, but the tests (obviously) need to build and pass :-)

[Chris Masone, 2010-12-03 16:40:02]

http://codereview.chromium.org/5641001/diff/1/chrome/browser/chromeos/login/l...
File chrome/browser/chromeos/login/login_performer.cc (right):

http://codereview.chromium.org/5641001/diff/1/chrome/browser/chromeos/login/l...
chrome/browser/chromeos/login/login_performer.cc:118: // TODO(nkostylev):
Execute CookieFetcher->AttemptFetch() here once
can take this comment out now :-)

[Nikita (slow), 2010-12-03 16:49:43]

Fixed build, will look at the tests.
Even if all is OK I won't commit this code - want to find out why sync is not
working on TOT.

http://codereview.chromium.org/5641001/diff/1/chrome/browser/chromeos/login/l...
File chrome/browser/chromeos/login/login_performer.cc (right):

http://codereview.chromium.org/5641001/diff/1/chrome/browser/chromeos/login/l...
chrome/browser/chromeos/login/login_performer.cc:118: // TODO(nkostylev):
Execute CookieFetcher->AttemptFetch() here once
On 2010/12/03 16:40:02, Chris Masone wrote:
> can take this comment out now :-)

Done.

[Nikita (slow), 2010-12-03 16:52:42]

http://codereview.chromium.org/5641001/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/login_utils.cc (right):

http://codereview.chromium.org/5641001/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/login_utils.cc:237: if (!pending_requests) {
Note:
When network is not connected, only online auth would pass and pending_requests
would be false.
In that case we still make attempt to fetch cookies/tokens.
Just in case if network will be up.

Credentials passed from offline auth success shouldn't work when passed to
FetchCookies/FetchTokens, right?

[Nikita (slow), 2010-12-03 16:53:25]

http://codereview.chromium.org/5641001/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/login_utils.cc (right):

http://codereview.chromium.org/5641001/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/login_utils.cc:237: if (!pending_requests) {
On 2010/12/03 16:52:42, Nikita Kostylev wrote:
> Note:
> only online auth would pass and pending_requests
> would be false.

only offline auth would pass and pending_requests
would be false.

[Chris Masone, 2010-12-03 18:47:38]

http://codereview.chromium.org/5641001/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/login_utils.cc (right):

http://codereview.chromium.org/5641001/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/login_utils.cc:237: if (!pending_requests) {
On 2010/12/03 16:52:42, Nikita Kostylev wrote:
> Note:
> When network is not connected, only online auth would pass and
pending_requests
> would be false.
> In that case we still make attempt to fetch cookies/tokens.
> Just in case if network will be up.
> 
> Credentials passed from offline auth success shouldn't work when passed to
> FetchCookies/FetchTokens, right?

They would not work, no.  I'm pretty sure they're valid Credentials objects, but
with empty values and stuff.

[Nikita (slow), 2010-12-06 15:09:48]

Update on testing:

I still couldn't make sync working on TOT - filed a bug on that.
I don't think that cookie/token fetching refactoring needs changes (just wait
for sync to work again to test it out).

[Chris Masone, 2010-12-13 08:00:19]

On 2010/12/06 15:09:48, Nikita Kostylev wrote:
> Update on testing:
> 
> I still couldn't make sync working on TOT - filed a bug on that.
> I don't think that cookie/token fetching refactoring needs changes (just wait
> for sync to work again to test it out).

Sync is working again, yes?  Can this be tested and landed now?

[Nikita (slow), 2010-12-13 09:17:41]

On Mon, Dec 13, 2010 at 11:00 AM, <cmasone@chromium.org> wrote:
>
> Sync is working again, yes?  Can this be tested and landed now?
>

Yes, I'll test it today.

Still need your LGTM though :)

-- 
Nikita

[Chris Masone, 2010-12-13 14:59:33]

D'oh.

Lgtm
On Dec 13, 2010 1:17 AM, "Nikita Kostylev" <nkostylev@chromium.org> wrote:
> On Mon, Dec 13, 2010 at 11:00 AM, <cmasone@chromium.org> wrote:
>>
>> Sync is working again, yes? Can this be tested and landed now?
>>
>
> Yes, I'll test it today.
>
> Still need your LGTM though :)
>
> --
> Nikita

[Nikita (slow), 2010-12-13 15:28:23]

On 2010/12/13 09:17:41, Nikita Kostylev wrote:
> Yes, I'll test it today.

Tested, fix works on TOT. Will land after trybots run.
There's a side issue though. If user has one of the tabs that opens at the
startup which requires proper cookie, it'll use stale version.

After correct new password is entered, new cookie/tokens are fetched and opening
apps such as Gmail doesn't require additional auth input.

[Chris Masone, 2010-12-13 15:41:12]

On 2010/12/13 15:28:23, Nikita Kostylev wrote:
> On 2010/12/13 09:17:41, Nikita Kostylev wrote:
> > Yes, I'll test it today.
> 
> Tested, fix works on TOT. Will land after trybots run.
> There's a side issue though. If user has one of the tabs that opens at the
> startup which requires proper cookie, it'll use stale version.
> 
> After correct new password is entered, new cookie/tokens are fetched and
opening
> apps such as Gmail doesn't require additional auth input.

I"m ok with that