[Sampling profiler] Factor out platform config 1/4

First in a series factoring the ThreadProfiler platform specific
configuration state from the code that takes action on the
state. Defines an IsSupported function that is true if the profiler
is supported on the chrome branding and channel for the platform.

The end goal is to reduce the configuration complexity, to support
enabling per-thread on Android.

Bug: 1129939
Change-Id: Ibbd6f4725804b40ac77e15aaa3bd15ad76a0e7b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425165Reviewed-by: ssid <ssid@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810835}

base/profiler/stack_sampling_profiler.cc

@@ -740,9 +740,9 @@ TimeTicks StackSamplingProfiler::TestPeer::GetNextSampleTime(
 }
 
 // static
-// The profiler is currently only implemented for Windows x64 and MacOSX.
-// TODO(https://crbug.com/1004855): enable for Android arm.
-bool StackSamplingProfiler::IsSupported() {
+// The profiler is currently supported for Windows x64, MacOSX x64, and Android
+// ARM32.
+bool StackSamplingProfiler::IsSupportedForCurrentPlatform() {
 #if (defined(OS_WIN) && defined(ARCH_CPU_X86_64)) || \
     (defined(OS_MAC) && defined(ARCH_CPU_X86_64)) || \
     (defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE))

base/profiler/stack_sampling_profiler.h

@@ -92,7 +92,7 @@ class BASE_EXPORT StackSamplingProfiler {
 
   // Returns true if the profiler is supported on the current platform
   // configuration.
-  static bool IsSupported();
+  static bool IsSupportedForCurrentPlatform();
 
   // Creates a profiler for the the thread associated with |thread_token|,
   // generated by GetSamplingProfilerCurrentThreadToken().

chrome/common/profiler/BUILD.gn

@@ -15,6 +15,8 @@ source_set("profiler") {
     "main_thread_stack_sampling_profiler.cc",
     "thread_profiler.cc",
     "thread_profiler_configuration.cc",
+    "thread_profiler_platform_configuration.cc",
+    "thread_profiler_platform_configuration.h",
   ]
 
   deps = [

chrome/common/profiler/stack_sampling_browsertest.cc

@@ -132,7 +132,7 @@ bool ShouldSkipTestForMacOS11() {
   // DCHECK that that remains the case so these tests are re-enabled when the
   // sampling profiler is re-enabled there.
   if (base::mac::IsAtLeastOS11()) {
-    DCHECK(!base::StackSamplingProfiler::IsSupported());
+    DCHECK(!base::StackSamplingProfiler::IsSupportedForCurrentPlatform());
     return true;
   }
 #endif

chrome/common/profiler/thread_profiler_configuration.cc

@@ -11,6 +11,7 @@
 #include "build/build_config.h"
 #include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/profiler/thread_profiler_platform_configuration.h"
 #include "components/version_info/version_info.h"
 #include "content/public/common/content_switches.h"
 #include "extensions/buildflags/buildflags.h"
@@ -66,21 +67,6 @@ bool IsBrowserTestModeEnabled() {
          switches::kStartStackProfilerBrowserTest;
 }
 
-bool IsProfilerEnabledForChannel() {
-#if defined(OS_ANDROID)
-  // Profiling is only enable in it's own dedicated browser tests on Android.
-  // TODO(crbug.com/1004855): Remove this logic to launch profiler.
-  return IsBrowserTestModeEnabled();
-#elif BUILDFLAG(GOOGLE_CHROME_BRANDING)
-  // Only run on canary and dev.
-  const version_info::Channel channel = chrome::GetChannel();
-  return channel == version_info::Channel::CANARY ||
-         channel == version_info::Channel::DEV;
-#else
-  return true;
-#endif
-}
-
 bool ShouldEnableProfilerForNextRendererProcess() {
   // Ensure deterministic behavior for testing the profiler itself.
   if (IsBrowserTestModeEnabled())
@@ -93,7 +79,11 @@ bool ShouldEnableProfilerForNextRendererProcess() {
 }  // namespace
 
 ThreadProfilerConfiguration::ThreadProfilerConfiguration()
-    : configuration_(GenerateConfiguration()) {}
+    : platform_configuration_(ThreadProfilerPlatformConfiguration::Create(
+          IsBrowserTestModeEnabled())),
+      configuration_(GenerateConfiguration(*platform_configuration_)) {}
+
+ThreadProfilerConfiguration::~ThreadProfilerConfiguration() = default;
 
 base::StackSamplingProfiler::SamplingParams
 ThreadProfilerConfiguration::GetSamplingParams() const {
@@ -129,10 +119,10 @@ bool ThreadProfilerConfiguration::GetSyntheticFieldTrial(
     std::string* group_name) const {
   DCHECK(IsBrowserProcess());
 
-  if (!base::StackSamplingProfiler::IsSupported())
-    return false;
-  if (!IsProfilerEnabledForChannel())
+  if (!platform_configuration_->IsSupported(BUILDFLAG(GOOGLE_CHROME_BRANDING),
+                                            chrome::GetChannel())) {
     return false;
+  }
 
   *trial_name = "SyntheticStackProfilingConfiguration";
   *group_name = std::string();
@@ -220,14 +210,16 @@ ThreadProfilerConfiguration::ChooseConfiguration(
 
 // static
 ThreadProfilerConfiguration::ProfileConfiguration
-ThreadProfilerConfiguration::GenerateConfiguration() {
+ThreadProfilerConfiguration::GenerateConfiguration(
+    const ThreadProfilerPlatformConfiguration& platform_configuration) {
   if (!IsBrowserProcess())
     return PROFILE_FROM_COMMAND_LINE;
 
-  if (!base::StackSamplingProfiler::IsSupported())
-    return PROFILE_DISABLED;
-  if (!IsProfilerEnabledForChannel())
+  const version_info::Channel channel = chrome::GetChannel();
+  if (!platform_configuration.IsSupported(BUILDFLAG(GOOGLE_CHROME_BRANDING),
+                                          channel)) {
     return PROFILE_DISABLED;
+  }
 
 #if defined(OS_ANDROID)
   // Allow profiling if the Android Java/native unwinder module is available at

chrome/common/profiler/thread_profiler_configuration.h

@@ -15,6 +15,8 @@ namespace base {
 class CommandLine;
 }  // namespace base
 
+class ThreadProfilerPlatformConfiguration;
+
 // ThreadProfilerConfiguration chooses a configuration for the enable state of
 // the stack sampling profiler across all processes. This configuration is
 // determined once at browser process startup. Configurations for child
@@ -22,6 +24,7 @@ class CommandLine;
 class ThreadProfilerConfiguration {
  public:
   ThreadProfilerConfiguration();
+  ~ThreadProfilerConfiguration();
 
   // Get the stack sampling params to use.
   base::StackSamplingProfiler::SamplingParams GetSamplingParams() const;
@@ -72,11 +75,15 @@ class ThreadProfilerConfiguration {
       const std::vector<Variation>& variations);
 
   // Generates sampling profiler configurations for all processes.
-  static ProfileConfiguration GenerateConfiguration();
+  static ProfileConfiguration GenerateConfiguration(
+      const ThreadProfilerPlatformConfiguration& platform_configuration);
 
   // NOTE: all state in this class must be const and initialized at construction
   // time to ensure thread-safe access post-construction.
 
+  const std::unique_ptr<ThreadProfilerPlatformConfiguration>
+      platform_configuration_;
+
   // In the browser process this represents the configuration to use across all
   // Chrome processes. In the child processes it is always
   // PROFILE_FROM_COMMAND_LINE.

chrome/common/profiler/thread_profiler_platform_configuration.cc

+// Copyright 2020 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/common/profiler/thread_profiler_platform_configuration.h"
+
+#include "base/profiler/stack_sampling_profiler.h"
+#include "build/build_config.h"
+
+namespace {
+
+// The default configuration to use in the absence of special circumstances on a
+// specific platform.
+class DefaultPlatformConfiguration
+    : public ThreadProfilerPlatformConfiguration {
+ public:
+  explicit DefaultPlatformConfiguration(bool browser_test_mode_enabled);
+
+ protected:
+  bool IsSupportedForChannel(bool is_chrome_branded,
+                             version_info::Channel channel) const override;
+
+  bool browser_test_mode_enabled() const { return browser_test_mode_enabled_; }
+
+ private:
+  const bool browser_test_mode_enabled_;
+};
+
+DefaultPlatformConfiguration::DefaultPlatformConfiguration(
+    bool browser_test_mode_enabled)
+    : browser_test_mode_enabled_(browser_test_mode_enabled) {}
+
+bool DefaultPlatformConfiguration::IsSupportedForChannel(
+    bool is_chrome_branded,
+    version_info::Channel channel) const {
+  // The profiler is always supported for local builds and the CQ.
+  if (!is_chrome_branded)
+    return true;
+
+  // Canary and dev are the only channels currently supported in release
+  // builds.
+  return channel == version_info::Channel::CANARY ||
+         channel == version_info::Channel::DEV;
+}
+
+#if defined(OS_ANDROID)
+// The configuration to use for the Android platform. Applies to ARM32 which is
+// the only Android architecture currently supported by StackSamplingProfiler.
+// Defined in terms of DefaultPlatformConfiguration where Android does not
+// differ from the default case.
+class AndroidPlatformConfiguration : public DefaultPlatformConfiguration {
+ public:
+  explicit AndroidPlatformConfiguration(bool browser_test_mode_enabled);
+
+ protected:
+  bool IsSupportedForChannel(bool is_chrome_branded,
+                             version_info::Channel channel) const override;
+};
+
+AndroidPlatformConfiguration::AndroidPlatformConfiguration(
+    bool browser_test_mode_enabled)
+    : DefaultPlatformConfiguration(browser_test_mode_enabled) {}
+
+bool AndroidPlatformConfiguration::IsSupportedForChannel(
+    bool is_chrome_branded,
+    version_info::Channel channel) const {
+  // On Android profiling is only enabled in its own dedicated browser tests
+  // in local builds and the CQ.
+  // TODO(https://crbug.com/1004855): Enable across all browser tests.
+  return browser_test_mode_enabled();
+}
+#endif  // defined(OS_ANDROID)
+
+}  // namespace
+
+// static
+std::unique_ptr<ThreadProfilerPlatformConfiguration>
+ThreadProfilerPlatformConfiguration::Create(bool browser_test_mode_enabled) {
+#if defined(OS_ANDROID)
+  using PlatformConfiguration = AndroidPlatformConfiguration;
+#else
+  using PlatformConfiguration = DefaultPlatformConfiguration;
+#endif
+  return std::make_unique<PlatformConfiguration>(browser_test_mode_enabled);
+}
+
+bool ThreadProfilerPlatformConfiguration::IsSupported(
+    bool is_chrome_branded,
+    version_info::Channel channel) const {
+  return base::StackSamplingProfiler::IsSupportedForCurrentPlatform() &&
+         IsSupportedForChannel(is_chrome_branded, channel);
+}

chrome/common/profiler/thread_profiler_platform_configuration.h

+// Copyright 2020 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.
+
+#ifndef CHROME_COMMON_PROFILER_THREAD_PROFILER_PLATFORM_CONFIGURATION_H_
+#define CHROME_COMMON_PROFILER_THREAD_PROFILER_PLATFORM_CONFIGURATION_H_
+
+#include <memory>
+
+#include "components/version_info/version_info.h"
+
+// Encapsulates the platform-specific configuration for the ThreadProfiler.
+//
+// The interface functions this class make a distinction between 'supported' and
+// 'enabled' state. Supported means the profiler can be run in *some*
+// circumstances for *some* fraction of the population on the
+// platform/branding/channel combination. This state is intended to enable
+// experiment reporting. This avoids spamming UMA with experiment state on
+// platforms/channels where the profiler is not being run.
+//
+// Enabled means we chose to the run the profiler on at least some threads on a
+// platform/branding/channel combination that is configured for profiling. The
+// overall enable/disable state should be reported to UMA in this case.
+class ThreadProfilerPlatformConfiguration {
+ public:
+  virtual ~ThreadProfilerPlatformConfiguration() = default;
+
+  // Create the platform configuration.
+  static std::unique_ptr<ThreadProfilerPlatformConfiguration> Create(
+      bool browser_test_mode_enabled);
+
+  // True if the platform supports the StackSamplingProfiler and the profiler is
+  // to be run for the channel/chrome branding.
+  bool IsSupported(bool is_chrome_branded, version_info::Channel channel) const;
+
+ protected:
+  // True if the profiler is to be run for the channel/chrome branding on the
+  // platform. Does not need to check whether the StackSamplingProfiler is
+  // supported on the platform since that's done in IsSupported().
+  virtual bool IsSupportedForChannel(bool is_chrome_branded,
+                                     version_info::Channel channel) const = 0;
+};
+
+#endif  // CHROME_COMMON_PROFILER_THREAD_PROFILER_PLATFORM_CONFIGURATION_H_

chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc

+// Copyright 2020 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/common/profiler/thread_profiler_platform_configuration.h"
+
+#include "base/profiler/profiler_buildflags.h"
+#include "build/build_config.h"
+#include "components/version_info/version_info.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if (defined(OS_WIN) && defined(ARCH_CPU_X86_64)) || \
+    (defined(OS_MAC) && defined(ARCH_CPU_X86_64)) || \
+    (defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE))
+#define THREAD_PROFILER_SUPPORTED_ON_PLATFORM true
+#else
+#define THREAD_PROFILER_SUPPORTED_ON_PLATFORM false
+#endif
+
+// The browser_test_mode_enabled=true scenario is already covered by the browser
+// tests so doesn't require separate testing here.
+TEST(ThreadProfilerPlatformConfigurationTest, IsSupported) {
+  const std::unique_ptr<ThreadProfilerPlatformConfiguration> config =
+      ThreadProfilerPlatformConfiguration::Create(
+          /*browser_test_mode_enabled=*/false);
+#if !THREAD_PROFILER_SUPPORTED_ON_PLATFORM
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::UNKNOWN));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::CANARY));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::DEV));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::BETA));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::STABLE));
+
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/false,
+                                   version_info::Channel::UNKNOWN));
+#elif defined(OS_ANDROID)
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::UNKNOWN));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::CANARY));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::DEV));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::BETA));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::STABLE));
+
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/false,
+                                   version_info::Channel::UNKNOWN));
+#else
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::UNKNOWN));
+  EXPECT_TRUE(config->IsSupported(/*is_chrome_branded=*/true,
+                                  version_info::Channel::CANARY));
+  EXPECT_TRUE(config->IsSupported(/*is_chrome_branded=*/true,
+                                  version_info::Channel::DEV));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::BETA));
+  EXPECT_FALSE(config->IsSupported(/*is_chrome_branded=*/true,
+                                   version_info::Channel::STABLE));
+
+  EXPECT_TRUE(config->IsSupported(/*is_chrome_branded=*/false,
+                                  version_info::Channel::UNKNOWN));
+#endif
+}

