Fixed a bug with URLFetcher where retrying the request would append the new r...

Fixed a bug with URLFetcher where retrying the request would append the new response data to the old one.

BUG=None.
TEST=Unit-tests


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

[eroman, 2010-12-01 01:34:40]

lgtm

http://codereview.chromium.org/5322015/diff/1/chrome/common/net/url_fetcher_u...
File chrome/common/net/url_fetcher_unittest.cc (right):

http://codereview.chromium.org/5322015/diff/1/chrome/common/net/url_fetcher_u...
chrome/common/net/url_fetcher_unittest.cc:242: std::string data_;
nit: looks like there is one space too many here.

http://codereview.chromium.org/5322015/diff/1/chrome/common/net/url_fetcher_u...
chrome/common/net/url_fetcher_unittest.cc:477: fetcher_->Start();
It is not clear from the docs whether calling Start() multiple times is valid. I
understand why you do this for the unit-test, but would be better if you could
cause it to internally retry instead.

I am OK with leaving as is if you can't find a clean way to do that though.

[eroman, 2010-12-01 01:34:41]

lgtm

[sanjeevr, 2010-12-01 02:10:00]

http://codereview.chromium.org/5322015/diff/1/chrome/common/net/url_fetcher_u...
File chrome/common/net/url_fetcher_unittest.cc (right):

http://codereview.chromium.org/5322015/diff/1/chrome/common/net/url_fetcher_u...
chrome/common/net/url_fetcher_unittest.cc:242: std::string data_;
On 2010/12/01 01:34:40, eroman wrote:
> nit: looks like there is one space too many here.

Done.

http://codereview.chromium.org/5322015/diff/1/chrome/common/net/url_fetcher_u...
chrome/common/net/url_fetcher_unittest.cc:477: fetcher_->Start();
On 2010/12/01 01:34:40, eroman wrote:
> It is not clear from the docs whether calling Start() multiple times is valid.
I
> understand why you do this for the unit-test, but would be better if you could
> cause it to internally retry instead.
> 
> I am OK with leaving as is if you can't find a clean way to do that though.

Other tests also use fetcher_->Start to retry. Besides, I use this in production
code too. Perhaps update the docs? Do you think I should change my production
code to not use Start() to retry?

[eroman, 2010-12-01 02:13:38]

http://codereview.chromium.org/5322015/diff/1/chrome/common/net/url_fetcher_u...
File chrome/common/net/url_fetcher_unittest.cc (right):

http://codereview.chromium.org/5322015/diff/1/chrome/common/net/url_fetcher_u...
chrome/common/net/url_fetcher_unittest.cc:477: fetcher_->Start();
On 2010/12/01 02:10:01, sanjeevr wrote:
> On 2010/12/01 01:34:40, eroman wrote:
> > It is not clear from the docs whether calling Start() multiple times is
valid.
> I
> > understand why you do this for the unit-test, but would be better if you
could
> > cause it to internally retry instead.
> > 
> > I am OK with leaving as is if you can't find a clean way to do that though.
> 
> Other tests also use fetcher_->Start to retry. Besides, I use this in
production
> code too. Perhaps update the docs? Do you think I should change my production
> code to not use Start() to retry?

ah ok, didn't realize it was a common pattern.