Add test for accessibility histograms.

Add test for accessibility histograms.

The goal is just to make sure this code has test coverage, not to test
correctness. There was an issue in the past where the code that computed
the histograms crashed in debug mode, this will help prevent something like
that from landing in the future.

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

[Jói, 2013-01-08 20:44:59]

+jam

I think the right approach here might be to move the UMAHistogramHelper test
helper class to content/test and make this test be part of the
content_browsertest target rather than the browser_test target.

Regardless, you should name the exposed-only-for-testing function
UpdateHistogramsForTesting and leave the UpdateHistograms implementation
function private, so that our presubmit check enforces that the function exposed
is used only by test code.

https://codereview.chromium.org/11778045/diff/1/content/public/browser/browse...
File content/public/browser/browser_accessibility_state.h (right):

https://codereview.chromium.org/11778045/diff/1/content/public/browser/browse...
content/public/browser/browser_accessibility_state.h:39: virtual void
UpdateHistograms() = 0;
I was going to tell you to name it UpdateHistogramsForTesting, until I saw the
comment in browser_accessibility_state_impl.h, that it's _public_ only for
testing, otherwise would presumably be private.

I think the method exposed in the API (and publicly on
BrowserAccessibilityStateImpl) should be named UpdateHistogramsForTesting.  It
could turn around and call UpdateHistograms which would be private.

[dmazzoni, 2013-01-08 22:39:51]

On 2013/01/08 20:44:59, Jói wrote:
> +jam
> 
> I think the right approach here might be to move the UMAHistogramHelper test
> helper class to content/test and make this test be part of the
> content_browsertest target rather than the browser_test target.

I actually prefer to have this test in chrome because the chromeos histograms
will only run if it's part of a chrome browser test, not a content test. I added
a comment.

> I think the method exposed in the API (and publicly on
> BrowserAccessibilityStateImpl) should be named UpdateHistogramsForTesting.  It
> could turn around and call UpdateHistograms which would be private.

Good idea, done.

[Jói, 2013-01-08 23:14:14]

LGTM

https://codereview.chromium.org/11778045/diff/1006/content/browser/accessibil...
File content/browser/accessibility/browser_accessibility_state_impl.h (right):

https://codereview.chromium.org/11778045/diff/1006/content/browser/accessibil...
content/browser/accessibility/browser_accessibility_state_impl.h:59: // This
method is public only for testing!
You can remove this line now.

https://codereview.chromium.org/11778045/diff/1006/content/public/browser/bro...
File content/public/browser/browser_accessibility_state.h (right):

https://codereview.chromium.org/11778045/diff/1006/content/public/browser/bro...
content/public/browser/browser_accessibility_state.h:38: // This method is
public only for testing!
The comment is redundant now, given the naming.

[dmazzoni, 2013-01-08 23:32:33]

https://codereview.chromium.org/11778045/diff/1006/content/browser/accessibil...
File content/browser/accessibility/browser_accessibility_state_impl.h (right):

https://codereview.chromium.org/11778045/diff/1006/content/browser/accessibil...
content/browser/accessibility/browser_accessibility_state_impl.h:59: // This
method is public only for testing!
On 2013/01/08 23:14:14, Jói wrote:
> You can remove this line now.

Done.

https://codereview.chromium.org/11778045/diff/1006/content/public/browser/bro...
File content/public/browser/browser_accessibility_state.h (right):

https://codereview.chromium.org/11778045/diff/1006/content/public/browser/bro...
content/public/browser/browser_accessibility_state.h:38: // This method is
public only for testing!
On 2013/01/08 23:14:14, Jói wrote:
> The comment is redundant now, given the naming.

Done.

[commit-bot: I haz the power, 2013-01-09 00:04:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/11778045/8001

[commit-bot: I haz the power, 2013-01-09 02:33:30]

Change committed as 175677