add UMA for no. of pages with no distilled data

add UMA to record if a page has distilled data.

BUG=418190
Committed: https://crrev.com/69231d048672856333da7d15ef6ce9afa59e3d0d
Cr-Commit-Position: refs/heads/master@{#297100}

[kuan, 2014-09-26 19:27:57]

kuan@chromium.org changed reviewers:
+ nyquist@chromium.org

[nyquist, 2014-09-26 21:52:55]

https://codereview.chromium.org/607013003/diff/1/components/dom_distiller/cor...
File components/dom_distiller/core/viewer.cc (right):

https://codereview.chromium.org/607013003/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/viewer.cc:99:
UMA_HISTOGRAM_COUNTS("DomDistiller.Statistics.NoDistilledDataCount", 1);
It doesn't feel right to do this as a histogram to me, given the number is hard
coded. If you  do a histogram, how about counting 0 on the outside and 1 on the
inside? And if you do that, you might want to use a boolean instead? See
UMA_HISTOGRAM_BOOLEAN. I think that makes sense

If you want to do a count in general, you could do that too, but then you would
have to compare it to some other count. Actions are sadly defined in //content
though:
https://chromium.googlesource.com/chromium/src.git/+/master/content/public/br...

Is that why you chose histogram?

Anyway: I vote for boolean about empty and non-empty histogram. That should give
us all the numbers we need.

[kuan, 2014-09-26 22:46:55]

i've addressed all comments in patch set 2.  ptal.  thx.

https://codereview.chromium.org/607013003/diff/1/components/dom_distiller/cor...
File components/dom_distiller/core/viewer.cc (right):

https://codereview.chromium.org/607013003/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/viewer.cc:99:
UMA_HISTOGRAM_COUNTS("DomDistiller.Statistics.NoDistilledDataCount", 1);
On 2014/09/26 21:52:54, nyquist wrote:
> It doesn't feel right to do this as a histogram to me, given the number is
hard
> coded. If you  do a histogram, how about counting 0 on the outside and 1 on
the
> inside? And if you do that, you might want to use a boolean instead? See
> UMA_HISTOGRAM_BOOLEAN. I think that makes sense
> 
> If you want to do a count in general, you could do that too, but then you
would
> have to compare it to some other count. Actions are sadly defined in //content
> though:
>
https://chromium.googlesource.com/chromium/src.git/+/master/content/public/br...
> 
> Is that why you chose histogram?
> 
> Anyway: I vote for boolean about empty and non-empty histogram. That should
give
> us all the numbers we need.

Done.

[kuan, 2014-09-26 22:46:55]

i've addressed all comments in patch set 2.  ptal.  thx.

[nyquist, 2014-09-26 22:59:30]

lgtm

thanks!

[kuan, 2014-09-26 23:04:42]

kuan@chromium.org changed reviewers:
+ isherman@chromium.org

[kuan, 2014-09-26 23:04:43]

isherman: ptal tools/metrics/histograms/histograms.xml.  thx!

[Ilya Sherman, 2014-09-27 00:30:16]

LGTM

https://codereview.chromium.org/607013003/diff/20001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/607013003/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:5119: +  <summary>Records if a page has
distilled data.</summary>
nit: "if" -> "whether"

[kuan, 2014-09-27 00:33:39]

i've addressed the nit in patch set 3.  thx.

https://codereview.chromium.org/607013003/diff/20001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/607013003/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:5119: +  <summary>Records if a page has
distilled data.</summary>
On 2014/09/27 00:30:16, Ilya Sherman wrote:
> nit: "if" -> "whether"

Done.

[kuan, 2014-09-27 00:34:09]

The CQ bit was checked by kuan@chromium.org

[commit-bot: I haz the power, 2014-09-27 00:35:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/607013003/40001

[commit-bot: I haz the power, 2014-09-27 03:29:54]

Committed patchset #3 (id:40001) as 62062fd9921dbf87f9582d23b0f74e966ce56344

[commit-bot: I haz the power, 2014-09-27 03:30:32]

Patchset 3 (id:??) landed as
https://crrev.com/69231d048672856333da7d15ef6ce9afa59e3d0d
Cr-Commit-Position: refs/heads/master@{#297100}