Fix SplashScreen crash

[Xiaobo-Wang-Amlogic]

No description provided.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[github-actions]

🤖 Gemini Suggested Commit Message

---

cobalt: Protect TokenStorage access

The TokenStorage::GetStorage method attempted an initial lock-free
read of existing tokens. This path was susceptible to data races when
multiple threads concurrently accessed and modified the storage.

Acquiring the lock before any access to the token storage ensures
thread-safety for both read and write operations. This prevents crashes
caused by race conditions, specifically observed during application
startup while processing various string tokens.

Bug: 9247

---

💡 Pro Tips for a Better Commit Message:

1. Influence the Result: Want to change the output? You can write custom prompts or instructions directly in the Pull Request description. The model uses that text to generate the message.
2. Re-run the Generator: Post a comment with: /generate-commit-message

[Xiaobo-Wang-Amlogic]

Do the stability test,find the Splashscreen coredump.
And then checked the coredump.
It seems as there is race condition.
Need use lock to protect the string compare at line 98

#0 __libc_do_syscall () at ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:47

#1 0xf4f2df98 in __pthread_kill_implementation (threadid=, signo=6, no_tid=) at pthread_kill.c:43

#2 0xf4effbf6 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26

#3 0xf4ef1614 in __GI_abort () at abort.c:79

#4 0x00a7eea6 in SbSystemBreakIntoDebugger ()

#5 0x00ada420 in starboard::shared::signal::(anonymous namespace)::Crash(int) ()

#6

#7 0xef6b7374 in ?? () at ../../third_party/musl/src/string/memcmp.c:6

#8 0xef2b10a0 in __constexpr_memcmp<char, char> () at ../../third_party/llvm-project/libcxx/include/__string/constexpr_c_functions.h:62

#9 compare () at ../../third_party/llvm-project/libcxx/include/__string/char_traits.h:219

#10 compare () at ../../third_party/llvm-project/libcxx/include/string:3663

#11 operator==<char, std::_LIBCPP_ABI_NAMESPACE::char_traits,
std::_LIBCPP_ABI_NAMESPACE::allocator > () at ../../third_party/llvm-project/libcxx/include/string:3836

#12 GetStorage () at ../../cobalt/base/token.cc:98

#13 0xef2b1508 in Token () at ../../cobalt/base/token.cc:78

#14 0xef4a8ce0 in RegisterHTMLElementWithSingleTagNamecobalt::dom::LottiePlayer () at ../../cobalt/dom/html_element_factory.cc:191

#15 HTMLElementFactory () at ../../cobalt/dom/html_element_factory.cc:163

#16 0xef4a846c in HTMLElementContext () at ../../cobalt/dom/html_element_context.cc:103

#17 0xef4ed374 in Window () at ../../cobalt/dom/window.cc:140

#18 0xef2a19d4 in Impl () at ../../cobalt/browser/web_module.cc:646

#19 0xef2a43c8 in InitializeTaskInThread () at ../../cobalt/browser/web_module.cc:1354

#20 0xef7a7804 in Run () at ../../base/functional/callback.h:333

#21 InitializeTaskInThread () at ../../cobalt/web/agent.cc:649

#22 0xef7a949c in Invoke<void (cobalt::web::Agent::*)(const cobalt::web::Agent::Options &, base::RepeatingCallback<void (cobalt::web::Context *)>), cobalt::web::Agent *, const cobalt::web::Agent::Options &, const base::RepeatingCallback<void (cobalt::web::Context *)> &> () at ../../base/functional/bind_internal.h:746

#23 MakeItSo<void (cobalt::web::Agent::*const &)(const cobalt::web::Agent::Options &, base::RepeatingCallback<void (cobalt::web::Context *)>), const std::_LIBCPP_ABI_NAMESPACE::tuple<base::internal::UnretainedWrapper<cobalt::web::Agent, base::unretained_traits::MayNotDangle, (base::RawPtrTraits)0>, cobalt::web::Agent::Options, base::RepeatingCallback<void (cobalt::web::Context *)> > &> () at ../../base/functional/bind_internal.h:925

#24 RunImpl<void (cobalt::web::Agent::*const &)(const cobalt::web::Agent::Options &, base::RepeatingCallback<void (cobalt::web::Context *)>), const std::_LIBCPP_ABI_NAMESPACE::tuple<base::internal::UnretainedWrapper<cobalt::web::Agent, base::unretained_traits::MayNotDangle, (base::RawPtrTraits)0>, cobalt::web::Agent::Options, base::RepeatingCallback<void (cobalt::web::Context *)> > &, 0U, 1U, 2U> () at ../../base/functional/bind_internal.h:1025

#25 Run () at ../../base/functional/bind_internal.h:989

#26 0xef51dc30 in Run () at ../../base/functional/callback.h:152

#27 RunTaskImpl () at ../../base/task/common/task_annotator.cc:259

[gemini-code-assist]

Code Review

This pull request aims to fix a crash by extending the scope of a lock in TokenStorage::GetStorage to prevent a race condition. While this approach effectively addresses the likely cause of the crash, it has introduced some issues. The code now contains outdated comments that are misleading. More importantly, there is a redundant check being performed while holding the lock, which impacts performance. My review includes suggestions to clean up the code for better maintainability and efficiency.

[gemini-code-assist]

Since the lock is now acquired before the first check (on line 91), this comment is misleading. More importantly, the subsequent check for the token's existence is redundant because it's already performed under the same lock. This leads to unnecessary work being done while the lock is held. Please remove this redundant check and its associated comments for improved performance and code clarity.

[gemini-code-assist]

With the lock now acquired on line 91, this comment stating the path is 'lock free' is incorrect. To avoid confusion, it should be removed.