Add histogram to measure string response size of URLFetcher.

Add histogram to measure string response size of URLFetcher.

BUG=101929
TEST=URLFetcher*Test.*


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

[jam, 2011-10-27 19:03:22]

I think as an API it's confusing that there's a function that is only called
once. why don't we histogram it first, and then track down anyone who's
incorrectly using this with large data?

[skerner, 2011-10-27 20:29:45]

On Thu, Oct 27, 2011 at 3:03 PM,  <jam@chromium.org> wrote:
> I think as an API it's confusing that there's a function that is only called
> once. why don't we histogram it first, and then track down anyone who's
> incorrectly using this with large data?
>
> http://codereview.chromium.org/8416001/
>

The copy bugs me because I expect that a histogram won't reveal a bad
actor quickly.  Suppose a histogram shows the we occasionally fetch a
10-15MB response.  We have no way to see what code did the fetch.
Copying that large response while we ask all URLFetcher users if it
might be their fault adds insult to injury.

Would you feel differently if I rename the method TakeResponseString()?

Can anyone think of a clever way to identify the code that created the
URLFetcher?  Short of adding FROM_HERE as an argument, I see no sane
way to identify the guilty code in the histogram.

[jam, 2011-10-27 20:37:22]

On Thu, Oct 27, 2011 at 1:29 PM, Sam Kerner <skerner@google.com> wrote:

> On Thu, Oct 27, 2011 at 3:03 PM,  <jam@chromium.org> wrote:
> > I think as an API it's confusing that there's a function that is only
> called
> > once. why don't we histogram it first, and then track down anyone who's
> > incorrectly using this with large data?
> >
> > http://codereview.chromium.org/8416001/
> >
>
> The copy bugs me because I expect that a histogram won't reveal a bad
> actor quickly.  Suppose a histogram shows the we occasionally fetch a
> 10-15MB response.  We have no way to see what code did the fetch.
> Copying that large response while we ask all URLFetcher users if it
> might be their fault adds insult to injury.
>
> Would you feel differently if I rename the method TakeResponseString()?
>

no the issue I have isn't with naming


>
> Can anyone think of a clever way to identify the code that created the
> URLFetcher?  Short of adding FROM_HERE as an argument, I see no sane
> way to identify the guilty code in the histogram.
>

first there's a premise that people are using it for MB size response. it
would be good to confirm that it happening, and we can get that through a
histogram.

if it turns out that that's the case, then we can audit the code. most
places that get a text response from a google server etc wouldn't get that
much data, so we can focus on places that get binary data. perhaps some of
the chromeos usage of downloading images might be big, but I doubt XX MB or
even X MB.

[Sam Kerner (Chrome), 2011-10-27 20:48:12]

On 2011/10/27 20:37:22, John Abd-El-Malek wrote:
> On Thu, Oct 27, 2011 at 1:29 PM, Sam Kerner <mailto:skerner@google.com> wrote:
> 
> > On Thu, Oct 27, 2011 at 3:03 PM,  <mailto:jam@chromium.org> wrote:
> > > I think as an API it's confusing that there's a function that is only
> > called
> > > once. why don't we histogram it first, and then track down anyone who's
> > > incorrectly using this with large data?
> > >
> > > http://codereview.chromium.org/8416001/
> > >
> >
> > The copy bugs me because I expect that a histogram won't reveal a bad
> > actor quickly.  Suppose a histogram shows the we occasionally fetch a
> > 10-15MB response.  We have no way to see what code did the fetch.
> > Copying that large response while we ask all URLFetcher users if it
> > might be their fault adds insult to injury.
> >
> > Would you feel differently if I rename the method TakeResponseString()?
> >
> 
> no the issue I have isn't with naming
> 
> 
> >
> > Can anyone think of a clever way to identify the code that created the
> > URLFetcher?  Short of adding FROM_HERE as an argument, I see no sane
> > way to identify the guilty code in the histogram.
> >
> 
> first there's a premise that people are using it for MB size response. it
> would be good to confirm that it happening, and we can get that through a
> histogram.
> 
> if it turns out that that's the case, then we can audit the code. most
> places that get a text response from a google server etc wouldn't get that
> much data, so we can focus on places that get binary data. perhaps some of
> the chromeos usage of downloading images might be big, but I doubt XX MB or
> even X MB.

Okay.  Changed the issue to just add a histogram.

[jam, 2011-10-27 20:49:59]

lgtm

[commit-bot: I haz the power, 2011-10-28 19:08:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skerner@chromium.org/8416001/1006

[commit-bot: I haz the power, 2011-10-28 20:10:41]

Change committed as 107780