Deflake the SubprocessMetricsProviderBrowserTest
There are two issues:
It seemed that triggering render process crash isn't reliable,
RenderProcessExit sometime wasn't sent on navigation return, as
the render process crash isn't critical case we want to test,
this patch uses the simulation to avoid the flaky.
The specific histogram the test checked wasn't always written after
the navigation, this patch picked a random histogram from render
process allocator and verifies if it was copied to the global
allocator.
Bug: 1102807,1102852
Change-Id: If1456b3fc87b0255c549e12350074e318737dcf8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2285319
Commit-Queue: Tao Bai <michaelbai@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#786877}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: d1006d308caa55dba3dbe99d86f5f84494540f25
diff --git a/content/DEPS b/content/DEPS
index 98dfb0f..9c560c6 100644
--- a/content/DEPS
+++ b/content/DEPS
@@ -5,7 +5,6 @@
specific_include_rules = {
"subprocess_metrics_provider_browsertest\.cc": [
- "+content/public/common/url_constants.h",
"+content/shell/browser/shell.h",
"+net/dns/mock_host_resolver.h",
]
diff --git a/content/subprocess_metrics_provider_browsertest.cc b/content/subprocess_metrics_provider_browsertest.cc
index e27cf02..c8f315b 100644
--- a/content/subprocess_metrics_provider_browsertest.cc
+++ b/content/subprocess_metrics_provider_browsertest.cc
@@ -10,7 +10,6 @@
#include "base/metrics/persistent_histogram_allocator.h"
#include "base/metrics/persistent_memory_allocator.h"
#include "content/public/browser/child_process_termination_info.h"
-#include "content/public/common/url_constants.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
@@ -21,21 +20,27 @@
namespace {
-bool HasTypicalRenderProcessMetrics(
+// Returns a random histogram in Allocator. It hard to know which metric will
+// not be deprecated and exist after navigation. this will work because we
+// always pick up a histogram from render process and verify if it is copied to
+// the global allocator.
+std::string GetAnyHistogramInAllocator(
base::PersistentHistogramAllocator* allocator) {
base::PersistentHistogramAllocator::Iterator iter(allocator);
- bool found = false;
- static const std::string kTypicalRenderHistogram(
- "Blink.MainFrame.UpdateTime");
- while (true) {
- std::unique_ptr<base::HistogramBase> histogram = iter.GetNext();
- if (!histogram)
- break;
- found = (kTypicalRenderHistogram == histogram->histogram_name());
- if (found)
- break;
+ std::string histogram_name;
+ if (std::unique_ptr<base::HistogramBase> histogram = iter.GetNext())
+ histogram_name = histogram->histogram_name();
+ return histogram_name;
+}
+
+bool HasHistogramInAllocator(base::PersistentHistogramAllocator* allocator,
+ const std::string& histogram_name) {
+ base::PersistentHistogramAllocator::Iterator iter(allocator);
+ while (std::unique_ptr<base::HistogramBase> histogram = iter.GetNext()) {
+ if (histogram_name == histogram->histogram_name())
+ return true;
}
- return found;
+ return false;
}
size_t GetRenderProcessHostCount() {
@@ -123,14 +128,18 @@
auto* main_frame_allocator = GetMainFrameAllocator();
EXPECT_TRUE(main_frame_allocator);
+ // Verify the render process's allocator have the render process metrics.
+ std::string render_process_histogram =
+ GetAnyHistogramInAllocator(main_frame_allocator);
+ EXPECT_FALSE(render_process_histogram.empty());
+
// Verify the global histogram allocator have no render process metrics.
base::GlobalHistogramAllocator* global_histogram_allocator =
base::GlobalHistogramAllocator::Get();
ASSERT_TRUE(global_histogram_allocator);
- EXPECT_FALSE(HasTypicalRenderProcessMetrics(global_histogram_allocator));
-
- // Verify the render process's allocator have the render process metrics.
- EXPECT_TRUE(HasTypicalRenderProcessMetrics(main_frame_allocator));
+ EXPECT_FALSE(HasHistogramInAllocator(global_histogram_allocator,
+ render_process_histogram))
+ << " The histogram in the context is " << render_process_histogram;
SimulateRenderProcessExit();
@@ -139,7 +148,9 @@
// Verify the render process metrics were merged to the global histogram
// allocator.
- EXPECT_TRUE(HasTypicalRenderProcessMetrics(global_histogram_allocator));
+ EXPECT_TRUE(HasHistogramInAllocator(global_histogram_allocator,
+ render_process_histogram))
+ << " The histogram in the context is " << render_process_histogram;
auto* main_frame_process_host =
shell()->web_contents()->GetMainFrame()->GetProcess();
@@ -160,28 +171,38 @@
// RenderProcessHost and the main frame allocator exists.
EXPECT_EQ(get_scoped_observer().GetSourcesCount(),
GetRenderProcessHostCount());
- EXPECT_TRUE(GetMainFrameAllocator());
+ auto* main_frame_allocator = GetMainFrameAllocator();
+ EXPECT_TRUE(main_frame_allocator);
+
+ // Verify the render process's allocator have the render process metrics.
+ std::string render_process_histogram =
+ GetAnyHistogramInAllocator(main_frame_allocator);
+ EXPECT_FALSE(render_process_histogram.empty());
// Verify the global histogram allocator have no render process metrics.
base::GlobalHistogramAllocator* global_histogram_allocator =
base::GlobalHistogramAllocator::Get();
ASSERT_TRUE(global_histogram_allocator);
- EXPECT_FALSE(HasTypicalRenderProcessMetrics(global_histogram_allocator));
+ EXPECT_FALSE(HasHistogramInAllocator(global_histogram_allocator,
+ render_process_histogram))
+ << " The histogram in the context is " << render_process_histogram;
- // Verify the render process's allocator have the render process metrics.
- EXPECT_TRUE(HasTypicalRenderProcessMetrics(GetMainFrameAllocator()));
-
- // Crash the render process.
- content::ScopedAllowRendererCrashes scoped_allow_renderer_crashes(shell());
- NavigateToURLBlockUntilNavigationsComplete(
- shell(), GURL(content::kChromeUICrashURL), 1);
-
- // Verify the render process metrics were merged to the global histogram
- // allocator.
- EXPECT_TRUE(HasTypicalRenderProcessMetrics(global_histogram_allocator));
+ SimulateRenderProcessExit();
// Verify the allocator deregistered.
EXPECT_FALSE(GetMainFrameAllocator());
+
+ // Verify the render process metrics were merged to the global histogram
+ // allocator.
+ EXPECT_TRUE(HasHistogramInAllocator(global_histogram_allocator,
+ render_process_histogram))
+ << " The histogram in the context is " << render_process_histogram;
+
+ auto* main_frame_process_host =
+ shell()->web_contents()->GetMainFrame()->GetProcess();
+ SimulateRenderProcessHostDestroyed();
+ // Verify the observer removed.
+ EXPECT_FALSE(get_scoped_observer().IsObserving(main_frame_process_host));
}
} // namespace metrics
