Reland "[Sampling profiler] Factor out platform config 1/4"

This reverts commit 2c8b578a.

Reason for reland: originally reverted because the change was broken
by https://crrev.com/810801 and that change has since been reverted

Original change's description:
> [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/+/2425165
> Reviewed-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}

TBR=etiennep@chromium.org, ssid@chromium.org

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

base/profiler/stack_sampling_profiler.cc

@@ -741,9 +741,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

@@ -3773,6 +3773,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