chrome/test/BUILD.gn

@@ -3771,6 +3771,7 @@ test("unit_tests") {
     "../common/mac/mock_launchd.mm",
     "../common/net/safe_search_util_unittest.cc",
     "../common/pref_names_util_unittest.cc",
+    "../common/profiler/thread_profiler_platform_configuration_unittest.cc",
     "../common/profiler/thread_profiler_unittest.cc",
     "../common/qr_code_generator/qr_code_generator_unittest.cc",
     "../renderer/chrome_content_renderer_client_unittest.cc",

services/tracing/public/cpp/stack_sampling/tracing_sampler_profiler.cc

@@ -686,7 +686,7 @@ void TracingSamplerProfiler::StartTracing(
   // On Android the sampling profiler is implemented by tracing service and is
   // not yet supported by base::StackSamplingProfiler. So, only check this if
   // service does not support unwinding in current platform.
-  if (!base::StackSamplingProfiler::IsSupported())
+  if (!base::StackSamplingProfiler::IsSupportedForCurrentPlatform())
     return;
 #endif  // !(ANDROID_ARM64_UNWINDING_SUPPORTED ||
         // ANDROID_CFI_UNWINDING_SUPPORTED)

services/tracing/public/cpp/stack_sampling/tracing_sampler_profiler_unittest.cc

@@ -190,7 +190,7 @@ bool ShouldSkipTestForMacOS11() {
   // DCHECK here so that when the sampling profiler is re-enabled on macOS 11,
   // these tests are also re-enabled.
   if (base::mac::IsAtLeastOS11()) {
-    DCHECK(!base::StackSamplingProfiler::IsSupported());
+    DCHECK(!base::StackSamplingProfiler::IsSupportedForCurrentPlatform());
     return true;
   }
 #endif