Autoroll from main to 27.lts#10042
Open
cobalt-github-releaser-bot wants to merge 130 commits into27.ltsfrom
Open
Autoroll from main to 27.lts#10042cobalt-github-releaser-bot wants to merge 130 commits into27.ltsfrom
cobalt-github-releaser-bot wants to merge 130 commits into27.ltsfrom
Conversation
…ismatch in copy_from Refer to original PR: #9671 Cherry-picks crrev.com/c/6660778. Original commit message follows. Bug: 375232937 --- When using base::span::copy_from, a CHECK failure would occurif the source and destination sizes did not match. Avoids a crash when copying IA descriptors and frame buffer by using split_at() to separate the destination buffer before copying. R=dalecurtis Bug: None Change-Id: I8a7d1b35ee5fd11956321d36b2bbe60a22a76741 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6660778 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Thomas Guilbert <tguilbert@chromium.org> Commit-Queue: Thomas Guilbert <tguilbert@chromium.org> Cr-Commit-Position: refs/heads/main@{#1483337} Co-authored-by: Dongheun Kang <dongheun.kang@lge.com> (cherry picked from commit 0284a96)
Refer to original PR: #9329 starboard: Fix gtest main for Starboard tests Modify the gtest_main target to conditionally use starboard/common/test_main.cc for Starboard builds. This change ensures that Starboard-specific initialization and the event loop are properly set up for test executables. Without this, tests running on Starboard platforms would use the generic gtest_main, leading to "sbeventhandle errors" when trying to interact with uninitialized Starboard services. This resolves failures observed in Opus benchmark tests requiring Starboard's event handler. Issue: 486053350 Issue: 489195504 (cherry picked from commit 82ca770)
Refer to original PR: #9677 Reorganize some global build variables to enable setting `is_starboard` and `build_with_separate_cobalt_toolchain` to true for AOSP builds. Remove unnecessary import statements that result from `is_androidtv` being now defined globally. Bug: 495203133 (cherry picked from commit d677d66)
…ecycle and window management Refer to original PR: #9589 Refer to the original PR: #9423 Introduce the core infrastructure for handling application lifecycle events and enhance Starboard window management robustness. With this commit, conceal, reveal, focus, and blur functionality is fully operational with respect to lazy Starboard window creation/destruction and the corresponding Web API focus and visibility states. Key Changes: - Implement static entry points in content::Shell for lifecycle events (Blur, Focus, Conceal, Reveal, Freeze, Unfreeze, Stop). - Add virtual handlers to ShellPlatformDelegate to propagate these events. - Implement explicit focus/blur propagation to WebContents in ShellPlatformDelegate (OnFocus calls WebContents::Focus, OnBlur calls RenderWidgetHost::Blur). - Update AppLifecycleDelegate to bridge global Starboard events to the newly added Shell handlers. - Enhance PlatformWindowStarboard to robustly manage window creation, destruction, and visibility transitions, preventing redundant events. - Map SIGWINCH to the Blur event in the POSIX signal handler to support integrated lifecycle testing. - Introduce test_lifecycle.sh and enhance test_preload.sh to verify Visibility and Focus API states using the Chrome DevTools Protocol (CDP). - Fix cyclic dependencies in Starboard Ozone by removing redundant ui/aura and ui/compositor inclusions. - Ensure proper widget activation and visibility management in DesktopWindowTreeHostPlatform for Starboard. - Enhance AppLifecycleDelegateTest to verify WebContents focus state transitions. All changes are verified with unit tests and automated integration scripts on linux-x64x11-modular. Bug: 448156280 --------- Co-authored-by: Jelle Foks <jfoks@google.com> (cherry picked from commit 0eb792f)
Refer to original PR: #9358 cobalt: Add CPU.Total.Usage UMA Introduce a CobaltCpuMetricsEmitter class responsible for periodically fetching and emitting cobalt's total CPU usage as a UMA histogram. This emitter is integrated into the CobaltMetricsServiceClient, ensuring CPU usage metrics are regularly collected alongside existing memory metrics. This change provides valuable telemetry on system resource utilization, aiding in performance monitoring and analysis. Bug: 487654198 (cherry picked from commit 661b733)
…lt builds Refer to original PR: #9686 The TTS fix (b/491841704) broke the RDK executable build. As a temporary workaround, this change disables speech synthesis support when building as an executable. Plugin builds continue to use the existing implementation. Bug: 491841704 Bug: 495477543 (cherry picked from commit 7f749d1)
Refer to original PR: #9647 #9611 added comments to clarify what |kMaxEgVersionLength| and the |version_length| parameter to loader_app::ReadEvergreenVersion() represent. But I think it would help even more to use more idiomatic names that indicate this constant and parameter are capacity sizes of buffers intended to store C style strings. #vibe-coded Issue: 493915202 (cherry picked from commit e8e3fd4)
Refer to original PR: #9656 This change introduces fragmentation metrics for BlinkGC in Cobalt's memory metrics emitter. It calculates fragmentation by subtracting the allocated objects size from the effective size for both the browser and renderer processes. Specifically: - Adds BlinkGC.Fragmentation and BlinkGC.Main.Heap.Fragmentation metrics. - Updates tests to verify the new metrics are correctly recorded. Test: `out/android-x86_devel/bin/run_cobalt_unittests -v -f "*RecordMemoryMetricsRecordsHistogram*"` Bug: 492201832 (cherry picked from commit 42598dd)
Refer to original PR: #9720 Add a new comment to the MediaCodecVideoDecoder::Reset() method. This clarifies the expected behavior of the input_buffer_written_ member variable following a Flush() operation. It explains that while the value may not be strictly accurate in counting buffers after a flush, its accumulated state correctly signals the post-initial pre-roll phase and is acceptable for pre-roll frame estimation. This improves code readability and maintainability by documenting a potentially confusing aspect of the video decoder's state management. Issue: 495813142 (cherry picked from commit 9caf318)
Refer to original PR: #9555 1. Simplify the entrance of cobalt_browsertests to bin/run_cobalt_browsertests like Android build. 2. Switch the build target in CI to cobalt_browsertests. The runner python script now is only an implementation and should be invisible to users and CI. 3. Correct the CI config to remove cobalt_browsertests from bots running unit tests. cobalt_browsertests now only runs on its dedicated workflow. Test: out/linux-x64x11_devel/bin/run_cobalt_browsertests --gtest_filter=H5vccSchemeURLLoaderFactoryBrowserTest.* Issue: 463991461 (cherry picked from commit 9b6ccaf)
…Client, enable by default Refer to original PR: #9627 Set `initialize_crashpad` to true by default, and add code to provide the default crash dump location on tvOS in CobaltCrashReporterClient (a subdirectory called "Crashpad" inside the app's cache directory). It is also possible to override this directory with the `--crash-dumps-dir` command-line argument, which is useful for debugging. Bug: 477518757 (cherry picked from commit e097732)
… metrics Refer to original PR: #9660 This change adds tracking for the Proportional Set Size (PSS) and Resident Set Size (RSS) of the libchrobalt.so library on Android and Linux platforms. These metrics are collected by parsing /proc/self/smaps and are reported as new histograms: - Memory.Browser.LibChrobaltPss - Memory.Browser.LibChrobaltRss This provides visibility into the fixed memory cost of the Cobalt engine's code and constant data resident in RAM. Test: `out/${PLAT}_${BUILD}/bin/run_cobalt_browsertests -f "*CobaltMetricsBrowserTest*"` Bug: 494004530 (cherry picked from commit a1dee31)
Refer to original PR: #9674 cobalt: Add splash screen performance tests Measure content fetch time in H5vcc scheme browser tests. This change instruments the H5vccSchemeURLLoaderFactoryCacheBrowserTest to record the duration of fetching cached content. Performance metrics are reported via Test::RecordProperty and printed to stdout, enabling automated performance monitoring and regression detection for the splash screen's content loading. Perf results are stored in the xml file like: ``` <?xml version='1.0' encoding='UTF-8'?> <testsuites tests="1" failures="0" disabled="0" errors="0" time="0" name="AllTests"><testsuite name="H5vccSchemeURLLoaderFactoryCacheBrowserTest" tests="1" failures="0" disabled="0" skipped="0" errors="0" time="0.287" timestamp="2026-03-25T12:16:30.345"> <testcase name="LoadSplashVideoFromDefaultCache" value_param="" type_param="" file="../../cobalt/shell/browser/h5vcc_scheme_url_loader_factory_browsertest.cc" line="372" status="run" result="completed" time="0.287" timestamp="2026-03-25T12:16:30.345" classname="H5vccSchemeURLLoaderFactoryCacheBrowserTest"> <properties> <property name="CacheReadTime_ms" value="9" /> <property name="CacheReadTime_Trace" value="default" /> </properties> </testcase> </testsuite> </testsuites> ``` Bug: 489501943 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> (cherry picked from commit eab41f5)
Refer to original PR: #9729 cobalt: Log splash screen built-in as info Previously, when the splash screen cache missed and the built-in splash screen was used as a fallback, an ERROR log was generated. This state is a valid fallback mechanism and not an error condition. Adjust the log level to INFO for this scenario to avoid misleading error reports. Bug: 490510288 (cherry picked from commit 7dbd232)
…links Refer to original PR: #9750 * Check out the existing remote autoroll branch to continue from the latest state on the remote. * Return all PRs cherry-picked onto the autoroll branch but not yet in the target LTS branch to keep the PR description up-to-date. * Adds list of specific commits to skip for certain LTS branches. Bug: 484351320 (cherry picked from commit 17ef287)
Refer to original PR: #9382 This essentially means triggering the processing of any intermediate crash dumps and later uploading them to the Crashpad server once the app's UI finishes loading. The code is based on the `ios/` implementation, especially `ios/chrome/browser/crash_report/model/crash_helper.mm` (in particular the way that the steps above are triggered only once and in a separate thread when a scene finishes activating). Integration with Cobalt's Crash Log Web API is done in #9408. Bug: 477518757 (cherry picked from commit 9c9a460)
Refer to original PR: #9641 On tvOS, the H5vccSchemeURLLoaderFactory browser test invokes Application::Get() internally. Without an initialized Application instance during browser test setup, this leads to an assertion failure. This change creates a dummy Application instance for testing specifically on tvOS, preventing the crash and allowing the test to execute correctly. Bug: 489556129 (cherry picked from commit 2d59808)
…data cache Refer to original PR: #9629 Refer to the original PR: #9031 Introduce a caching mechanism for Skia font file data. When enabled via a RefDefault parameter, Skia's file stream creation uses a global cache, mapping font file paths to their memory-mapped SkData. This reduces redundant I/O and mapping for frequently accessed font files. NOTE: the passing of command line parameter from Java is not cherry-picked over as we don't plan to cherry-pick the experiment setup PRs from 26.android to main. Bug: 479948202 --------- Co-authored-by: xiaomings <xiaomings@google.com> (cherry picked from commit 9330385)
Refer to original PR: #9453 This PR refactors SbEventHandle() of cobalt_browsertests, to be compatible with Evergreen build. 1. Move initialization under SbEventHandle(), as in Evergreen mode the test entrance is SbEventHandle() not main(). 2. Replace TerminateCurrentProcessImmediately with _Exit() to avoid crashing in Starboard turn down after the test has done. Follow up PRs will try to build the test runner script into the loader.py. TEST: out/evergreen-x64_devel/cobalt_browsertests_loader.py \ --single-process-tests \ --no-sandbox --single-process \ --no-zygote --ozone-platform=starboard \ --gtest_filter=ContentMainRunnerImplBrowserTest.StartupSequence Issue: 463991461 (cherry picked from commit 5f5c472)
Refer to original PR: #9759 Use correct Git user config to pass CLA check. Bonus: * Remove 27.lts from label cherry pick workflow while autoroller is active. * Change trigger to push event to cherry pick immediately. * Remove parent pick. Merge commits must be applied without cherry picking and should be on the skip list. Bug: 484351320 (cherry picked from commit a479b97)
…lush timing Refer to original PR: #9669 Refer to the original PR: #9646 This change bring the cookie flush delays to be the same as the local storage and closer to the policy we had in Cobalt. The delay is 1s and the delays in Cobalt was between 0.5 to 2s. Issue: 482074098 Co-authored-by: Yavor Goulishev <yavor@google.com> (cherry picked from commit 45a5e29)
…fo and AdvertisingId leaks Refer to original PR: #10000 Refer to the original PR: #9089 Ensure that executor services in ClientLogInfo and AdvertisingId are properly shut down when their respective components are closed or when the application is being destroyed. This prevents thread leaks and ensures all resources are released, improving application stability and preventing potential ANRs on shutdown. Bug: 484183546 --------- Co-authored-by: Jason <jasonzh@google.com> (cherry picked from commit 4de1203)
…fferAllocator strategies Refer to original PR: #9997 Refer to the original PR: #9250 This change refactors how DecoderBufferAllocator strategies are managed. It removes explicit Enable methods and a StrategyType enum from media/base/decoder_buffer.h. Instead, strategy instantiation logic is moved into a StrategyCreateCB lambda factory. DecoderBufferAllocator now accepts this lambda to update its strategy. This allows concrete strategy implementations to be injected from Starboard-specific contexts, such as h5vcc_settings.cc, without polluting the core media/base interface. It also exposes DecoderBuffer::Allocator::Get() for global access to the allocator. This improves modularity and testability of buffer allocation logic. Bug: 454441375 --------- Co-authored-by: Kyujung Youn <kjyoun@google.com> (cherry picked from commit fbbc55b)
…ze linear state machine Refer to original PR: #9698 Refer to the original PR: #9424 Cobalt Lifecycle: Formalize linear state machine With this change, platforms that may send events at unexpected times compared to other events, Cobalt will now behave in a predictable and tested way. This replaces `app_lifecycle_delegate` with `app_event_delegate` and `app_event_runner`, which has better naming but more importantly it has robust handling of event sequencing. The sequence handling is kept in `app_event_delegate`, while `app_event_runner` contains the calls for the actions to be taken on incoming system events. In addition to allowing much better handling of input event sequence edge cases, this split also allows thorough testing. In detail, this implements a formal, linear state machine for managing the application lifecycle in ShellPlatformDelegate, ensuring predictable and traceable state transitions. Key Changes: - Introduce the ApplicationState enum (Started, Blurred, Concealed, Frozen, Stopped) to track the high-level application lifecycle. - Implement TransitionToState(state) logic that enforces a linear traversal of states, ensuring all intermediate side effects are triggered exactly once. - Refactor lifecycle logic into granular "Do*" side-effect methods (DoBlur, DoConceal, DoFreeze, etc.) for better maintainability and testability. - Enhance integration tests (test_lifecycle.sh and test_preload.sh) to verify internal state machine log sequences (e.g., ensuring Blur always precedes Conceal). - Update MockShellPlatformDelegate to support base class delegation in unit tests, enabling verification of state machine transitions while mocking individual side effects. - Ensure proper state initialization in platform-specific delegates (Android, Aura, Views, iOS). - Update IntegratedBlurFocus unit test to verify WebContents focus state transitions during Blur/Focus events. This formalization provides a robust framework for handling complex application state transitions consistently across all Cobalt platforms. Bug: 448156280 --------- Co-authored-by: Jelle Foks <jfoks@google.com> (cherry picked from commit 167cd91)
…oderInitialPrerollCount experimental feature Refer to original PR: #9998 Refer to the original PR: #9298 This change introduces an experimental configuration setting to customize the initial number of frames the video decoder must buffer (preroll) before playback begins on Android. This allows for fine-tuning of startup latency and stability. This re-applies the logic from ce5045e using the current plumbing. Bug: 487097162 Co-authored-by: Kyujung Youn <kjyoun@google.com> (cherry picked from commit b24113e)
…8130: Port storage flush calls to AppEventRunnerImpl lifecycle Refer to original PR: #10010 Refer to the original PR: #9700 Ensures that cookies and LocalStorage are explicitly flushed when the application transitions to the 'Blurred' (Paused) or 'Frozen' states. This is critical for data persistence on platforms where the OS may kill the application shortly after these transitions. Issue: 371433332 Bug: 451215656 Bug: 450936106 Bug: 444451968 Co-authored-by: Jelle Foks <jfoks@google.com> (cherry picked from commit 60e24cb)
…c consider pending input Refer to original PR: #10013 Refer to the original PR: #9385 Modify video preroll completion criteria to consider both the number of input buffers provided to the decoder and the count of decoded frames. Previously, preroll completed solely based on the number of decoded frames. This change introduces new configuration options to require a minimum number of input buffers alongside decoded frames, offering a more robust and flexible preroll readiness check. This helps ensure sufficient data has been processed before playback begins. Local test showed, by considering pending input, we can achieve the better Time-to-first-frame time. For the test result, see http://b/485225923#comment2 Bug: 485225923 Co-authored-by: Kyujung Youn <kjyoun@google.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> (cherry picked from commit ed779ce)
…ngBytesPerParse to h5vcc Refer to original PR: #10002 Differences From Original: Additional logic was added to account for builds that do not use a starboard layer. Buildflags like USE_STARBOARD_MEDIA have been added, along with corresponding error messages if the flag is not being used. Refer to the original PR: #9114 Expose the StreamParser's kMaxPendingBytesPerParse value for runtime configuration. This is achieved by defining a new override value, `g_max_pending_bytes_per_parse`. A new h5vcc_settings API entry, Media.ExperimentalMaxPendingBytesPerParse, allows dynamic tuning of the maximum bytes the stream parser inspects per parse call. The internal value is updated to std::atomic<int> to support runtime modification, guarded by BUILDFLAG(USE_STARBOARD_MEDIA). Bug: 471099187 --------- Co-authored-by: John Ocampo <johneocampo@google.com> (cherry picked from commit f9ecf0d)
Refer to original PR: #9980 Adds unit tests for the MediaCodecOutputTracker class to validate its memory estimation and metric reporting logic. test: ``` autoninja -C out/android-arm_devel cobalt_coat_junit_tests out/android-arm_devel/bin/run_cobalt_coat_junit_tests -f "dev.cobalt.media.MediaCodecOutputTrackerTest.*" -v ``` Bug: 500811958 (cherry picked from commit 1eb8a7e)
…y net error code Refer to original PR: #9952 The PR adds a new UMA `Cobalt.WebContentsObserver.FailedNavigationError`. This provides more detailed breakdown of navigation failures in the primary main frame based on `net_error_code`, which helps in understanding and prioritizing errors that appear more commonly for further investigation on network issues. Bug: 496950342 (cherry picked from commit 420a44b)
…ut storageDirectoryAtURL Refer to original PR: #9904 Follow-up to #9840. There is one call that only occurs in device builds that was triggering a warning and now causing the build to fail: the `storageDirectoryAtURL` parameter must not be nil when specified. Switch to the other method signature that does not take this parameter instead. Bug: 432508012 (cherry picked from commit db6253c)
…sistence Refer to original PR: #9681 Refer to the original PR: #9673 The underlying leveldb backend is asynchronous by default, which means that after power off the storage may not be actually persisted. Turing the sync options to true explicitly guaranties the flush would happen. Issue: 494696660 Co-authored-by: Yavor Goulishev <yavor@google.com> (cherry picked from commit 107e1c5)
…heckout Refer to original PR: #9989 Install fuse-overlayfs to manage the cobalt checkout and file fetched by gclient. As changed files are stored in the upper path via overlay, we can simply restore the runner cache to its initial state(checked out at TOT of Cobalt). So that the next run could start from the same clean cache. Also correct the runner for cobalt browser tests job, which should also use chrobalt-linux-runner like the unit test jobs. Bug: 501486531 (cherry picked from commit c948b4c)
…derer preroll parameters from H5VCC settings Refer to original PR: #10015 Refer to the original PR: #9403 Introduce H5VCC settings for video renderer minimum input buffers and minimum decoded frames. These parameters are plumbed through the media and Starboard layers, including updates to the Starboard VideoDecoderConfiguration extension API, ultimately reaching the VideoRendererImpl. This change enables the dynamic configuration of video preroll settings from H5VCC, laying the groundwork for future platform- specific tuning to improve video playback startup and buffer management. Bug: 491104896 --------- Co-authored-by: Kyujung Youn <kjyoun@google.com> (cherry picked from commit cf67085)
… for idle buffer memory Refer to original PR: #10016 Refer to the original PR: #9286 Introduce a mechanism to release physical memory associated with decoder buffers when their allocation pools become idle. This change adds a Decommit method to the common allocator interface, which is implemented by StarboardMemoryAllocator using madvise(MADV_DONTNEED). The ReuseAllocatorBase and InPlaceReuseAllocatorBase now invoke this Decommit method when all internally managed blocks are freed or during batched free operations. This effectively signals to the operating system that the physical pages backing these buffers are no longer actively needed, allowing them to be reclaimed. A new DecoderBufferAllocator strategy, enabled via H5VCC settings, configures the media pipeline to utilize this decommit-capable allocator, reducing Cobalt's resident set size and improving overall memory efficiency when media playback is not active. Bug: 454441375 --------- Co-authored-by: xiaomings <xiaomings@google.com> Co-authored-by: Kyujung Youn <kjyoun@google.com> (cherry picked from commit 879d26b)
…formance software video decoders Refer to original PR: #10007 Refer to the original PR: #7732 1. Reject low performance software decoders if software codec is not explicitly required. 2. Update MediaCodecUtil.isSoftwareDecoder() to match latest Chromium implementation. Bug: 435204402 --------- Co-authored-by: Jason <jasonzh@google.com> (cherry picked from commit 153198c)
…arnings Refer to original PR: #9920 This commit addresses compiler warnings related to signed/unsigned comparison mismatches and API usage in Widevine and tvOS DRM implementations exposed after treat_warnings_as_errors was enabled in GN configuration (83fedaf). Specifically, it caches subsample_count as size_t in Widevine's decrypt loop, casts content.size() for SbFile::WriteAll comparison, and casts offset+recordSize to NSUInteger in tvOS DRM session parsing. It also corrects AVContentKeySession initialization and removes trailing semicolons from macro invocations. Bug: 471365322 (cherry picked from commit eb0eaaf)
…default Refer to original PR: #10009 Refer to the original PR: #9500 Disable PartitionAllocBackupRefPtr (BRP) as a default feature override for Cobalt on Android. This change explicitly adds BRP to the list of features disabled by default, ensuring it is not active unless re-enabled. The previous JavaSwitches mechanism for controlling BRP has been removed as it is no longer required due to the new default state. Bug: 481385937 --------- Co-authored-by: Jason <jasonzh@google.com> (cherry picked from commit a76d7f1)
…xperimental feature into a struct Refer to original PR: #10022 Refer to the original PR: #9327 Consolidate various experimental player features into a single PlayerComponents::ExperimentalFeatures struct. This refactoring groups parameters such as flush_decoder_during_reset, reset_audio_decoder, and video decoder input/preroll counts. This change improves code organization by reducing the number of individual parameters in constructors and centralizing the management of experimental player settings. Bug: 480134029 --------- Co-authored-by: Kyujung Youn <kjyoun@google.com> (cherry picked from commit c920d18)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements a Starboard-to-Chromium storage migration pipeline, refactors application lifecycle management via a new event delegate, and introduces flexible media buffer allocation strategies. It also adds CPU metrics polling and various robustness fixes. Feedback highlights several issues in the migration manager, including a missing safety timeout for local storage injection, synchronous file I/O on the UI thread, potential memory overflows with large records, and incorrect URL formatting for IPv6-based cookies. A concern regarding duplicate iterator erasure in the callback list refactor was also raised.
Comment on lines
+743
to
+820
| Task MigrationManager::LocalStorageTask( | ||
| content::StoragePartition* partition, | ||
| const url::Origin& origin, | ||
| std::vector<std::unique_ptr<std::pair<std::string, std::string>>> pairs, | ||
| scoped_refptr<MigrationState> state) { | ||
| return base::BindOnce( | ||
| [](content::StoragePartition* partition, url::Origin origin, | ||
| std::vector<std::unique_ptr<std::pair<std::string, std::string>>> | ||
| pairs, | ||
| scoped_refptr<MigrationState> state, base::OnceClosure callback) { | ||
| if (pairs.empty()) { | ||
| LOG(INFO) << "No LocalStorage pairs, finishing."; | ||
| std::move(callback).Run(); | ||
| return; | ||
| } | ||
|
|
||
| // Shared context to keep the mojo::Remote alive until the entire | ||
| // pipeline finishes. | ||
| auto shared_state = base::MakeRefCounted<SharedClosureState>(); | ||
| shared_state->cb = std::move(callback); | ||
|
|
||
| GetLocalStorageArea(partition, origin, shared_state->storage_area); | ||
|
|
||
| // STEP 2: Flush step. | ||
| // Tells the Storage Service to commit the LevelDB transactions to disk. | ||
| // We use BindOnce with shared_state to ensure it is kept alive. | ||
| base::OnceClosure flush_step = base::BindOnce( | ||
| [](content::StoragePartition* partition, | ||
| scoped_refptr<MigrationState> state, | ||
| scoped_refptr<SharedClosureState> shared_state) { | ||
| LOG(INFO) << "LocalStorage Puts complete. Flushing..."; | ||
| partition->GetLocalStorageControl()->Flush(); | ||
|
|
||
| // Proceed immediately. Flush() is non-blocking and we don't need | ||
| // to wait for disk IO to complete before starting the app. | ||
| shared_state->Run(); | ||
| }, | ||
| base::Unretained(partition), state, shared_state); | ||
|
|
||
| // STEP 1: Put keys step. | ||
| // Iterates through all k/v pairs and sends Mojo Put commands. | ||
| // The `flush_step` runs only when the barrier closes (all Puts have | ||
| // responded). | ||
| base::RepeatingClosure barrier = | ||
| base::BarrierClosure(pairs.size(), std::move(flush_step)); | ||
|
|
||
| for (const auto& pair : pairs) { | ||
| std::string key_str = pair->first; | ||
| std::vector<uint8_t> key = FormatStringForLocalStorage(pair->first); | ||
| std::vector<uint8_t> value = | ||
| FormatStringForLocalStorage(pair->second); | ||
|
|
||
| LOG(INFO) << "Put for key: " << key_str; | ||
| shared_state->storage_area->Put( | ||
| key, value, absl::nullopt, "migration", | ||
| base::BindOnce( | ||
| [](base::RepeatingClosure barrier, std::string key_str, | ||
| scoped_refptr<MigrationState> state, | ||
| scoped_refptr<SharedClosureState> keep_alive, | ||
| bool success) { | ||
| if (success) { | ||
| LOG(INFO) << "Put SUCCESS for key: " << key_str; | ||
| } else { | ||
| LOG(ERROR) << "Put FAILED for key: " << key_str; | ||
| } | ||
| InjectionResult inj_result = success | ||
| ? InjectionResult::kSuccess | ||
| : InjectionResult::kError; | ||
| base::UmaHistogramEnumeration( | ||
| kLocalStorageInjectionResultHistogram, inj_result); | ||
| state->UpdateLocalStorageResult(inj_result); | ||
| barrier.Run(); | ||
| }, | ||
| barrier, key_str, state, shared_state)); | ||
| } | ||
| }, | ||
| partition, origin, std::move(pairs), state); | ||
| } |
Contributor
There was a problem hiding this comment.
Unlike CookieTask, LocalStorageTask lacks a timeout mechanism. If the Chromium Storage Service hangs or fails to respond to the Mojo Put calls, the migration pipeline will block indefinitely, preventing the application from starting. Given that this is a critical path for app launch, a safety timeout (similar to the 2-second timeout in CookieTask) should be implemented to ensure app startup resumes even if LocalStorage injection stalls.
Comment on lines
+352
to
+358
| for (auto pending_it : pending_erasures_) { | ||
| // Note: `pending_it` was originally an iterator into `callbacks_`, but | ||
| // the node it points to has just been spliced into `null_callbacks_`. The | ||
| // iterator itself remains valid and can now be used for erasure from | ||
| // `null_callbacks_`. | ||
| null_callbacks_.erase(pending_it); | ||
| } |
Contributor
There was a problem hiding this comment.
The loop over pending_erasures_ assumes that each iterator in the vector is still valid and points to a node that was successfully spliced into null_callbacks_. While std::list::splice preserves iterator validity, if CancelCallback were called multiple times for the same subscription during a single Notify cycle, pending_erasures_ would contain duplicate iterators. The second attempt to erase the same iterator from null_callbacks_ would result in undefined behavior. Although subscriptions are typically destroyed once, adding a check or using a container that handles duplicates would be more robust.
| if (!base::PathService::Get(base::DIR_CACHE, ¤t_cache_path)) { | ||
| LOG(ERROR) << "Failed to get current cache directory. Skipping " | ||
| "deletion of old cache path."; | ||
| return; |
Contributor
There was a problem hiding this comment.
The ReadAndParseLegacyStorageRecord function performs synchronous file I/O (record->Read) on the UI thread. While this occurs during the PreMainMessageLoopRun phase, it can still cause noticeable startup delays if the legacy storage record is large or if the disk is slow. This operation should ideally be moved to a background task in the ThreadPool.
| return; | ||
| } | ||
|
|
||
| auto bytes = std::vector<uint8_t>(record->GetSize()); |
Contributor
There was a problem hiding this comment.
The allocation of the bytes vector uses record->GetSize(), which returns an int64_t. On 32-bit platforms, this could lead to an overflow if not properly cast to size_t. Additionally, there is no upper bound check on the size, which could lead to Out-Of-Memory (OOM) issues if the legacy storage file is corrupted or unexpectedly large. Consider using base::checked_cast<size_t> and enforcing a reasonable maximum size limit.
| if (!host.empty() && host[0] == '.') { | ||
| host = host.substr(1); | ||
| } | ||
| GURL source_url("https://" + host + cookie->Path()); |
Contributor
There was a problem hiding this comment.
When constructing source_url, the host string is derived from cookie->Domain(). If the domain is an IPv6 address (e.g., ::1), it must be enclosed in square brackets to form a valid GURL (e.g., https://[::1]/). net::CanonicalCookie::Domain() typically returns the raw IP string without brackets, which will result in an invalid source_url and cause SetCanonicalCookie to fail.
…ngs processing Refer to original PR: #10023 Refer to the original PR: #9191 This PR is to avoid repeated code by introducing helper methods Introduce a ProcessLongAs template helper to consolidate redundant V8 type checking and promise rejection logic for boolean settings. This streamlines the validation and conversion of V8UnionLongOrString values, simplifying H5vccSettings::set and ProcessDecoderBufferSettings. It also standardizes promise handling. Issue: 480134029 ---- Differences From The Original PR: - Added connection for DecommitableAllocatorStrategy. This makes cherry-picking #9462 unnecessary. - We cannot use `SettingContext` as the original commit does. main branch does not allow getting raw pointer to garbage-collectable memory - Help method takes a callback method returning boolean, since on main, we have to handle non-starboard build --------- Co-authored-by: Kyujung Youn <kjyoun@google.com> (cherry picked from commit d70c34a)
linxinan-chops
approved these changes
Apr 15, 2026
…media_codec_bridge_ in callbacks Refer to original PR: #10031 Refer to the original PR: #9691 Avoid a potential race condition during MediaDecoder destruction. Remove the usage of `media_codec_bridge_` in `OnMediaCodecOutputFormatChanged`, which was only used for logging purposes. This prevents a crash if the callback is invoked after the bridge has been destroyed. Bug: 495444400 --------- Co-authored-by: Jason <jasonzh@google.com> (cherry picked from commit cd91608)
…lifecycle and focus reporting issues Refer to original PR: #10011 Refer to the original PR: #9798 This PR significantly hardens the Cobalt application lifecycle, specifically addressing synchronous blocking requirements during downward state transitions and improving the robustness of the message pump for nested loops. #### Key Improvements * **MessagePumpUIStarboard nested loop support:** Implemented the previously missing `Run()` method (which formerly threw a `NOTREACHED()`) to support a full Chromium-style message loop. This is critical for enabling nested `base::RunLoop` calls, allowing the main OS thread to block during teardown while the UI thread continues to process asynchronous tasks (e.g., surface destruction). The loop blocks highly efficiently using `base::WaitableEvent::TimedWait()`. * **Synchronous State Transitions:** Refactored `AppEventDelegate` to explicitly block the Starboard OS thread during downward transitions (Blur, Conceal, Freeze) until the target state is reached and all asynchronous side effects are completed. This uses a nested `RunLoop` on the UI thread and dynamically instantiated `WaitableEvent`s on non-UI threads. Added a 5-second safety timeout (`kTransitionTimeout`) to prevent permanent Starboard thread hangs. * **Stability & Thread Safety:** * **Deadlock Prevention:** The final `kStopped` state transition (which destroys the `SequenceManager`) is now safely executed *synchronously* outside of the nested `RunLoop`. Attempting to tear down the task queue from within one of its own tasks would cause a fatal deadlock. * Replaced `ContentMain` with `RunContentProcess` using a process-lifetime static `ContentMainRunner` to resolve SIGSEGV issues during shutdown. * Introduced `base::Lock` and `std::atomic` state variables in `AppEventDelegate` and `AppEventRunner` to serialize lifecycle event processing and ensure thread safety across multiple threads without lock contention. * Improved `MessagePumpUIStarboard` destruction (calling `CancelAll()` and `AfterRun()`) to prevent `DCHECK` crashes in `SequenceManager` during teardown. * **Visibility & Focus Reporting:** * Added explicit calls to `web_contents()->WasHidden()` and `WasShown()` in `ShellPlatformDelegate` to ensure the compositor and Aura correctly track visibility changes. * **Deterministic Focus:** Implemented a robust focus deferral mechanism in `cobalt::Shell`. Focus requests arriving while the window is hidden are queued and applied precisely when Aura signals visibility via `OnVisibilityChanged`. * **Robust Integration Tests:** Rewrote `test_lifecycle.sh` and `test_preload.sh` to include precise process cleanup validation (handling zombie states) and a new `wait_for_state.sh` helper that polls for JS state via DevTools, eliminating race conditions previously handled by brittle `sleep` commands. Bug: 492166511 Bug: 491841704 Bug: 475990372 Bug: 477170017 Bug: 474496110 Bug: 492693465 Bug: 494002968 Co-authored-by: Jelle Foks <jfoks@google.com> (cherry picked from commit a793a7c)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated cherry-pick roll to 27.lts.
Original pull requests:
SbAudioSinkCreateTest.MultiSink(#9243) #9761