[Sampling profiler] Trial re-enable on ARM32 Android canary render thread

Re-enables the profiler at a low rate on the render thread in
Android ARM32 canary, to test the hypothesis that the crashes seen
in https://crbug.com/1135152 were due to the module loading occurring
after sandbox initialization.

The re-enabling moves ~50ms of work onto the main thread so is intended
to be landed only temporarily.

Bug: 1135152, 1004855
Change-Id: I66f71aa391c5f3369c77ef8ca1d7866221d9c66c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2453795Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814399}

chrome/common/profiler/thread_profiler.cc

@@ -31,9 +31,6 @@
 #include "sandbox/policy/sandbox.h"
 
 #if defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE)
-#include <sys/types.h>
-#include <unistd.h>
-
 #include "base/android/apk_assets.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/profiler/arm_cfi_table.h"
@@ -129,17 +126,7 @@ class NativeUnwinderCreator {
   const std::unique_ptr<stack_unwinder::MemoryRegionsMap> memory_regions_map_;
 };
 
-// This function must only be called via the closure created in
-// CreateCoreUnwindersFactory(), which is passed into StackSamplingProfiler.
-// This ensures the expensive work involved in constructing the unwinder
-// creators is done on the profiler thread rather than the profiled thread.
-// These take 50+ ms to execute due to the creation of the memory regions map
-// and lookup of the modules in the process.
 std::vector<std::unique_ptr<base::Unwinder>> CreateCoreUnwinders() {
-  // This function is expected to be run on the profiler thread which is never
-  // the main thread in the process.
-  DCHECK_NE(getpid(), gettid());
-
   static base::NoDestructor<NativeUnwinderCreator> native_unwinder_creator;
   static base::NoDestructor<ChromeUnwinderCreator> chrome_unwinder_creator;
 
@@ -159,7 +146,15 @@ base::StackSamplingProfiler::UnwindersFactory CreateCoreUnwindersFactory() {
   CHECK(
       ThreadProfilerConfiguration::Get()->IsProfilerEnabledForCurrentProcess());
 
-  return base::BindOnce(&CreateCoreUnwinders);
+  // Temporarily run CreateCoreUnwinders() on the main thread to test a
+  // hypothesis about cause of crashes seen in https://crbug.com/1135152.
+  // TODO(https://crbug.com/1135152): Move CreateCoreUnwinders() execution back
+  // into the bound function.
+  return base::BindOnce(
+      [](std::vector<std::unique_ptr<base::Unwinder>> unwinders) {
+        return unwinders;
+      },
+      CreateCoreUnwinders());
 #else
   return base::StackSamplingProfiler::UnwindersFactory();
 #endif

chrome/common/profiler/thread_profiler_platform_configuration.cc

@@ -192,6 +192,11 @@ double AndroidPlatformConfiguration::GetChildProcessEnableFraction(
   if (browser_test_mode_enabled())
     return DefaultPlatformConfiguration::GetChildProcessEnableFraction(process);
 
+  // TODO(https://crbug.com/1135152): Increase enable rate once we've validated
+  // the hypothesis about crash cause, and mitigated the issue.
+  if (process == metrics::CallStackProfileParams::RENDERER_PROCESS)
+    return 0.006;
+
   // TODO(https://crbug.com/1004855): Enable for all the default processes.
   return 0.0;
 }
@@ -199,15 +204,23 @@ double AndroidPlatformConfiguration::GetChildProcessEnableFraction(
 bool AndroidPlatformConfiguration::IsEnabledForThread(
     metrics::CallStackProfileParams::Process process,
     metrics::CallStackProfileParams::Thread thread) const {
-  // Disable for all supported threads pending launch. Enable only for browser
-  // tests.
+  // Enable on renderer process main thread in production, for now.
+  if (process == metrics::CallStackProfileParams::RENDERER_PROCESS &&
+      thread == metrics::CallStackProfileParams::MAIN_THREAD) {
+    return true;
+  }
+
+  // Otherwise enable in dedicated ThreadProfiler browser tests.
   return browser_test_mode_enabled();
 }
 
 bool AndroidPlatformConfiguration::IsSupportedForChannel(
     base::Optional<version_info::Channel> release_channel) const {
-  // On Android profiling is only enabled in its own dedicated browser tests
-  // in local builds and the CQ.
+  // Enable on canary, for now.
+  if (release_channel && *release_channel == version_info::Channel::CANARY)
+    return true;
+
+  // Otherwise enable in dedicated ThreadProfiler browser tests.
   // TODO(https://crbug.com/1004855): Enable across all browser tests.
   return browser_test_mode_enabled();
 }

chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc

@@ -72,7 +72,7 @@ TEST_F(ThreadProfilerPlatformConfigurationTest, IsSupported) {
   EXPECT_FALSE(config()->IsSupported(base::nullopt));
 #elif defined(OS_ANDROID)
   EXPECT_FALSE(config()->IsSupported(version_info::Channel::UNKNOWN));
-  EXPECT_FALSE(config()->IsSupported(version_info::Channel::CANARY));
+  EXPECT_TRUE(config()->IsSupported(version_info::Channel::CANARY));
   EXPECT_FALSE(config()->IsSupported(version_info::Channel::DEV));
   EXPECT_FALSE(config()->IsSupported(version_info::Channel::BETA));
   EXPECT_FALSE(config()->IsSupported(version_info::Channel::STABLE));
@@ -126,14 +126,15 @@ MAYBE_PLATFORM_CONFIG_TEST_F(ThreadProfilerPlatformConfigurationTest,
 
 MAYBE_PLATFORM_CONFIG_TEST_F(ThreadProfilerPlatformConfigurationTest,
                              GetEnableRates) {
-  // Note: death tests aren't supported on Android. Otherwise this test would
-  // check that all inputs result in CHECKs.
-#if !defined(OS_ANDROID)
   using RelativePopulations =
       ThreadProfilerPlatformConfiguration::RelativePopulations;
-  EXPECT_CHECK_DEATH(config()->GetEnableRates(version_info::Channel::UNKNOWN));
   EXPECT_EQ((RelativePopulations{80, 20}),
             config()->GetEnableRates(version_info::Channel::CANARY));
+#if defined(OS_ANDROID)
+  // Note: death tests aren't supported on Android. Otherwise this test would
+  // check that the other inputs result in CHECKs.
+#else
+  EXPECT_CHECK_DEATH(config()->GetEnableRates(version_info::Channel::UNKNOWN));
   EXPECT_EQ((RelativePopulations{80, 20}),
             config()->GetEnableRates(version_info::Channel::DEV));
   EXPECT_CHECK_DEATH(config()->GetEnableRates(version_info::Channel::BETA));
@@ -149,8 +150,8 @@ MAYBE_PLATFORM_CONFIG_TEST_F(ThreadProfilerPlatformConfigurationTest,
 #if defined(OS_ANDROID)
   EXPECT_EQ(0.0, config()->GetChildProcessEnableFraction(
                      metrics::CallStackProfileParams::GPU_PROCESS));
-  EXPECT_EQ(0.0, config()->GetChildProcessEnableFraction(
-                     metrics::CallStackProfileParams::RENDERER_PROCESS));
+  EXPECT_EQ(0.006, config()->GetChildProcessEnableFraction(
+                       metrics::CallStackProfileParams::RENDERER_PROCESS));
   EXPECT_EQ(0.0, config()->GetChildProcessEnableFraction(
                      metrics::CallStackProfileParams::NETWORK_SERVICE_PROCESS));
   EXPECT_EQ(0.0, config()->GetChildProcessEnableFraction(
@@ -191,7 +192,7 @@ MAYBE_PLATFORM_CONFIG_TEST_F(ThreadProfilerPlatformConfigurationTest,
       metrics::CallStackProfileParams::GPU_PROCESS,
       metrics::CallStackProfileParams::COMPOSITOR_THREAD));
 
-  EXPECT_FALSE(config()->IsEnabledForThread(
+  EXPECT_TRUE(config()->IsEnabledForThread(
       metrics::CallStackProfileParams::RENDERER_PROCESS,
       metrics::CallStackProfileParams::MAIN_THREAD));
   EXPECT_FALSE(config()->IsEnabledForThread(