Fix ExhaustMemory on clang.

chrome/browser/metrics/metrics_service_browsertest.cc

Index: chrome/browser/metrics/metrics_service_browsertest.cc
diff --git a/chrome/browser/metrics/metrics_service_browsertest.cc b/chrome/browser/metrics/metrics_service_browsertest.cc
index 2167d5ae714c302f266eaacea391fed3d10a899b..2ed3b425ab5cd746a4beeb447dd62394989a0af2 100644
--- a/chrome/browser/metrics/metrics_service_browsertest.cc
+++ b/chrome/browser/metrics/metrics_service_browsertest.cc
@@ -12,6 +12,8 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/path_service.h"
+#include "base/process/memory.h"
+#include "base/test/histogram_tester.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/ui/browser.h"
@@ -28,6 +30,31 @@
 #include "ui/base/window_open_disposition.h"
 #include "url/gurl.h"
 
+#if defined(OS_POSIX)
+#include <sys/wait.h>
+#endif
+
+#if defined(OS_WIN)
+#include "sandbox/win/src/sandbox_types.h"
+#endif
+
+#if defined(OS_MACOSX) || defined(OS_LINUX)
+namespace {

[Ilya Sherman, 2017/02/07 16:50:26]

nit: Please leave a blank space after this line.

[Will Harris, 2017/02/07 17:45:20]

Done.

+// Check specified histogram for a single bucket entry and then verify that the
+// bucket entry contains a signal and the signal is |signal|.
+void verifyRendererExitCodeIsSignal(const base::HistogramTester& hist_tester,

[Ilya Sherman, 2017/02/07 16:50:26]

nit: Please spell out "histogram" rather than "hist" (applies throughout)

[Ilya Sherman, 2017/02/07 16:50:26]

nit: Please start this method with an Uppercase letter

[Will Harris, 2017/02/07 17:45:20]

Done.

[Will Harris, 2017/02/07 17:45:20]

Done. Not sure how that happened... too much go in my life.

+                                    int signal) {
+  const auto buckets = hist_tester.GetAllSamples("CrashExitCodes.Renderer");
+  EXPECT_EQ(1UL, buckets.size());
+  EXPECT_EQ(1, buckets[0].count);
+  int32_t exit_code = buckets[0].min;

[Ilya Sherman, 2017/02/07 16:50:26]

Can you use hist_tester.ExpectUniqueSample() in place of these four lines?  Or
is it harder to map from the signal to the expected exit code?

[Will Harris, 2017/02/07 17:45:20]

yup I can't use the existing APIs because I need to pull out the bucket and then
perform an operation on it (the WTERMSIG takes the low 4 bits). Whether there is
a core dump (0x80 bit) or not depends on system configuration at the time.

If I knew that the expected signal was always going to be SIGSEGV (11) or
SIGSEGV | WCOREFLAG (139) then I could use the normal functions, but I wanted to
keep this function flexible as it's likely once OOM gets added to Linux, then
the function will have to be called with a non-SIGSEGV parameter.

+  EXPECT_TRUE(WIFSIGNALED(exit_code));
+  EXPECT_EQ(signal, WTERMSIG(exit_code));
+}
+
+}  // namespace
+#endif  // OS_MACOSX || OS_LINUX
+
 class MetricsServiceBrowserTest : public InProcessBrowserTest {
  public:
   void SetUpCommandLine(base::CommandLine* command_line) override {
@@ -79,6 +106,7 @@ IN_PROC_BROWSER_TEST_F(MetricsServiceBrowserTest, CloseRenderersNormally) {
 #define MAYBE_CrashRenderers CrashRenderers
 #endif
 IN_PROC_BROWSER_TEST_F(MetricsServiceBrowserTest, MAYBE_CrashRenderers) {
+  base::HistogramTester hist_tester;
   OpenTabs();
 
   // Kill the process for one of the tabs.
@@ -107,5 +135,59 @@ IN_PROC_BROWSER_TEST_F(MetricsServiceBrowserTest, MAYBE_CrashRenderers) {
   // metrics::prefs::kStabilityExitedCleanly
   // is set to true, but this preference isn't set until the browser
   // exits... it's not clear to me how to test that.
+
+#if defined(OS_WIN)
+  hist_tester.ExpectUniqueSample(
+      "CrashExitCodes.Renderer",
+      std::abs(static_cast<int32_t>(STATUS_ACCESS_VIOLATION)), 1);
+#elif defined(OS_MACOSX) || defined(OS_LINUX)
+  verifyRendererExitCodeIsSignal(hist_tester, SIGSEGV);
+#endif
+  hist_tester.ExpectUniqueSample("Tabs.SadTab.CrashCreated", 1, 1);
 }
 
+// OOM code only works on Windows.
+#if defined(OS_WIN) && !defined(ADDRESS_SANITIZER)
+IN_PROC_BROWSER_TEST_F(MetricsServiceBrowserTest, OOMRenderers) {
+  base::HistogramTester hist_tester;
+  OpenTabs();
+
+  // Kill the process for one of the tabs.
+  content::RenderProcessHostWatcher observer(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  ui_test_utils::NavigateToURL(browser(),
+                               GURL(content::kChromeUIMemoryExhaustURL));
+  observer.Wait();
+
+  // The MetricsService listens for the same notification, so the |observer|
+  // might finish waiting before the MetricsService has a chance to process the
+  // notification.  To avoid racing here, we repeatedly run the message loop
+  // until the MetricsService catches up.  This should happen "real soon now",
+  // since the notification is posted to all observers essentially
+  // simultaneously... so busy waiting here shouldn't be too bad.
+  const PrefService* prefs = g_browser_process->local_state();
+  while (!prefs->GetInteger(metrics::prefs::kStabilityRendererCrashCount)) {
+    content::RunAllPendingInMessageLoop();
+  }
+
+  // Verify that the expected stability metrics were recorded.
+  EXPECT_EQ(1, prefs->GetInteger(metrics::prefs::kStabilityLaunchCount));
+  EXPECT_EQ(4, prefs->GetInteger(metrics::prefs::kStabilityPageLoadCount));

[Ilya Sherman, 2017/02/07 16:50:26]

nit: Mebbe document where this page load count is coming from?

[Will Harris, 2017/02/07 17:45:20]

Hmm this is copied from above :) tbh I'm not sure where 4 comes from I just
copy'n'paste, but I can try and work it out if you want.

+  EXPECT_EQ(1, prefs->GetInteger(metrics::prefs::kStabilityRendererCrashCount));
+
+// On 64-bit, the Job object should terminate the renderer on an OOM.
+#if defined(ARCH_CPU_64_BITS)
+  const int expected_exit_code = sandbox::SBOX_FATAL_MEMORY_EXCEEDED;
+#else
+  const int expected_exit_code = base::win::kOomExceptionCode;
+#endif
+
+  // Exit codes are recorded after being passed through std::abs see
+  // MapCrashExitCodeForHistogram.
+  hist_tester.ExpectUniqueSample("CrashExitCodes.Renderer",
+                                 std::abs(expected_exit_code), 1);
+
+  hist_tester.ExpectUniqueSample("Tabs.SadTab.OomCreated", 1, 1);
+}
+#endif  // OS_WIN && !ADDRESS_SANITIZER