From 2aa7a444bed04e770741d2142cef91875a3e8283 Mon Sep 17 00:00:00 2001 From: yuying-y <78829091+yuying-y@users.noreply.github.com> Date: Tue, 13 Feb 2024 17:52:39 -0800 Subject: [PATCH] Remove the option to start crash handler at launch (#2387) This is a tech debt cleanup. Starting the crash handler when a crash happens instead of when the app starts has been the default behavior. This behavior saves 2-3MB memory that the app consumes. This behavior has been proved reliable. The code to allow the other behavior can be removed. b/242101457 Change-Id: Icc725cb801a72b4635d6104abc8226a6f6323f46 (cherry picked from commit 1c7af6362230c3763a9edcdbb34773c001e28e55) --- starboard/CHANGELOG.md | 58 +++++++++++++++++++ starboard/android/shared/BUILD.gn | 2 - starboard/android/shared/android_main.cc | 12 +--- starboard/doc/crash_handlers.md | 5 +- starboard/linux/shared/BUILD.gn | 2 - starboard/linux/x64x11/main.cc | 9 +-- starboard/raspi/shared/BUILD.gn | 2 - starboard/raspi/shared/main.cc | 9 +-- .../shared/starboard/starboard_switches.cc | 26 --------- .../shared/starboard/starboard_switches.h | 37 ------------ starboard/stub/BUILD.gn | 2 - third_party/crashpad/wrapper/wrapper.cc | 28 +++------ third_party/crashpad/wrapper/wrapper.h | 6 +- third_party/crashpad/wrapper/wrapper_stub.cc | 3 +- 14 files changed, 75 insertions(+), 126 deletions(-) delete mode 100644 starboard/shared/starboard/starboard_switches.cc delete mode 100644 starboard/shared/starboard/starboard_switches.h diff --git a/starboard/CHANGELOG.md b/starboard/CHANGELOG.md index f113a5df1d49..3537399ab2c1 100644 --- a/starboard/CHANGELOG.md +++ b/starboard/CHANGELOG.md @@ -10,9 +10,67 @@ version previous to it. ## Experimental Version +<<<<<<< HEAD A description of all changes currently in the experimental Starboard version can be found in the comments of the "Experimental Feature Defines" section of [configuration.h](configuration.h). +======= +### Changed InstallCrashpadHandler API +This API doesn't support the option to start the crashpad handler at the +same time as the app launches anymore. Instead, the crashpad handler is +started when a crash happens. See details in starboard/doc/crash_handlers.md. + +### Convert SbUiNavGetInterface Starboard API into an extension +The `SbUiNavGetInterface` API is deprecated and replaced with a Starboard +extension named `SbUiNavInterface`. + +### Decrecated SbTime APIs and migrated to POSIX time APIs +The time APIs `SbTimeGetNow`, `SbTimeGetMonotonicNow`, +`SbTimeIsTimeThreadNowSupported` and `SbTimeGetMonotonicThreadNow` are +deprecated and the standard APIs `gettimeofday` from `` and +`clock_gettime` from `` should be used instead. + +### Deprecated SbStringFormat APIs and migrated to POSIX memory APIs +The StringFormat management APIs `SbStringFormat`, `SbStringFormatF`, +`SbStringFormatWide`, `SbStringFormatUnsifeF` are deprecated and the +standard APIs `vsnprintf`, `vfnprintf`, `vswprintf`, `snprintf` +from `` should be used instead. + +### Deprecated SbMemoryMap APIs and migrated to POSIX mmap +The memory mapping APIs `SbMemoryMap`, `SbMemoryUnmap`, `SbMemoryProtect` and +`SbMemoryFlush` are deprecated and the standard APIs `mmap`, `munmap`, +`mprotect`, `msync` from `` should be used instead. + +### Deprecated SbMemory allocation APIs and migrated to POSIX memory APIs +The memory management APIs `SbMemoryAllocate`, `SbMemoryReallocate`, +`SbMemoryCalloc`, `SbMemoryAllocateAligned`, `SbMemoryDeallocate`, +`SbMemoryDeallocateAligned` `SbStringDuplicate` are deprecated and the +standard APIs `malloc`, `realloc`, `calloc`, `posix_memalign`, `free` +from `` and `strdup` from `` should be used instead. + +### Deprecated SbMediaGetBufferAlignment +The `SbMediaGetBufferAlignment` API was deprecated. + +### Removed SbUser from SbStorageOpenRecord and SbStorageDeleteRecord +The `SbStorageOpenRecord` and `SbStorageDeleteRecord` APIs defined in +`starboard/storage.h` no longer have a parameter for `SbUser` as the APIs are +user-agnostic. + +### Removed SbUserGetCurrent, SbUserGetSignedIn, SbUserGetProperty, SbUserGetPropertySize, and kSbUserMaxSignedIn +The APIs defined in `starboard/user.h` are no longer used and have been +deprecated. + +### Removed SbByteSwapS16, SbByteSwapS32, SbByteSwapS64, SbByteSwapU16, SbByteSwapU32, and SbByteSwapU64 +The APIs defined in `starboard/byte_swap.h` are no longer used and have been +deprecated. + +### Removed SbImageDecode and SbImageIsDecodeSupported +The APIs defined in `starboard/image.h` are no longer used and have been +deprecated. + +### Deprecated SbStringScan and SbStringScanF +The APIs defined in `starboard/string.h` are deprecated and the standard API `vsscanf` and `sscanf` are used instead. +>>>>>>> 1c7af636223 (Remove the option to start crash handler at launch (#2387)) ## Version 15 ### Removed version suffixes of SbPlayer functions and structures diff --git a/starboard/android/shared/BUILD.gn b/starboard/android/shared/BUILD.gn index a34fdd56d6ed..87af4704ab28 100644 --- a/starboard/android/shared/BUILD.gn +++ b/starboard/android/shared/BUILD.gn @@ -456,8 +456,6 @@ static_library("starboard_platform") { sources += [ "//starboard/shared/starboard/crash_handler.cc", "//starboard/shared/starboard/crash_handler.h", - "//starboard/shared/starboard/starboard_switches.cc", - "//starboard/shared/starboard/starboard_switches.h", ] public_deps += [ diff --git a/starboard/android/shared/android_main.cc b/starboard/android/shared/android_main.cc index 7bccf5080083..72db8d941388 100644 --- a/starboard/android/shared/android_main.cc +++ b/starboard/android/shared/android_main.cc @@ -28,7 +28,6 @@ #include "starboard/event.h" #include "starboard/log.h" #include "starboard/shared/starboard/command_line.h" -#include "starboard/shared/starboard/starboard_switches.h" #include "starboard/thread.h" #if SB_IS(EVERGREEN_COMPATIBLE) #include "third_party/crashpad/wrapper/wrapper.h" // nogncheck @@ -195,15 +194,6 @@ std::string ExtractCertificatesToFileSystem() { } void InstallCrashpadHandler(const CommandLine& command_line) { - if (command_line.HasSwitch( - starboard::shared::starboard::kStartHandlerAtLaunch)) { - SB_LOG(WARNING) << "--" - << starboard::shared::starboard::kStartHandlerAtLaunch - << " not supported for AOSP Evergreen, not installing " - << "Crashpad handler"; - return; - } - std::string extracted_ca_certificates_path = ExtractCertificatesToFileSystem(); if (extracted_ca_certificates_path.empty()) { @@ -213,7 +203,7 @@ void InstallCrashpadHandler(const CommandLine& command_line) { } third_party::crashpad::wrapper::InstallCrashpadHandler( - /*start_at_crash=*/true, extracted_ca_certificates_path); + extracted_ca_certificates_path); } #endif // SB_IS(EVERGREEN_COMPATIBLE) diff --git a/starboard/doc/crash_handlers.md b/starboard/doc/crash_handlers.md index c11516bd46a6..5f98f869f11b 100644 --- a/starboard/doc/crash_handlers.md +++ b/starboard/doc/crash_handlers.md @@ -32,7 +32,7 @@ const void* SbSystemGetExtension(const char* name) { } ``` -3. Calling the `third_party::crashpad::wrapper::InstallCrashpadHandler(bool start_at_crash)` hook +3. Calling the `third_party::crashpad::wrapper::InstallCrashpadHandler()` hook directly after installing system crash handlers. On linux, for example, this could look like: @@ -44,7 +44,8 @@ int main(int argc, char** argv) { starboard::shared::signal::InstallCrashSignalHandlers(); starboard::shared::signal::InstallSuspendSignalHandlers(); - third_party::crashpad::wrapper::InstallCrashpadHandler(true); + std::string ca_certificates_path = starboard::common::GetCACertificatesPath(); + third_party::crashpad::wrapper::InstallCrashpadHandler(ca_certificates_path); int result = application.Run(argc, argv); ... diff --git a/starboard/linux/shared/BUILD.gn b/starboard/linux/shared/BUILD.gn index 507a764834f4..7234deab6478 100644 --- a/starboard/linux/shared/BUILD.gn +++ b/starboard/linux/shared/BUILD.gn @@ -314,8 +314,6 @@ static_library("starboard_platform_sources") { "//starboard/shared/starboard/media/media_is_buffer_using_memory_pool.cc", "//starboard/shared/starboard/memory.cc", "//starboard/shared/starboard/queue_application.cc", - "//starboard/shared/starboard/starboard_switches.cc", - "//starboard/shared/starboard/starboard_switches.h", "//starboard/shared/starboard/string_duplicate.cc", "//starboard/shared/starboard/system_get_random_uint64.cc", "//starboard/shared/starboard/system_request_blur.cc", diff --git a/starboard/linux/x64x11/main.cc b/starboard/linux/x64x11/main.cc index 19293362b2ee..a6cb6cffa450 100644 --- a/starboard/linux/x64x11/main.cc +++ b/starboard/linux/x64x11/main.cc @@ -24,7 +24,6 @@ #include "starboard/common/paths.h" #include "starboard/elf_loader/elf_loader_constants.h" #include "starboard/shared/starboard/command_line.h" -#include "starboard/shared/starboard/starboard_switches.h" #endif #include "starboard/shared/starboard/link_receiver.h" #include "starboard/shared/x11/application_x11.h" @@ -53,13 +52,7 @@ extern "C" SB_EXPORT_PLATFORM int main(int argc, char** argv) { } #if !SB_IS(MODULAR) - bool start_handler_at_crash = - command_line.HasSwitch( - starboard::shared::starboard::kStartHandlerAtCrash) || - !command_line.HasSwitch( - starboard::shared::starboard::kStartHandlerAtLaunch); - third_party::crashpad::wrapper::InstallCrashpadHandler(start_handler_at_crash, - ca_certificates_path); + third_party::crashpad::wrapper::InstallCrashpadHandler(ca_certificates_path); #endif // !SB_IS(MODULAR) #endif diff --git a/starboard/raspi/shared/BUILD.gn b/starboard/raspi/shared/BUILD.gn index 39ca20e177c6..568dcb78fb38 100644 --- a/starboard/raspi/shared/BUILD.gn +++ b/starboard/raspi/shared/BUILD.gn @@ -282,8 +282,6 @@ static_library("starboard_platform_sources") { "//starboard/shared/starboard/memory.cc", "//starboard/shared/starboard/new.cc", "//starboard/shared/starboard/queue_application.cc", - "//starboard/shared/starboard/starboard_switches.cc", - "//starboard/shared/starboard/starboard_switches.h", "//starboard/shared/starboard/string_duplicate.cc", "//starboard/shared/starboard/system_get_random_uint64.cc", "//starboard/shared/starboard/system_request_blur.cc", diff --git a/starboard/raspi/shared/main.cc b/starboard/raspi/shared/main.cc index b07e33ef1299..27b0dff87f65 100644 --- a/starboard/raspi/shared/main.cc +++ b/starboard/raspi/shared/main.cc @@ -25,7 +25,6 @@ #include "starboard/common/paths.h" #include "starboard/elf_loader/elf_loader_constants.h" #include "starboard/shared/starboard/command_line.h" -#include "starboard/shared/starboard/starboard_switches.h" #endif #include "third_party/crashpad/wrapper/wrapper.h" @@ -51,13 +50,7 @@ int main(int argc, char** argv) { SB_LOG(ERROR) << "Failed to get CA certificates path"; } - bool start_handler_at_crash = - command_line.HasSwitch( - starboard::shared::starboard::kStartHandlerAtCrash) || - !command_line.HasSwitch( - starboard::shared::starboard::kStartHandlerAtLaunch); - third_party::crashpad::wrapper::InstallCrashpadHandler(start_handler_at_crash, - ca_certificates_path); + third_party::crashpad::wrapper::InstallCrashpadHandler(ca_certificates_path); #endif // SB_IS(EVERGREEN_COMPATIBLE) #if SB_API_VERSION >= 15 diff --git a/starboard/shared/starboard/starboard_switches.cc b/starboard/shared/starboard/starboard_switches.cc deleted file mode 100644 index ad4917e148dd..000000000000 --- a/starboard/shared/starboard/starboard_switches.cc +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2022 The Cobalt Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "starboard/shared/starboard/starboard_switches.h" - -namespace starboard { -namespace shared { -namespace starboard { - -const char kStartHandlerAtCrash[] = "start_handler_at_crash"; -const char kStartHandlerAtLaunch[] = "start_handler_at_launch"; - -} // namespace starboard -} // namespace shared -} // namespace starboard diff --git a/starboard/shared/starboard/starboard_switches.h b/starboard/shared/starboard/starboard_switches.h deleted file mode 100644 index 55fea4ec998a..000000000000 --- a/starboard/shared/starboard/starboard_switches.h +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2022 The Cobalt Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef STARBOARD_SHARED_STARBOARD_STARBOARD_SWITCHES_H_ -#define STARBOARD_SHARED_STARBOARD_STARBOARD_SWITCHES_H_ - -#include "starboard/configuration.h" - -namespace starboard { -namespace shared { -namespace starboard { - -// Start the crash handler only when a crash happens, instead of starting it -// before the app runs and keeping it running all the time. This option reduces -// memory consumption by the crash handler. -extern const char kStartHandlerAtCrash[]; -// Use this flag to start the handler -// before the app launches. Without this flag, the crash handler starts only -// when a crash happens. This option increases the memory consumption. -extern const char kStartHandlerAtLaunch[]; - -} // namespace starboard -} // namespace shared -} // namespace starboard - -#endif // STARBOARD_SHARED_STARBOARD_STARBOARD_SWITCHES_H_ diff --git a/starboard/stub/BUILD.gn b/starboard/stub/BUILD.gn index d85bef3de7be..db0ccd7dc6c3 100644 --- a/starboard/stub/BUILD.gn +++ b/starboard/stub/BUILD.gn @@ -41,8 +41,6 @@ static_library("stub_sources") { "//starboard/shared/starboard/file_mode_string_to_flags.cc", "//starboard/shared/starboard/memory.cc", "//starboard/shared/starboard/queue_application.cc", - "//starboard/shared/starboard/starboard_switches.cc", - "//starboard/shared/starboard/starboard_switches.h", "//starboard/shared/stub/accessibility_get_caption_settings.cc", "//starboard/shared/stub/accessibility_get_display_settings.cc", "//starboard/shared/stub/accessibility_get_text_to_speech_settings.cc", diff --git a/third_party/crashpad/wrapper/wrapper.cc b/third_party/crashpad/wrapper/wrapper.cc index 93eb5ae42d6a..e8da56a77533 100644 --- a/third_party/crashpad/wrapper/wrapper.cc +++ b/third_party/crashpad/wrapper/wrapper.cc @@ -189,8 +189,7 @@ std::map GetPlatformInfo() { } // namespace -void InstallCrashpadHandler(bool start_at_crash, - const std::string& ca_certificates_path) { +void InstallCrashpadHandler(const std::string& ca_certificates_path) { ::crashpad::CrashpadClient* client = GetCrashpadClient(); const base::FilePath handler_path = GetPathToCrashpadHandlerBinary(); @@ -225,24 +224,13 @@ void InstallCrashpadHandler(bool start_at_crash, client->SetUnhandledSignals({}); - if (start_at_crash) - client->StartHandlerAtCrash(handler_path, - database_directory_path, - default_metrics_dir, - kUploadUrl, - ca_certificates_path, - default_annotations, - default_arguments); - else - client->StartHandler(handler_path, - database_directory_path, - default_metrics_dir, - kUploadUrl, - ca_certificates_path, - default_annotations, - default_arguments, - false, - false); + client->StartHandlerAtCrash(handler_path, + database_directory_path, + default_metrics_dir, + kUploadUrl, + ca_certificates_path, + default_annotations, + default_arguments); ::crashpad::SanitizationInformation sanitization_info = {0, 0, 0, 1}; client->SendSanitizationInformationToHandler(sanitization_info); diff --git a/third_party/crashpad/wrapper/wrapper.h b/third_party/crashpad/wrapper/wrapper.h index 3a384ec693fb..d51c3d4320e6 100644 --- a/third_party/crashpad/wrapper/wrapper.h +++ b/third_party/crashpad/wrapper/wrapper.h @@ -36,14 +36,12 @@ extern const char kCrashpadUserAgentStringKey[]; extern const char kCrashpadCertScopeKey[]; // Installs a signal handler to handle a crash. The signal handler will launch a -// Crashpad handler process in response to a crash when |start_at_crash| is -// true, otherwise a Crashpad handler process will be started immediately. +// Crashpad handler process in response to a crash. // |ca_certificates_path| is the absolute path to a directory containing // Cobalt's trusted Certificate Authority (CA) root certificates, and must be // passed so that the certificates can be accessed by the handler process during // upload. -void InstallCrashpadHandler(bool start_at_crash, - const std::string& ca_certificates_path); +void InstallCrashpadHandler(const std::string& ca_certificates_path); bool AddEvergreenInfoToCrashpad(EvergreenInfo evergreen_info); diff --git a/third_party/crashpad/wrapper/wrapper_stub.cc b/third_party/crashpad/wrapper/wrapper_stub.cc index 46bc3fb8175e..8b6679679fff 100644 --- a/third_party/crashpad/wrapper/wrapper_stub.cc +++ b/third_party/crashpad/wrapper/wrapper_stub.cc @@ -23,8 +23,7 @@ const char kCrashpadProductKey[] = ""; const char kCrashpadUserAgentStringKey[] = ""; const char kCrashpadCertScopeKey[] = ""; -void InstallCrashpadHandler(bool start_at_crash, - const std::string& ca_certificates_path) {} +void InstallCrashpadHandler(const std::string& ca_certificates_path) {} bool AddEvergreenInfoToCrashpad(EvergreenInfo evergreen_info) { return false;