Match Pepper UMA histograms by prefix

Match Pepper UMA histograms by prefix

Previously each individual histogram name had to be hashed and whitelisted.
This had the drawback of older versions of Chrome not knowing about newer
histogram names which were added, and then would be discarded.  Now we match
against known prefixes to allow a broader range of future histogram names.

BUG=317833
R=bbudge@chromium.org, isherman@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255487

[elijahtaylor1, 2014-03-06 00:19:34]

PTAL

isherman: as discussed offline
bbudge: pepper host OWNERS

[Ilya Sherman, 2014-03-06 00:34:14]

https://codereview.chromium.org/187593009/diff/1/chrome/renderer/pepper/peppe...
File chrome/renderer/pepper/pepper_uma_host.cc (right):

https://codereview.chromium.org/187593009/diff/1/chrome/renderer/pepper/peppe...
chrome/renderer/pepper/pepper_uma_host.cc:29: const int
kWhitelistedHistogramPrefixLength = 12;
Hmm, maybe scan up to the first dot instead?

https://codereview.chromium.org/187593009/diff/1/chrome/renderer/pepper/peppe...
chrome/renderer/pepper/pepper_uma_host.cc:31: const char*
kWhitelistedHistogramPrefixes[] = {
nit: "const char* const"

https://codereview.chromium.org/187593009/diff/1/chrome/renderer/pepper/peppe...
chrome/renderer/pepper/pepper_uma_host.cc:32:
"CD190EA2B764EDF0BB97552A638D32072F3CFD41",  // see crbug.com/317833
nit: Links should start with "http://".  Also, this is a restricted bug, which
might frustrate some folks trying to understand the code... but maybe that's the
best we can do in this case?

[elijahtaylor1, 2014-03-06 00:46:26]

https://codereview.chromium.org/187593009/diff/1/chrome/renderer/pepper/peppe...
File chrome/renderer/pepper/pepper_uma_host.cc (right):

https://codereview.chromium.org/187593009/diff/1/chrome/renderer/pepper/peppe...
chrome/renderer/pepper/pepper_uma_host.cc:29: const int
kWhitelistedHistogramPrefixLength = 12;
On 2014/03/06 00:34:14, Ilya Sherman wrote:
> Hmm, maybe scan up to the first dot instead?

Great suggestion.  Done.

https://codereview.chromium.org/187593009/diff/1/chrome/renderer/pepper/peppe...
chrome/renderer/pepper/pepper_uma_host.cc:31: const char*
kWhitelistedHistogramPrefixes[] = {
On 2014/03/06 00:34:14, Ilya Sherman wrote:
> nit: "const char* const"

Done.

https://codereview.chromium.org/187593009/diff/1/chrome/renderer/pepper/peppe...
chrome/renderer/pepper/pepper_uma_host.cc:32:
"CD190EA2B764EDF0BB97552A638D32072F3CFD41",  // see crbug.com/317833
On 2014/03/06 00:34:14, Ilya Sherman wrote:
> nit: Links should start with "http://".  Also, this is a restricted bug, which
> might frustrate some folks trying to understand the code... but maybe that's
the
> best we can do in this case?

Fixed "http://"

Yeah, I agree that could be frustrating, but the alternative right now is no bug
link to explain it, which would frustrate all folks.

[Ilya Sherman, 2014-03-06 00:47:38]

https://codereview.chromium.org/187593009/diff/20001/chrome/renderer/pepper/p...
File chrome/renderer/pepper/pepper_uma_host.cc (right):

https://codereview.chromium.org/187593009/diff/20001/chrome/renderer/pepper/p...
chrome/renderer/pepper/pepper_uma_host.cc:35: histogram.substr(0,
histogram.find('.')));
Did you verify that this does the right thing if the histogram name happens to
not contain a dot?

[elijahtaylor1, 2014-03-06 00:52:33]

On 2014/03/06 00:47:38, Ilya Sherman wrote:
>
https://codereview.chromium.org/187593009/diff/20001/chrome/renderer/pepper/p...
> File chrome/renderer/pepper/pepper_uma_host.cc (right):
> 
>
https://codereview.chromium.org/187593009/diff/20001/chrome/renderer/pepper/p...
> chrome/renderer/pepper/pepper_uma_host.cc:35: histogram.substr(0,
> histogram.find('.')));
> Did you verify that this does the right thing if the histogram name happens to
> not contain a dot?

Yes, find returns npos if the character is not found, and substr's default args
are 0,npos  meaning the whole string

[Ilya Sherman, 2014-03-06 00:52:59]

Cool.  LGTM, thanks.

[bbudge, 2014-03-06 01:15:20]

lgtm

[elijahtaylor1, 2014-03-06 16:10:32]

The CQ bit was checked by elijahtaylor@chromium.org

[commit-bot: I haz the power, 2014-03-06 16:12:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/elijahtaylor@chromium.org/187593009/20001

[commit-bot: I haz the power, 2014-03-06 16:15:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 16:15:37]

Try jobs failed on following builders: mac_chromium_rel

[elijahtaylor1, 2014-03-06 16:26:03]

The CQ bit was checked by elijahtaylor@chromium.org

[commit-bot: I haz the power, 2014-03-06 16:26:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/elijahtaylor@chromium.org/187593009/20001

[commit-bot: I haz the power, 2014-03-06 19:56:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 19:56:41]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[elijahtaylor1, 2014-03-06 21:07:20]

The CQ bit was checked by elijahtaylor@chromium.org

[commit-bot: I haz the power, 2014-03-06 21:17:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/elijahtaylor@chromium.org/187593009/20001

[commit-bot: I haz the power, 2014-03-06 22:04:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/elijahtaylor@chromium.org/187593009/20001

[commit-bot: I haz the power, 2014-03-07 01:09:23]

Change committed as 255487