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

This reverts commit b01abc31.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 810835 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2IwMWFiYzMxZjNlOWYzMjM4MzlkYjRmN2RmMjM1MjdmY2ZhYTA5ZTQM

Sample Failed Build: https://ci.chromium.org/b/8868141438660196432

Sample Failed Step: compile

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}


Change-Id: Idab7058fe3be31bb32bc88a95a8495a9b8dbf525
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1129939
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432998
Cr-Commit-Position: refs/heads/master@{#810875}

base/profiler/stack_sampling_profiler.cc

@@ -740,9 +740,9 @@ TimeTicks StackSamplingProfiler::TestPeer::GetNextSampleTime(
 }
 
 // static
-// The profiler is currently supported for Windows x64, MacOSX x64, and Android
-// ARM32.
-bool StackSamplingProfiler::IsSupportedForCurrentPlatform() {
+// The profiler is currently only implemented for Windows x64 and MacOSX.
+// TODO(https://crbug.com/1004855): enable for Android arm.
+bool StackSamplingProfiler::IsSupported() {
 #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 IsSupportedForCurrentPlatform();
+  static bool IsSupported();
 
   // Creates a profiler for the the thread associated with |thread_token|,
   // generated by GetSamplingProfilerCurrentThreadToken().

chrome/common/profiler/BUILD.gn

@@ -15,8 +15,6 @@ 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::IsSupportedForCurrentPlatform());
+    DCHECK(!base::StackSamplingProfiler::IsSupported());
     return true;
   }
 #endif

chrome/common/profiler/thread_profiler_configuration.cc

@@ -11,7 +11,6 @@
 #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"
@@ -67,6 +66,21 @@ 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())
@@ -79,11 +93,7 @@ bool ShouldEnableProfilerForNextRendererProcess() {
 }  // namespace
 
 ThreadProfilerConfiguration::ThreadProfilerConfiguration()
-    : platform_configuration_(ThreadProfilerPlatformConfiguration::Create(
-          IsBrowserTestModeEnabled())),
-      configuration_(GenerateConfiguration(*platform_configuration_)) {}
-
-ThreadProfilerConfiguration::~ThreadProfilerConfiguration() = default;
+    : configuration_(GenerateConfiguration()) {}
 
 base::StackSamplingProfiler::SamplingParams
 ThreadProfilerConfiguration::GetSamplingParams() const {
@@ -119,10 +129,10 @@ bool ThreadProfilerConfiguration::GetSyntheticFieldTrial(
     std::string* group_name) const {
   DCHECK(IsBrowserProcess());
 
-  if (!platform_configuration_->IsSupported(BUILDFLAG(GOOGLE_CHROME_BRANDING),
-                                            chrome::GetChannel())) {
+  if (!base::StackSamplingProfiler::IsSupported())
+    return false;
+  if (!IsProfilerEnabledForChannel())
     return false;
-  }
 
   *trial_name = "SyntheticStackProfilingConfiguration";
   *group_name = std::string();
@@ -210,16 +220,14 @@ ThreadProfilerConfiguration::ChooseConfiguration(
 
 // static
 ThreadProfilerConfiguration::ProfileConfiguration
-ThreadProfilerConfiguration::GenerateConfiguration(
-    const ThreadProfilerPlatformConfiguration& platform_configuration) {
+ThreadProfilerConfiguration::GenerateConfiguration() {
   if (!IsBrowserProcess())
     return PROFILE_FROM_COMMAND_LINE;
 
-  const version_info::Channel channel = chrome::GetChannel();
-  if (!platform_configuration.IsSupported(BUILDFLAG(GOOGLE_CHROME_BRANDING),
-                                          channel)) {
+  if (!base::StackSamplingProfiler::IsSupported())
+    return PROFILE_DISABLED;
+  if (!IsProfilerEnabledForChannel())
     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,8 +15,6 @@ 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
@@ -24,7 +22,6 @@ class ThreadProfilerPlatformConfiguration;
 class ThreadProfilerConfiguration {
  public:
   ThreadProfilerConfiguration();
-  ~ThreadProfilerConfiguration();
 
   // Get the stack sampling params to use.
   base::StackSamplingProfiler::SamplingParams GetSamplingParams() const;
@@ -75,15 +72,11 @@ class ThreadProfilerConfiguration {
       const std::vector<Variation>& variations);
 
   // Generates sampling profiler configurations for all processes.
-  static ProfileConfiguration GenerateConfiguration(
-      const ThreadProfilerPlatformConfiguration& platform_configuration);
+  static ProfileConfiguration GenerateConfiguration();
 
   // 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,7 +3771,6 @@ 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::IsSupportedForCurrentPlatform())
+  if (!base::StackSamplingProfiler::IsSupported())
     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::IsSupportedForCurrentPlatform());
+    DCHECK(!base::StackSamplingProfiler::IsSupported());
     return true;
   }
 #endif