Skip to content

Commit

Permalink
Remove the option to start crash handler at launch (#2387)
Browse files Browse the repository at this point in the history
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 1c7af63)
  • Loading branch information
yuying-y authored and anonymous1-me committed Apr 3, 2024
1 parent a99c34e commit 2aa7a44
Show file tree
Hide file tree
Showing 14 changed files with 75 additions and 126 deletions.
58 changes: 58 additions & 0 deletions starboard/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<sys/time.h>` and
`clock_gettime` from `<time.h>` 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 `<stdlib.h>` 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 `<sys/mman.h>` 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 `<stdlib.h>` and `strdup` from `<string.h>` 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
Expand Down
2 changes: 0 additions & 2 deletions starboard/android/shared/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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 += [
Expand Down
12 changes: 1 addition & 11 deletions starboard/android/shared/android_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()) {
Expand All @@ -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)

Expand Down
5 changes: 3 additions & 2 deletions starboard/doc/crash_handlers.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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);
...
Expand Down
2 changes: 0 additions & 2 deletions starboard/linux/shared/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 1 addition & 8 deletions starboard/linux/x64x11/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down
2 changes: 0 additions & 2 deletions starboard/raspi/shared/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 1 addition & 8 deletions starboard/raspi/shared/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
26 changes: 0 additions & 26 deletions starboard/shared/starboard/starboard_switches.cc

This file was deleted.

37 changes: 0 additions & 37 deletions starboard/shared/starboard/starboard_switches.h

This file was deleted.

2 changes: 0 additions & 2 deletions starboard/stub/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
28 changes: 8 additions & 20 deletions third_party/crashpad/wrapper/wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,7 @@ std::map<std::string, std::string> 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();
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions third_party/crashpad/wrapper/wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions third_party/crashpad/wrapper/wrapper_stub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 2aa7a44

Please sign in to comment.