Rework UMAHistogramHelper and StatisticsDeltaReader into [Chrome]HistogramTester.

chrome/test/base/chrome_histogram_tester.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/test/base/chrome_histogram_tester.h"
+
+#include "base/bind.h"
+#include "base/test/test_timeouts.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/histogram_fetcher.h"
+
+ChromeHistogramTester::ChromeHistogramTester() {
+}
+
+ChromeHistogramTester::~ChromeHistogramTester() {
+}
+
+void ChromeHistogramTester::FetchTestingSnapshot() {
+  scoped_refptr<content::MessageLoopRunner> runner =
+      new content::MessageLoopRunner;
+
+  content::FetchHistogramsAsynchronously(
+      base::MessageLoop::current(),
+      runner->QuitClosure(),
+      // If this call times out, it means that a child process is not
+      // responding, which is something we should not ignore.  The timeout is
+      // set to be longer than the normal browser test timeout so that it will
+      // be prempted by the normal timeout.
+      TestTimeouts::action_max_timeout() * 2);
+  runner->Run();
+}

[Ilya Sherman, 2014/07/16 20:01:12]

It's interesting to note that this method does not interact with the base class
in any way.  Perhaps rather than providing a ChromeHistogramTester class, we
should just provide a free function FetchHistogramsFromChildProcesses(), which
we define in e.g. //content/public/test/browser_test_utils.h?

[Mike Lerman, 2014/07/17 15:04:18]

I was hoping to have something I could add under //content rather than //chrome,
since that's the where the dependency actually is. I like this suggestion. Done.

My big concern now is usability/find-ability. Someone's going to write a
browser_test, use the HistogramTester, get inconsistent or flaky results and
will have a hard time figuring out how to fix it. A subclass like
ChromeHistogramTester might at least come up in someone's search. I think I may
need to add a comment to //base/test/histogram_tester.h for future usability,
although I know it breaks the practice of referencing files further up the
dependency tree. WDYT?

[Ilya Sherman, 2014/07/17 18:35:09]

I don't think the comment is needed, for two reasons:  First of all, collecting
histograms from other processes is a very rare need in tests.  Secondly, if the
test author doesn't call this code, their test won't be flaky -- it just won't
work.  There's no race condition, as there's no code that periodically
synchronizes histograms across processes in tests.  It's possible that somebody
would reimplement this method, and get it wrong, but I'm not too worried about
that possibility.