Commit ec645d52 authored by Etienne Pierre-doray's avatar Etienne Pierre-doray Committed by Commit Bot

[Clank SSM]: Enable stack sampling in android browsertests.

android_browsertests must depend on libunwindstack directly instead of
through DFM, since loading of a partitioned library is not supported in
an APK.
(internal discussion)
https://groups.google.com/a/google.com/g/clank-components/c/ktt285Gtax4/m/yRy8qm8LAQAJ

This CL also re-enables tracing use of StackSampingProfiler, which
was accidentally disabled in https://crrev.com/784147?

Bug: 1004855
Change-Id: I5967bf3815008843ba8a77e926977e39a4ccfbd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2293071Reviewed-by: default avatarWez <wez@chromium.org>
Reviewed-by: default avatarMike Wittman <wittman@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@google.com>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789530}
parent 0eefb312
......@@ -91,11 +91,6 @@ dep_libevent =
# Determines whether message_pump_libevent should be used.
use_libevent = dep_libevent && !is_ios
# Whether or not cfi table should be enabled on arm.
# TODO(crbug.com/1090409): Replace can_unwind_with_cfi_table once sampling
# profiler is enabled on android.
enable_arm_cfi_table = is_android && !is_component_build && current_cpu == "arm"
if (is_android) {
import("//build/config/android/rules.gni")
}
......
......@@ -742,8 +742,9 @@ TimeTicks StackSamplingProfiler::TestPeer::GetNextSampleTime(
// 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_MACOSX) && defined(ARCH_CPU_X86_64) && !defined(OS_IOS))
#if (defined(OS_WIN) && defined(ARCH_CPU_X86_64)) || \
(defined(OS_MACOSX) && defined(ARCH_CPU_X86_64) && !defined(OS_IOS)) || \
(defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE))
#if defined(OS_MACOSX)
// TODO(https://crbug.com/1098119): Fix unwinding on OS X 10.16. The OS
// has moved all system libraries into the dyld shared cache and this
......
......@@ -172,6 +172,11 @@ assert(!can_unwind_with_frame_pointers || enable_frame_pointers)
can_unwind_with_cfi_table = is_android && !is_component_build &&
!enable_frame_pointers && current_cpu == "arm"
# Whether or not cfi table should be enabled on arm.
# TODO(crbug.com/1090409): Replace can_unwind_with_cfi_table with this once
# sampling profiler is enabled on android.
enable_arm_cfi_table = is_android && !is_component_build && current_cpu == "arm"
declare_args() {
# Set to true to use lld, the LLVM linker.
use_lld = is_clang && (!is_ios && !is_mac)
......
......@@ -52,3 +52,16 @@ component("stack_unwinder") {
cflags = [ "-fsymbol-partition=stack_unwinder_partition" ]
}
}
# Since loading of a partitioned library is not supported in an APK, a separate
# target without partition is necessary for testing.
source_set("stack_unwinder_for_testing") {
testonly = true
sources = [ "stack_unwinder_module_contents_impl.cc" ]
deps = [
":jni_headers",
"//base",
"//base:native_unwinder_android",
"//chrome/android/features/stack_unwinder/public:native",
]
}
......@@ -11,14 +11,20 @@
#include "base/test/scoped_run_loop_timeout.h"
#include "base/thread_annotations.h"
#include "base/threading/platform_thread.h"
#include "build/build_config.h"
#include "chrome/common/channel_info.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/metrics/call_stack_profile_metrics_provider.h"
#include "components/version_info/channel.h"
#include "content/public/test/browser_test.h"
#include "third_party/metrics_proto/sampled_profile.pb.h"
#if defined(OS_ANDROID)
#include "chrome/test/base/android/android_browser_test.h"
#else
#include "chrome/test/base/in_process_browser_test.h"
#endif
namespace {
// Class that intercepts and stores profiles provided to the
......@@ -95,7 +101,7 @@ bool MatchesProfile(metrics::SampledProfile::TriggerEvent trigger_event,
profile.process() == process && profile.thread() == thread;
}
class StackSamplingBrowserTest : public InProcessBrowserTest {
class StackSamplingBrowserTest : public PlatformBrowserTest {
public:
void SetUp() override {
// Arrange to intercept the CPU profiles at the time they're provided to the
......@@ -104,7 +110,7 @@ class StackSamplingBrowserTest : public InProcessBrowserTest {
SetCpuInterceptorCallbackForTesting(base::BindRepeating(
&ProfileInterceptor::Intercept,
base::Unretained(&ProfileInterceptor::GetInstance())));
InProcessBrowserTest::SetUp();
PlatformBrowserTest::SetUp();
}
void SetUpCommandLine(base::CommandLine* command_line) override {
......@@ -190,8 +196,15 @@ IN_PROC_BROWSER_TEST_F(StackSamplingBrowserTest,
metrics::COMPOSITOR_THREAD));
}
// Android doesn't have a network service process.
#if defined(OS_ANDROID)
#define MAYBE_NetworkServiceProcessIOThread \
DISABLED_NetworkServiceProcessIOThread
#else
#define MAYBE_NetworkServiceProcessIOThread NetworkServiceProcessIOThread
#endif
IN_PROC_BROWSER_TEST_F(StackSamplingBrowserTest,
NetworkServiceProcessIOThread) {
MAYBE_NetworkServiceProcessIOThread) {
EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP,
metrics::NETWORK_SERVICE_PROCESS,
metrics::IO_THREAD));
......
......@@ -37,17 +37,6 @@ namespace {
base::LazyInstance<StackSamplingConfiguration>::Leaky g_configuration =
LAZY_INSTANCE_INITIALIZER;
bool IsProfilerEnabledForChannel() {
#if 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
}
// Returns true if the current execution is taking place in the browser process.
bool IsBrowserProcess() {
const base::CommandLine* command_line =
......@@ -77,6 +66,22 @@ 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();
#endif
#if 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())
......
......@@ -499,6 +499,26 @@ if (is_android) {
"base/android/android_browser_test_browsertest_android.cc",
]
# Add unwind tables in android_browsertests apk to support enabling
# sampling profiler. The unwind tables are generated from debug info in the
# binary. Removing "default_symbols" and adding symbols config removes the
# "strip_debug" config that strips the debug info, on android_browsertests
# apk.
if (enable_arm_cfi_table) {
sources += [ "../common/profiler/stack_sampling_browsertest.cc" ]
deps += [
"//chrome/android/modules/stack_unwinder/internal:java",
"//chrome/android/modules/stack_unwinder/internal:stack_unwinder_for_testing",
]
configs -= [ "//build/config/compiler:default_symbols" ]
if (symbol_level == 2) {
configs += [ "//build/config/compiler:symbols" ]
} else {
configs += [ "//build/config/compiler:minimal_symbols" ]
}
add_unwind_tables_in_apk = true
}
data = [
"$root_gen_dir/chrome/android/chrome_apk_paks/chrome_100_percent.pak",
"$root_gen_dir/chrome/android/chrome_apk_paks/locales/en-US.pak",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment