Record UMA status on websql results (chrome)

Record UMA status on websql results.
This is the chrome-side of https://bugs.webkit.org/show_bug.cgi?id=73258

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

[michaeln, 2011-11-29 22:45:07]

ptal

[ramant (doing other things), 2011-11-29 23:29:35]

http://codereview.chromium.org/8590043/diff/11001/content/common/web_database...
File content/common/web_database_observer_impl.cc (right):

http://codereview.chromium.org/8590043/diff/11001/content/common/web_database...
content/common/web_database_observer_impl.cc:77: DCHECK(callsite <
kCallsiteHistogramSize);
nit:

could we define a routine that takes the histrogram names as arguments and could
the following code be made smaller?

something like 
RecordHistogramData(
    database,
    callsite, 
    webSqlErrorCode,
    sqliteErrorCode 
    histogram_name_sync_prefix, 
    histogram_name_async_prefix)

In reportChangeVersionResult
we call RecordHistogramData(database, callsite, webSqlErrorCode,
sqliteErrorCode,  "websql.Sync.ChangeVersionResult",
"websql.Async.ChangeVersionResult")
   
In reportStartTransactionResult
we call RecordHistogramData(database, callsite, webSqlErrorCode,
sqliteErrorCode,  "websql.Sync.BeginResult", "websql.Async.BeginResult")

thanks

[michaeln, 2011-11-29 23:50:13]

http://codereview.chromium.org/8590043/diff/11001/content/common/web_database...
File content/common/web_database_observer_impl.cc (right):

http://codereview.chromium.org/8590043/diff/11001/content/common/web_database...
content/common/web_database_observer_impl.cc:77: DCHECK(callsite <
kCallsiteHistogramSize);
On 2011/11/29 23:29:36, ramant wrote:
> nit:
> 
> could we define a routine that takes the histrogram names as arguments and
could
> the following code be made smaller?

I believe the histogram names can't vary for a particular macro usage...

    // Ensure calling contract is upheld, and the name does NOT vary.
    DCHECK(histogram_pointer->histogram_name() == constant_histogram_name);

... is that not right?

If that's right, i don't think i can do quite what you suggest (call a common
helper function). I could come up with a template driven means of generating a
macro per name if you think its worth it. After compilation of the expanded
templates, the binary code size would be pretty much the same, but there could
be less source code.

[michaeln, 2011-11-29 23:52:27]

I guess a custom macro could work here too.

> I could come up with a template driven means of generating a
> macro per name if you think its worth it.

[ramant (doing other things), 2011-11-30 00:13:21]

LGTM.

We need to do something like LogUMAHistogramEnumeration in
/chrome/browser/autofill/autofill_metrics.cc (or define a macro as it was done
in disk_cache). It may not be worth the effort (would like to leave it to you).

thanks,
raman

http://codereview.chromium.org/8590043/diff/11001/content/common/web_database...
File content/common/web_database_observer_impl.cc (right):

http://codereview.chromium.org/8590043/diff/11001/content/common/web_database...
content/common/web_database_observer_impl.cc:77: DCHECK(callsite <
kCallsiteHistogramSize);
On 2011/11/29 23:50:13, michaeln wrote:
> On 2011/11/29 23:29:36, ramant wrote:
> > nit:
> > 
> > could we define a routine that takes the histrogram names as arguments and
> could
> > the following code be made smaller?
> 
> I believe the histogram names can't vary for a particular macro usage...
> 
>     // Ensure calling contract is upheld, and the name does NOT vary.
>     DCHECK(histogram_pointer->histogram_name() == constant_histogram_name);
> 
> ... is that not right?
> 
> If that's right, i don't think i can do quite what you suggest (call a common
> helper function). I could come up with a template driven means of generating a
> macro per name if you think its worth it. After compilation of the expanded
> templates, the binary code size would be pretty much the same, but there could
> be less source code.
> 
> 

We have to define a macro. It is your call.

[michaeln, 2011-11-30 01:49:31]

john, sorry to bug you... but i need a content owner

[jam, 2011-11-30 18:53:34]

lgtm