AU: Retry up to 3 times 30 seconds apart on HTTP response code 0.

AU: Retry up to 3 times 30 seconds apart on HTTP response code 0.

This is a temporary way to avoid failing to do an update check on resume by
allowing at least 1.5 minutes for the network to go online.

BUG=9705
TEST=tested on device through suspend/resume; unit tests

Change-Id: I291a1c31ce87c17d5dce0e30488d454d7690ddbc

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=a092955

[petkov, 2010-11-29 21:13:31]

made some changes -- make sure you review the latest patch

[petkov, 2010-11-29 21:19:03]

I'd also need a permission to integrate into the 128.B branch, I guess.

[adlr, 2010-11-29 21:23:28]

LGTM w/ 2 comment nits.

Thanks for your expedience!

http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h
File libcurl_http_fetcher.h (right):

http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h#newco...
libcurl_http_fetcher.h:194: // Number of resumes due to HTTP response code 0.
... due to no network (i.e., HTTP response code 0).

This comment helps make it clear that "no" in the var names is the word "no" and
not the abbreviation for "number."

http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc
File update_attempter.cc (right):

http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc#newcode176
update_attempter.cc:176: update_check_fetcher));
// passes ownership

[petkov, 2010-11-29 21:28:19]

Review comments addressed.

sumit, rtc: should I push and merge into 128.B?

http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h
File libcurl_http_fetcher.h (right):

http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h#newco...
libcurl_http_fetcher.h:194: // Number of resumes due to HTTP response code 0.
On 2010/11/29 21:23:28, adlr wrote:
> ... due to no network (i.e., HTTP response code 0).
> 
> This comment helps make it clear that "no" in the var names is the word "no"
and
> not the abbreviation for "number."

Done.

http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc
File update_attempter.cc (right):

http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc#newcode176
update_attempter.cc:176: update_check_fetcher));
On 2010/11/29 21:23:28, adlr wrote:
> // passes ownership

Done.

[adlr, 2010-11-29 21:32:29]

On Mon, Nov 29, 2010 at 1:28 PM, <petkov@chromium.org> wrote:

> Review comments addressed.
>
> sumit, rtc: should I push and merge into 128.B?


Personally I vote for yes, but we need testers to hammer on this. They
already need to hammer on it due to proxy support going in, tho, so I don't
think this changes drastically the amount of hammering needed.

-andrew


>
>
>
>
> http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h
> File libcurl_http_fetcher.h (right):
>
>
>
http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h#newco...
> libcurl_http_fetcher.h:194: // Number of resumes due to HTTP response
> code 0.
> On 2010/11/29 21:23:28, adlr wrote:
>
>> ... due to no network (i.e., HTTP response code 0).
>>
>
>  This comment helps make it clear that "no" in the var names is the
>>
> word "no" and
>
>> not the abbreviation for "number."
>>
>
> Done.
>
>
> http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc
> File update_attempter.cc (right):
>
>
>
http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc#newcode176
> update_attempter.cc:176: update_check_fetcher));
> On 2010/11/29 21:23:28, adlr wrote:
>
>> // passes ownership
>>
>
> Done.
>
>
> http://codereview.chromium.org/5260004/
>

[sumitg, 2010-11-29 21:38:53]

LGTM

One minor nit:
no_network_... --> num_network_... to be more clear


Sounds good to me to push this CL and merge onto 128.B.
Ryan, any concerns?

On Mon, Nov 29, 2010 at 1:28 PM, <petkov@chromium.org> wrote:

> Review comments addressed.
>
> sumit, rtc: should I push and merge into 128.B?
>
>
>
>
> http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h
> File libcurl_http_fetcher.h (right):
>
>
>
http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h#newco...
> libcurl_http_fetcher.h:194: // Number of resumes due to HTTP response
> code 0.
> On 2010/11/29 21:23:28, adlr wrote:
>
>> ... due to no network (i.e., HTTP response code 0).
>>
>
>  This comment helps make it clear that "no" in the var names is the
>>
> word "no" and
>
>> not the abbreviation for "number."
>>
>
> Done.
>
>
> http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc
> File update_attempter.cc (right):
>
>
>
http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc#newcode176
> update_attempter.cc:176: update_check_fetcher));
> On 2010/11/29 21:23:28, adlr wrote:
>
>> // passes ownership
>>
>
> Done.
>
>
> http://codereview.chromium.org/5260004/
>

[petkov, 2010-11-29 21:43:42]

On 2010/11/29 21:38:53, sumitg wrote:
> LGTM
> 
> One minor nit:
> no_network_... --> num_network_... to be more clear

no_network_... stands for No Network (as in there's no network). It's fine as
is, I think -- we should remove this code in R10.

So just waiting for an approval from Ryan then.

> 
> 
> Sounds good to me to push this CL and merge onto 128.B.
> Ryan, any concerns?
> 
> On Mon, Nov 29, 2010 at 1:28 PM, <mailto:petkov@chromium.org> wrote:
> 
> > Review comments addressed.
> >
> > sumit, rtc: should I push and merge into 128.B?
> >
> >
> >
> >
> > http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h
> > File libcurl_http_fetcher.h (right):
> >
> >
> >
>
http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h#newco...
> > libcurl_http_fetcher.h:194: // Number of resumes due to HTTP response
> > code 0.
> > On 2010/11/29 21:23:28, adlr wrote:
> >
> >> ... due to no network (i.e., HTTP response code 0).
> >>
> >
> >  This comment helps make it clear that "no" in the var names is the
> >>
> > word "no" and
> >
> >> not the abbreviation for "number."
> >>
> >
> > Done.
> >
> >
> > http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc
> > File update_attempter.cc (right):
> >
> >
> >
>
http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc#newcode176
> > update_attempter.cc:176: update_check_fetcher));
> > On 2010/11/29 21:23:28, adlr wrote:
> >
> >> // passes ownership
> >>
> >
> > Done.
> >
> >
> > http://codereview.chromium.org/5260004/
> >

