Autoroll from main to staging#10786
Conversation
…updater APIs Refer to original PR: #10666 evergreen: Implement updater H5vcc APIs This change implements the H5vcc updater APIs in H5vccUpdaterImpl. Previously, these APIs were stubbed out with NOTIMPLEMENTED() calls. Now, for Evergreen builds, these methods delegate to the UpdaterModule, providing concrete functionality for managing updater channels, installation status, and other related update features. This enables the browser to expose full updater capabilities via the H5vcc API. Bug: 454440974 Bug: 515122610 Co-authored-by: Madhura Jayaraman <madjay@google.com> (cherry picked from commit 4087dbd)
…g in mojo for sideloading APIs Refer to original PR: #10667 Evergreen: Isolate updater sideloading configuration Move updater configuration setters to a dedicated sideloading Mojo interface. This separates sensitive update settings, such as allowing self-signed packages, update server URL, and network encryption requirements, into an interface only bound in non-release Evergreen builds with sideloading enabled. This prevents unauthorized modification of update settings in production environments. Bug: 454440974 Bug: 515122610 Co-authored-by: Madhura Jayaraman <madjay@google.com> (cherry picked from commit 51eebc4)
Refer to original PR: #10668 Comprehensive web platform tests are added for all H5vccUpdater methods, including specific tests that verify promise rejection upon Mojo connection errors. Bug: 454440974 Bug: 515122610 --------- Co-authored-by: Madhura Jayaraman <madjay@google.com> (cherry picked from commit 7041d5d)
…5vccUpdater binders Refer to original PR: #10694 Register dummy binders for H5vccUpdater and H5vccUpdaterSideloading Mojo interfaces in builds where they are not natively supported (e.g., non-Evergreen or Linux-x64). Previously, JavaScript probing for these interfaces would result in a ReportBadMessage error if no binder was registered. This error caused the BrowserInterfaceBroker (Mojo Master Pipe) to disconnect for the entire frame. This disconnection prevented subsequent interface requests, such as MediaDevicesDispatcherHost for getUserMedia(), from reaching the browser process, leading to microphone recognition failures. Registering dummy binders ensures these probes fail gracefully without severing the Mojo connection. Bug: 479656309 Bug: 515122610 Co-authored-by: thorsten sideb0ard <sideboard@google.com> (cherry picked from commit 976e786)
Refer to original PR: #10323 The DecoderBuffer allocator now utilizes a configurable decommit strategy. This strategy allows for a tiered approach to memory management when the buffer pool is idle. It defines distinct regions for memory to be retained, conservatively decommitted (e.g., using MADV_FREE), or aggressively decommitted (e.g., using MADV_DONTNEED). This refactoring replaces the old fixed decommitable allocator strategy, enabling more granular control and flexibility for memory management experiments. The new strategy is configured via the h5vcc setting DecoderBuffer.EnableConfigurableDecommitStrategy, which encodes unit size, retain size, and conservative decommit size into a single integer. Bug: 454441375 (cherry picked from commit c131bd9)
Refer to original PR: #10566 starboard/RDK: Use gold linker instead of the default ld. The RDK platform now explicitly uses the gold linker instead of the default linker. This change addresses linker errors[1] encountered while building the loader_app using the RDK7 toolchain. The gold linker is also known to be faster, uses less memory, and more efficient than the default ld. [1] ``` aarch64-rdk-linux/usr/lib/libEGL.so: .dynsym local symbol at index 3 (>= sh_info of 3) aarch64-rdk-linux/usr/lib/libEGL.so: .dynsym local symbol at index 4 (>= sh_info of 3) arch64-rdk-linux/usr/lib/libEGL.so: .dynsym local symbol at index 5 (>= sh_info of 3) collect2: error: ld returned 1 exit status ``` Bug: 510541109 Change-Id: I5fbb070079c6727e456da1e22f72bec68f6de0b5 (cherry picked from commit d0bba59)
…size" Refer to original PR: #10693 This PR relands #10649, which was reverted due to 3P builds crashing #10649 (comment) which occurred due to a uncaught `nullptr` in `cobalt_module_stubs.cc`'s `static const WrapperTypeInfo g_dummy_wrapper_type_info` which has been replaced with a `DummyInstallInterfaceTemplateFunc` (which is just a dummy function that does nothing) and pass it instead of nullptr . This safely satisfies the V8 bindings system without introducing any WebXR dependency. Original PR Description --------------------------------------------------------------- This PR decouples Cobalt's dependency on WebXR, which is important in pruning Dawn/WebGPU. This is Step 2 of 3 (See step 1 #10644) to safely remove the heavy Dawn/WebGPU dependencies and achieve a significant binary size reduction on Android (~800 KB total). Pruning WebXR on top of WebNN shows an additional binary size decrease of ~278.5 KB (93,093,829 to 92,815,301 bytes), bringing total cumulative savings to ~557 KB. #### Key Changes: 1. Build Graph Pruning (GN Subtraction): • Subtracted //third_party/blink/renderer/modules/xr from sub_modules in modules/BUILD.gn . • Gated the xr dependency behind !is_cobalt in third_party/blink/renderer/modules/video_rvfc/BUILD. • Subtracted the xr dependency in third_party/blink/renderer/modules/webgl/BUILD.gn for Cobalt builds to ensure WebGL remains fully functional without WebXR. 2. Bindings Filtering: • Added cobalt_webxr_exclude_patterns to bindings.gni to target *v8_xr.* and *v8_xr_* . • Filtered out generated WebXR V8 bindings in bindings/modules/v8/BUILD.gn (concatenated with cobalt_webnn_exclude_patterns ) to prevent their compilation into libv8.a . 3. Lightweight C++ Stubbing & Best Practices: • Appended WebXR stubs (V8 wrappers and C++ entry points like XRSystem::From() returning nullptr ) to cobalt_modules_stubs.cc . Bug: 512589073 (cherry picked from commit 69056ee)
Refer to original PR: #10183 This adds wrappers for `inotify_init`, `inotify_init1`, `inotify_add_watch`, and `inotify_rm_watch` Linux syscalls. Chromium uses these functions in several places, e.g. to watch for changes to the DNS config or timezone. Bug: 500417827 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> (cherry picked from commit 1c0e681)
…helper functions Refer to original PR: #10684 Renames the internal helper functions `SbThreadPriorityToRelativeSchedPriority` and `SbPriorityToNice` in `starboard/common/thread` to clarify that they are internal C++ utilities rather than public Starboard C APIs. ### Rationale Following the deprecation and cleanup of public Starboard C threading APIs, some internal helper functions remained in `starboard/common` that still carried the legacy C-style `Sb` prefix (e.g., `SbPriorityToNice`). This could easily lead to the false impression that these helpers are part of the public Starboard API surface. This PR renames them to clarify they are strictly private/internal Starboard C++ utilities and updates all internal callers accordingly. Issue: 517310205 (cherry picked from commit 3c3d60e)
…rtup Refer to original PR: #10664 Adds a mechanism to clear stale lock files on startup to prevent permanent storage failures caused by improper shutdowns. See [b/488465561#comment19](https://b.corp.google.com/issues/488465561#comment19) and [b/488465561#comment20](https://b.corp.google.com/issues/488465561#comment20) Bug: 488465561 (cherry picked from commit 1f80bee)
…tion_unittests Refer to original PR: #10708 Enable `cobalt_memory_instrumentation_unittests` on Linux, Android, and Evergreen. This unittest suite was introduced as part of the smaps parsing refactoring for Cobalt. See PR #10135 for its introduction. Bug: 498702469 (cherry picked from commit 6be88b3)
…laceholders Refer to original PR: #10517 Remove unneeded not_empty.txt placeholders from //cobalt/android:coat_assets. Since the target was not using renaming_sources/destinations, both paths were flattened to assets/not_empty.txt. In addition, since the logo file was already present at assets/, none of them are actually needed. Bug: 492403745 (cherry picked from commit a85452f)
Refer to original PR: #10470 Reduce unnecessary logging. Since the recv call uses the MSG_DONTWAIT flag, it will return immediately if no data is available, setting errno to EAGAIN or EWOULDBLOCK . This is a normal flow control condition, not an actual error that requires logging. Bug: 506953317 --------- Co-authored-by: Oscar Vestlie <oxv@google.com> (cherry picked from commit 8431c2d)
Refer to original PR: #10552 Deep link topic detection currently only evaluates standard query parameters. This update enables the identification of launch and topic parameters within the URL fragment section. This change ensures that deep links formatted with fragments are correctly processed, allowing for the appropriate topic-specific splash screen to be loaded during initialization. Same as #10553. Issue: 489756553 (cherry picked from commit 1daa340)
Refer to original PR: #10675 The AudioTimeStretcher component requires audio data to be in a float32 interleaved format. This update ensures that the audio renderer always resamples decoded audio to this format, which prevents compatibility issues and eliminates the need for additional conversions later in the pipeline. Bug: 515433551 (cherry picked from commit f548207)
Refer to original PR: #10717 starboard: Remove SbThreadId and thread.h ### Deprecated/Removed Starboard APIs - **`SbThreadId`** (Typedef): Deprecated in `starboard/thread.h`. Internal usages migrated to POSIX `pid_t`. - **`kSbThreadInvalidId`** (Macro): Removed from `starboard/thread.h`. Replaced with `0` in migrated code. - **`SbThreadIsValidId`** (Inline function): Removed from `starboard/thread.h`. Replaced with standard comparison (`!= 0`) in migrated code. ### Deprecate starboard/thread.h - **` starboard/thread.h`** : After removing above 3 Starboard API's , deprecate starboard/thread.h and remove all call sites where it is referenced. Issue: 457772688 Issue: 517657866 (cherry picked from commit d9f3e6e)
…ng and Allocator Observers" Refer to original PR: #10723 Reverts #10539 Reverts this build failure: ``` 2026-05-28T17:52:01.3576033Z [15828/42371] 2m38.61s F CXX obj/base/base/cobalt_memory_attribution_observer.o 2026-05-28T17:52:01.3576751Z stdout: 2026-05-28T17:52:01.3578166Z ../../base/memory/cobalt_memory_attribution_observer.cc:37:9: warning: 'pthread_getname_np' is only available on Android 26 or newer [-Wunguarded-availability] 2026-05-28T17:52:01.3579848Z 37 | if (pthread_getname_np(pthread_self(), tls_thread_name, sizeof(tls_thread_name)) != 0) { 2026-05-28T17:52:01.3580788Z | ^~~~~~~~~~~~~~~~~~ 2026-05-28T17:52:01.3582225Z ../../starboard/android/shared/posix_emu/include/pthread.h:25:5: note: 'pthread_getname_np' has been marked as being introduced in Android 26 here, but the deployment target is Android 24 2026-05-28T17:52:01.3584076Z 25 | int pthread_getname_np(pthread_t thread, char* name, size_t len); 2026-05-28T17:52:01.3584764Z | ^ 2026-05-28T17:52:01.3585953Z ../../base/memory/cobalt_memory_attribution_observer.cc:37:9: note: enclose 'pthread_getname_np' in a __builtin_available check to silence this warning 2026-05-28T17:52:01.3587539Z 37 | if (pthread_getname_np(pthread_self(), tls_thread_name, sizeof(tls_thread_name)) != 0) { 2026-05-28T17:52:01.3588646Z | ^~~~~~~~~~~~~~~~~~ 2026-05-28T17:52:01.3589040Z 38 | tls_thread_name[0] = '\0'; 2026-05-28T17:52:01.3589425Z 39 | return nullptr; 2026-05-28T17:52:01.3589756Z 40 | } 2026-05-28T17:52:01.3590032Z | 2026-05-28T17:52:01.3590323Z 1 warning generated. 2026-05-28T17:52:01.3590598Z 2026-05-28T17:52:01.3590603Z 2026-05-28T17:52:01.3622934Z [15829/42371] 2m38.62s F CXX obj/third_party/webrtc_overrides/task_queue_factory/low_precision_timer.o 2026-05-28T17:52:01.3630007Z [15830/42371] 2m38.62s F CXX obj/skia/skia/DrawAtlasPathOp.o ``` Bug: 498702469 (cherry picked from commit 1c44efa)
There was a problem hiding this comment.
Code Review
This pull request removes deprecated Starboard APIs (such as SbFileAtomicReplace and SbThreadId) in favor of standard POSIX equivalents, introduces a configurable decommit strategy for the decoder buffer allocator, adds a feature to delete stale local storage lock files, and separates sideloading updater APIs into a new Mojo interface. Feedback on these changes highlights a potential temporary file leak on failure in FileAtomicReplace, redundant decommit system calls when reclaiming already decommitted blocks, a missing validation check for zero block size in H5vccSettings, and a style guide violation where auto is used instead of auto* for pointers.
| if (file_exists && unlink(path)) { | ||
| return false; | ||
| } | ||
| if (rename(temp_path.data(), path) != 0) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
If unlink(path) or rename fails, the temporary file created at temp_path is leaked on the disk. We should unlink temp_path on failure to prevent temporary file accumulation and potential disk space exhaustion.
if (file_exists && unlink(path)) {
unlink(temp_path.data());
return false;
}
if (rename(temp_path.data(), path) != 0) {
unlink(temp_path.data());
return false;
}| void ReuseAllocatorBase::ReclaimFallbackBlocks() { | ||
| SB_LOG(INFO) << "Allocator reached idle state, reclaiming " | ||
| << fallback_allocations_.size() << " fallback allocations."; | ||
| for (const auto& fallback_allocation : fallback_allocations_) { | ||
| fallback_allocator_->Decommit(fallback_allocation.address, | ||
| fallback_allocation.size); | ||
| allocation_counter_ = 0; | ||
| for (size_t i = 0; i < fallback_allocations_.size(); ++i) { | ||
| auto& fallback_block = fallback_allocations_[i]; | ||
| if (i < retain_blocks_) { | ||
| fallback_block.state = kIdleRetained; | ||
| } else if (i < retain_blocks_ + conservative_decommit_blocks_) { | ||
| fallback_block.state = kIdlePendingFree; | ||
| has_pending_decommits_ = true; | ||
| } else { | ||
| fallback_block.state = kIdlePendingDecommit; | ||
| has_pending_decommits_ = true; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
When reclaiming fallback blocks, if a block is already in a decommitted state (kIdleDecommitted or kIdleFreed), setting its state back to kIdlePendingDecommit or kIdlePendingFree and setting has_pending_decommits_ = true will cause the allocator to redundantly call Decommit (which invokes madvise) on it again. This introduces unnecessary system call overhead every time the allocator transitions to idle. We should only transition blocks to pending decommit states if they are not already decommitted.
void ReuseAllocatorBase::ReclaimFallbackBlocks() {
SB_LOG(INFO) << "Allocator reached idle state, reclaiming "
<< fallback_allocations_.size() << " fallback allocations.";
allocation_counter_ = 0;
for (size_t i = 0; i < fallback_allocations_.size(); ++i) {
auto& fallback_block = fallback_allocations_[i];
if (i < retain_blocks_) {
fallback_block.state = kIdleRetained;
} else if (i < retain_blocks_ + conservative_decommit_blocks_) {
if (fallback_block.state != kIdleFreed &&
fallback_block.state != kIdleDecommitted) {
fallback_block.state = kIdlePendingFree;
has_pending_decommits_ = true;
}
} else {
if (fallback_block.state != kIdleDecommitted) {
fallback_block.state = kIdlePendingDecommit;
has_pending_decommits_ = true;
}
}
}
}| return ProcessSettingAsPositiveInt( | ||
| script_state, exception_context, name, *value, [](int value) { | ||
| bool aggressive_decommit_on_suspend = ((value >> 24) & 0xFF) != 0; | ||
| int block_size_mb = (value >> 16) & 0xFF; | ||
| int retain_blocks = (value >> 8) & 0xFF; | ||
| int conservative_decommit_blocks = value & 0xFF; | ||
| ::media::DecoderBufferAllocator::EnableConfigurableDecommitStrategy( | ||
| block_size_mb * 1024 * 1024, retain_blocks, | ||
| conservative_decommit_blocks, aggressive_decommit_on_suspend); | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
If block_size_mb is extracted as 0 (which can happen if the packed integer has 0 in the corresponding byte, even if the overall integer is positive), it will result in a block size of 0 being passed to EnableConfigurableDecommitStrategy. This can cause division-by-zero or other undefined behaviors in the allocator. We should explicitly validate that block_size_mb > 0 and return false if it is not.
return ProcessSettingAsPositiveInt(
script_state, exception_context, name, *value, [](int value) {
bool aggressive_decommit_on_suspend = ((value >> 24) & 0xFF) != 0;
int block_size_mb = (value >> 16) & 0xFF;
if (block_size_mb == 0) {
return false;
}
int retain_blocks = (value >> 8) & 0xFF;
int conservative_decommit_blocks = value & 0xFF;
::media::DecoderBufferAllocator::EnableConfigurableDecommitStrategy(
block_size_mb * 1024 * 1024, retain_blocks,
conservative_decommit_blocks, aggressive_decommit_on_suspend);
return true;
});| void H5vccUpdaterSideloadingImpl::GetUpdateServerUrl( | ||
| GetUpdateServerUrlCallback callback) { | ||
| DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); | ||
| auto updater_module = cobalt::updater::UpdaterModule::GetInstance(); |
There was a problem hiding this comment.
According to the Chromium style guide, we should use auto* instead of auto for pointers to keep them clearly visible as pointers. This also maintains consistency with the other methods in this file.
| auto updater_module = cobalt::updater::UpdaterModule::GetInstance(); | |
| auto* updater_module = cobalt::updater::UpdaterModule::GetInstance(); |
References
- All code submitted to this repository must adhere to the established upstream Chromium style guides. Chromium style guide recommends using auto* for pointers. (link)
Automated cherry-pick roll to staging.
Original pull requests:
alarm#10705