crash in BrowserFrame::OnTouchUiChanged()

[misros]

Hello Marshal,,
In our shipped product, we are seeing the following crash with CEF 129.0.12+gf09539f+chromium-129.0.6668.101 in the main browser thread - 'CrBrowserMain'. This is on Windows OS.
Logs indicate the users are using a touch screen. I haven't yet tried to reproduce this but does this seem familiar? Has it been fixed in recent CEF versions?

-SM

> libcef.dll!views::View::InvalidateLayout() Line 951 C++ Symbols loaded.
libcef.dll!BrowserFrame::OnTouchUiChanged() Line 636 C++ Symbols loaded.
[Inline Frame] libcef.dll!base::RepeatingCallback<void ()>::Run() Line 344 C++ Symbols loaded.
[Inline Frame] libcef.dll!base::RepeatingCallbackList<void ()>::RunCallback(std::__Cr::__list_iterator<base::RepeatingCallback<void ()>,void *> it) Line 328 C++ Symbols loaded.
libcef.dll!base::internal::CallbackListBase<base::RepeatingCallbackList<void ()>>::Notify<>() Line 217 C++ Symbols loaded.
libcef.dll!ui::TouchUiController::TouchUiChanged() Line 233 C++ Symbols loaded.
[Inline Frame] libcef.dll!base::OnceCallback<void ()>::Run() Line 156 C++ Symbols loaded.
libcef.dll!base::TaskAnnotator::RunTaskImpl(base::PendingTask & pending_task) Line 203 C++ Symbols loaded.
[Inline Frame] libcef.dll!base::TaskAnnotator::RunTask(perfetto::StaticString event_name, base::PendingTask & pending_task, base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl::<lambda_4> && args) Line 90 C++ Symbols loaded.
[Inline Frame] libcef.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow * continuation_lazy_now) Line 484 C++ Symbols loaded.
libcef.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() Line 346 C++ Symbols loaded.
libcef.dll!base::MessagePumpForUI::DoRunLoop() Line 260 C++ Symbols loaded.
libcef.dll!base::MessagePumpWin::Run(base::MessagePump::Delegate * delegate) Line 85 C++ Symbols loaded.
libcef.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool application_tasks_allowed, base::TimeDelta timeout) Line 657 C++ Symbols loaded.
libcef.dll!base::RunLoop::Run(const base::Location & location) Line 136 C++ Symbols loaded.
libcef.dll!CefMainRunner::RunMessageLoop() Line 198 C++ Symbols loaded.
libcef.dll!CefUIThread::ThreadMain() Line 106 C++ Symbols loaded.
libcef.dll!base::`anonymous namespace'::ThreadFunc(void * params=0x00000cc000054780) Line 124 C++ Symbols loaded.
kernel32.dll!BaseThreadInitThunk() Unknown Symbols loaded.
ntdll.dll!RtlUserThreadStart() Unknown Symbols loaded.

[ndesktop]

You need this patch on browser_frame.cc

Code: Select all

 void BrowserFrame::OnTouchUiChanged() {
-  client_view()->InvalidateLayout();
-
-  // For standard browser frame, if we do not invalidate the NonClientFrameView
-  // the client window bounds will not be properly updated which could cause
-  // visual artifacts. See crbug.com/1035959 for details.
-  if (non_client_view()->frame_view()) {
-    // Note that invalidating a view invalidates all of its ancestors, so it is
-    // not necessary to also invalidate the NonClientView or RootView here.
-    non_client_view()->frame_view()->InvalidateLayout();
-  } else {
-    non_client_view()->InvalidateLayout();
+  if (client_view()) {
+    client_view()->InvalidateLayout();
+  }
+
+  if (non_client_view()) {
+    // For standard browser frame, if we do not invalidate the NonClientFrameView
+    // the client window bounds will not be properly updated which could cause
+    // visual artifacts. See crbug.com/1035959 for details.
+    if (non_client_view()->frame_view()) {
+      // Note that invalidating a view invalidates all of its ancestors, so it is
+      // not necessary to also invalidate the NonClientView or RootView here.
+      non_client_view()->frame_view()->InvalidateLayout();
+    } else {
+      non_client_view()->InvalidateLayout();
+    }
+  }
+  views::View* root_view = GetRootView();
+  if (root_view != nullptr) {
+    root_view->InvalidateLayout();
   }
-  GetRootView()->InvalidateLayout();
 }


client_view()->InvalidateLayout();
client_view() might be null.

[misros]

Thanks. I don't compile chromium code and just use whatever comes down with CEF distribution.
Is this patch available in any new version of CEF?

Thanks
SM

[ndesktop]

I don't know for sure (I am currently on CEF 130 and it wasn't). But the issue was on Chromium and I remember it was fixed, but they switched to another issue system and I can't find it right now.

[misros]

I don't see the fix implemented even in 135.0.7026.1:

https://chromium.googlesource.com/chrom ... r_frame.cc

Can this be fixed please?

Thanks
SM

[ndesktop]

That is the Chromium source code. CEF patches normally do not deal with Chromium bugs. It is also possible Chromium modified substantially the area so the crash do not occur anymore (although I doubt this, I have seen several such cases where my custom patches were rendered obsolete).
Anyways, maybe Marshall can fill an issue if this affects CEF heavily (since usually he does this when CEF is faulted by Chromium), or you can fill an issue yourself (and perhaps link with existing issues, since this touch crash appeared in the past).