[sumitg, 2010-11-29 21:51:58]

On Mon, Nov 29, 2010 at 1:43 PM, <petkov@chromium.org> wrote:

> On 2010/11/29 21:38:53, sumitg wrote:
>
>> LGTM
>>
>
>  One minor nit:
>> no_network_... --> num_network_... to be more clear
>>
>
> no_network_... stands for No Network (as in there's no network). It's fine
> as
> is, I think -- we should remove this code in R10.
>

sounds good.


>
> So just waiting for an approval from Ryan then.
>
>
>
>
>  Sounds good to me to push this CL and merge onto 128.B.
>> Ryan, any concerns?
>>
>
>  On Mon, Nov 29, 2010 at 1:28 PM, <mailto:petkov@chromium.org> wrote:
>>
>
>  > Review comments addressed.
>> >
>> > sumit, rtc: should I push and merge into 128.B?
>> >
>> >
>> >
>> >
>> > http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h
>> > File libcurl_http_fetcher.h (right):
>> >
>> >
>> >
>>
>
>
>
http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h#newco...
>
>> > libcurl_http_fetcher.h:194: // Number of resumes due to HTTP response
>> > code 0.
>> > On 2010/11/29 21:23:28, adlr wrote:
>> >
>> >> ... due to no network (i.e., HTTP response code 0).
>> >>
>> >
>> >  This comment helps make it clear that "no" in the var names is the
>> >>
>> > word "no" and
>> >
>> >> not the abbreviation for "number."
>> >>
>> >
>> > Done.
>> >
>> >
>> > http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc
>> > File update_attempter.cc (right):
>> >
>> >
>> >
>>
>
>
>
http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc#newcode176
>
>> > update_attempter.cc:176: update_check_fetcher));
>> > On 2010/11/29 21:23:28, adlr wrote:
>> >
>> >> // passes ownership
>> >>
>> >
>> > Done.
>> >
>> >
>> > http://codereview.chromium.org/5260004/
>> >
>>
>
>
>
> http://codereview.chromium.org/5260004/
>

[rtc, 2010-11-29 22:16:39]

I think it is worth pushing

On Mon, Nov 29, 2010 at 1:51 PM, Sumit Gwalani <sumitg@google.com> wrote:

>
>
> On Mon, Nov 29, 2010 at 1:43 PM, <petkov@chromium.org> wrote:
>
>> On 2010/11/29 21:38:53, sumitg wrote:
>>
>>> LGTM
>>>
>>
>>  One minor nit:
>>> no_network_... --> num_network_... to be more clear
>>>
>>
>> no_network_... stands for No Network (as in there's no network). It's fine
>> as
>> is, I think -- we should remove this code in R10.
>>
>
> sounds good.
>
>
>>
>> So just waiting for an approval from Ryan then.
>>
>>
>>
>>
>>  Sounds good to me to push this CL and merge onto 128.B.
>>> Ryan, any concerns?
>>>
>>
>>  On Mon, Nov 29, 2010 at 1:28 PM, <mailto:petkov@chromium.org> wrote:
>>>
>>
>>  > Review comments addressed.
>>> >
>>> > sumit, rtc: should I push and merge into 128.B?
>>> >
>>> >
>>> >
>>> >
>>> >
>>> http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h
>>> > File libcurl_http_fetcher.h (right):
>>> >
>>> >
>>> >
>>>
>>
>>
>>
http://codereview.chromium.org/5260004/diff/3001/libcurl_http_fetcher.h#newco...
>>
>>> > libcurl_http_fetcher.h:194: // Number of resumes due to HTTP response
>>> > code 0.
>>> > On 2010/11/29 21:23:28, adlr wrote:
>>> >
>>> >> ... due to no network (i.e., HTTP response code 0).
>>> >>
>>> >
>>> >  This comment helps make it clear that "no" in the var names is the
>>> >>
>>> > word "no" and
>>> >
>>> >> not the abbreviation for "number."
>>> >>
>>> >
>>> > Done.
>>> >
>>> >
>>> > http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc
>>> > File update_attempter.cc (right):
>>> >
>>> >
>>> >
>>>
>>
>>
>>
http://codereview.chromium.org/5260004/diff/3001/update_attempter.cc#newcode176
>>
>>> > update_attempter.cc:176: update_check_fetcher));
>>> > On 2010/11/29 21:23:28, adlr wrote:
>>> >
>>> >> // passes ownership
>>> >>
>>> >
>>> > Done.
>>> >
>>> >
>>> > http://codereview.chromium.org/5260004/
>>> >
>>>
>>
>>
>>
>> http://codereview.chromium.org/5260004/
>>
>
>

[petkov, 2010-11-29 22:18:21]

Thanks, pushing and merging to 128.B then.