[Sampling profiler] Move ThreadDelegateAndroid to ThreadDelegatePosix

This class is POSIX-specific rather than Android-specific. Treating it
as such will facilitate build configuration for future Android/Linux
profiler work.

Adds a StackSampler stub for iOS to avoid a dependency on
ThreadDelegatePosix since that platform doesn't have
pthread_getattr_np.

Also cleans up some out of date comments.

TBR=gab@chromium.org

Bug: 988579
Change-Id: If4df5da23f4ba9664f33a097c5387f6725b8301d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854502
Commit-Queue: Mike Wittman <wittman@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708220}

base/BUILD.gn

@@ -629,6 +629,7 @@ jumbo_component("base") {
     "profiler/stack_sampler_android.cc",
     "profiler/stack_sampler_impl.cc",
     "profiler/stack_sampler_impl.h",
+    "profiler/stack_sampler_ios.cc",
     "profiler/stack_sampler_mac.cc",
     "profiler/stack_sampler_win.cc",
     "profiler/stack_sampling_profiler.cc",
@@ -639,8 +640,6 @@ jumbo_component("base") {
     "profiler/suspendable_thread_delegate_win.cc",
     "profiler/suspendable_thread_delegate_win.h",
     "profiler/thread_delegate.h",
-    "profiler/thread_delegate_android.cc",
-    "profiler/thread_delegate_android.h",
     "profiler/unwinder.h",
     "rand_util.cc",
     "rand_util.h",
@@ -1185,7 +1184,6 @@ jumbo_component("base") {
       "process/process_handle_posix.cc",
       "process/process_metrics_posix.cc",
       "process/process_posix.cc",
-      "profiler/stack_sampler_posix.cc",
       "rand_util_posix.cc",
       "sampling_heap_profiler/module_cache_posix.cc",
       "strings/string_util_posix.h",
@@ -1222,6 +1220,14 @@ jumbo_component("base") {
 
     if (is_posix) {
       sources += [ "base_paths_posix.h" ]
+
+      if (!is_mac && !is_ios) {
+        sources += [
+          "profiler/stack_sampler_posix.cc",
+          "profiler/thread_delegate_posix.cc",
+          "profiler/thread_delegate_posix.h",
+        ]
+      }
     }
 
     if (is_linux) {
@@ -1819,10 +1825,7 @@ jumbo_component("base") {
 
   # Desktop Mac.
   if (is_mac) {
-    sources -= [
-      "process/launch_posix.cc",
-      "profiler/stack_sampler_posix.cc",
-    ]
+    sources -= [ "process/launch_posix.cc" ]
     sources += [
       "files/file_path_watcher_fsevents.cc",
       "files/file_path_watcher_fsevents.h",

base/profiler/register_context.h

@@ -17,7 +17,7 @@
 #include <windows.h>
 #elif defined(OS_MACOSX)
 #include <mach/machine/thread_status.h>
-#elif defined(OS_ANDROID) && !defined(ARCH_CPU_64_BITS)
+#elif defined(OS_ANDROID) || defined(OS_LINUX)
 #include <sys/ucontext.h>
 #endif
 
@@ -85,7 +85,8 @@ inline uintptr_t& RegisterContextInstructionPointer(
   return AsUintPtr(&context->__rip);
 }
 
-#elif defined(OS_ANDROID) && defined(ARCH_CPU_ARM_FAMILY) && \
+#elif (defined(OS_ANDROID) || defined(OS_LINUX)) && \
+    defined(ARCH_CPU_ARM_FAMILY) &&                 \
     defined(ARCH_CPU_32_BITS)  // #if defined(OS_WIN)
 
 using RegisterContext = mcontext_t;

base/profiler/stack_sampler_android.cc

@@ -9,7 +9,7 @@
 #include "base/profiler/native_unwinder_android.h"
 #include "base/profiler/stack_copier_signal.h"
 #include "base/profiler/stack_sampler_impl.h"
-#include "base/profiler/thread_delegate_android.h"
+#include "base/profiler/thread_delegate_posix.h"
 #include "base/threading/platform_thread.h"
 
 namespace base {
@@ -20,7 +20,7 @@ std::unique_ptr<StackSampler> StackSampler::Create(
     StackSamplerTestDelegate* test_delegate) {
   return std::make_unique<StackSamplerImpl>(
       std::make_unique<StackCopierSignal>(
-          std::make_unique<ThreadDelegateAndroid>(thread_id)),
+          std::make_unique<ThreadDelegatePosix>(thread_id)),
       std::make_unique<NativeUnwinderAndroid>(), module_cache, test_delegate);
 }
 

base/profiler/stack_sampler_ios.cc

+// Copyright 2019 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.
+
+// Placeholder functions for the StackSampler on iOS, which is not currently
+// supported.
+
+#include "base/profiler/stack_sampler.h"
+
+namespace base {
+
+// static
+std::unique_ptr<StackSampler> StackSampler::Create(
+    PlatformThreadId thread_id,
+    ModuleCache* module_cache,
+    StackSamplerTestDelegate* test_delegate) {
+  return nullptr;
+}
+
+// static
+size_t StackSampler::GetStackBufferSize() {
+  return 0;
+}
+
+}  // namespace base

base/profiler/thread_delegate_android.cc → base/profiler/thread_delegate_posix.cc

@@ -4,17 +4,10 @@
 
 #include <pthread.h>
 
-#include "base/profiler/thread_delegate_android.h"
+#include "base/profiler/thread_delegate_posix.h"
 
 #include "build/build_config.h"
 
-// IMPORTANT NOTE: Some functions within this implementation are invoked while
-// the target thread is suspended so it must not do any allocation from the
-// heap, including indirectly via use of DCHECK/CHECK or other logging
-// statements. Otherwise this code can deadlock on heap locks acquired by the
-// target thread before it was suspended. These functions are commented with "NO
-// HEAP ALLOCATIONS".
-
 namespace base {
 
 namespace {
@@ -30,14 +23,14 @@ uintptr_t GetThreadStackBaseAddress(PlatformThreadId thread_id) {
 
 }  // namespace
 
-ThreadDelegateAndroid::ThreadDelegateAndroid(PlatformThreadId thread_id)
+ThreadDelegatePosix::ThreadDelegatePosix(PlatformThreadId thread_id)
     : thread_stack_base_address_(GetThreadStackBaseAddress(thread_id)) {}
 
-uintptr_t ThreadDelegateAndroid::GetStackBaseAddress() const {
+uintptr_t ThreadDelegatePosix::GetStackBaseAddress() const {
   return thread_stack_base_address_;
 }
 
-std::vector<uintptr_t*> ThreadDelegateAndroid::GetRegistersToRewrite(
+std::vector<uintptr_t*> ThreadDelegatePosix::GetRegistersToRewrite(
     RegisterContext* thread_context) {
 #if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
   return {
@@ -59,6 +52,7 @@ std::vector<uintptr_t*> ThreadDelegateAndroid::GetRegistersToRewrite(
       // addresses of executable code, not addresses in the stack.
   };
 #else  // #if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
+  // Unimplemented for other architectures.
   return {};
 #endif
 }

base/profiler/thread_delegate_android.h → base/profiler/thread_delegate_posix.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef BASE_PROFILER_THREAD_DELEGATE_ANDROID_H_
-#define BASE_PROFILER_THREAD_DELEGATE_ANDROID_H_
+#ifndef BASE_PROFILER_THREAD_DELEGATE_POSIX_H_
+#define BASE_PROFILER_THREAD_DELEGATE_POSIX_H_
 
 #include "base/base_export.h"
 #include "base/profiler/thread_delegate.h"
@@ -12,15 +12,13 @@
 namespace base {
 
 // Platform- and thread-specific implementation in support of stack sampling on
-// Android.
-//
-// TODO(https://crbug.com/988579): Implement this class.
-class BASE_EXPORT ThreadDelegateAndroid : public ThreadDelegate {
+// POSIX.
+class BASE_EXPORT ThreadDelegatePosix : public ThreadDelegate {
  public:
-  ThreadDelegateAndroid(PlatformThreadId thread_id);
+  ThreadDelegatePosix(PlatformThreadId thread_id);
 
-  ThreadDelegateAndroid(const ThreadDelegateAndroid&) = delete;
-  ThreadDelegateAndroid& operator=(const ThreadDelegateAndroid&) = delete;
+  ThreadDelegatePosix(const ThreadDelegatePosix&) = delete;
+  ThreadDelegatePosix& operator=(const ThreadDelegatePosix&) = delete;
 
   // ThreadDelegate
   uintptr_t GetStackBaseAddress() const override;
@@ -33,4 +31,4 @@ class BASE_EXPORT ThreadDelegateAndroid : public ThreadDelegate {
 
 }  // namespace base
 
-#endif  // BASE_PROFILER_THREAD_DELEGATE_ANDROID_H_
+#endif  // BASE_PROFILER_THREAD_DELEGATE_POSIX_H_