Avoid possible reentrancy of ThemeChanged (see #3671)

The call to SelectNativeTheme from ChromeBrowserFrame::Initialized was
causing Widget::ThemeChanged reentrancy via OnColorProviderCacheResetMissed
when running with `--enable-chrome-runtime --use-native`. Make all calls to
ThemeChanged async to avoid this and possible future issues.
This commit is contained in:
Marshall Greenblatt 2024-04-14 21:34:45 -04:00
parent 81a0648ee1
commit 62c93f01f4
4 changed files with 23 additions and 7 deletions

View File

@ -5,6 +5,7 @@
#include "libcef/browser/chrome/views/chrome_browser_frame.h" #include "libcef/browser/chrome/views/chrome_browser_frame.h"
#include "libcef/browser/chrome/chrome_browser_host_impl.h" #include "libcef/browser/chrome/chrome_browser_host_impl.h"
#include "libcef/browser/thread_util.h"
#include "libcef/browser/views/window_view.h" #include "libcef/browser/views/window_view.h"
#include "chrome/browser/themes/theme_service.h" #include "chrome/browser/themes/theme_service.h"
@ -93,10 +94,7 @@ void ChromeBrowserFrame::UserChangedTheme(
// Calls ThemeChanged() and possibly SelectNativeTheme(). // Calls ThemeChanged() and possibly SelectNativeTheme().
BrowserFrame::UserChangedTheme(theme_change_type); BrowserFrame::UserChangedTheme(theme_change_type);
if (window_view_) { NotifyThemeColorsChanged(/*chrome_theme=*/!native_theme_change_);
window_view_->OnThemeColorsChanged(/*chrome_theme=*/!native_theme_change_);
ThemeChanged();
}
} }
views::internal::RootView* ChromeBrowserFrame::CreateRootView() { views::internal::RootView* ChromeBrowserFrame::CreateRootView() {
@ -156,8 +154,15 @@ void ChromeBrowserFrame::OnColorProviderCacheResetMissed() {
return; return;
} }
NotifyThemeColorsChanged(/*chrome_theme=*/false);
}
void ChromeBrowserFrame::NotifyThemeColorsChanged(bool chrome_theme) {
if (window_view_) { if (window_view_) {
window_view_->OnThemeColorsChanged(/*chrome_theme=*/false); window_view_->OnThemeColorsChanged(chrome_theme);
ThemeChanged();
// Call ThemeChanged() asynchronously to avoid possible reentrancy.
CEF_POST_TASK(TID_UI, base::BindOnce(&ChromeBrowserFrame::ThemeChanged,
weak_ptr_factory_.GetWeakPtr()));
} }
} }

View File

@ -9,6 +9,7 @@
#include "libcef/browser/views/color_provider_tracker.h" #include "libcef/browser/views/color_provider_tracker.h"
#include "libcef/browser/views/widget.h" #include "libcef/browser/views/widget.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/views/frame/browser_frame.h" #include "chrome/browser/ui/views/frame/browser_frame.h"
@ -136,6 +137,8 @@ class ChromeBrowserFrame : public BrowserFrame,
// CefColorProviderTracker::Observer methods: // CefColorProviderTracker::Observer methods:
void OnColorProviderCacheResetMissed() override; void OnColorProviderCacheResetMissed() override;
void NotifyThemeColorsChanged(bool chrome_theme);
CefWindowView* window_view_; CefWindowView* window_view_;
BrowserView* browser_view_ = nullptr; BrowserView* browser_view_ = nullptr;
@ -143,6 +146,8 @@ class ChromeBrowserFrame : public BrowserFrame,
bool native_theme_change_ = false; bool native_theme_change_ = false;
CefColorProviderTracker color_provider_tracker_{this}; CefColorProviderTracker color_provider_tracker_{this};
base::WeakPtrFactory<ChromeBrowserFrame> weak_ptr_factory_{this};
}; };
#endif // CEF_LIBCEF_BROWSER_CHROME_VIEWS_CHROME_BROWSER_FRAME_H_ #endif // CEF_LIBCEF_BROWSER_CHROME_VIEWS_CHROME_BROWSER_FRAME_H_

View File

@ -4,6 +4,7 @@
#include "libcef/browser/views/widget_impl.h" #include "libcef/browser/views/widget_impl.h"
#include "libcef/browser/thread_util.h"
#include "libcef/browser/views/window_view.h" #include "libcef/browser/views/window_view.h"
#include "libcef/features/runtime.h" #include "libcef/features/runtime.h"
@ -226,7 +227,9 @@ void CefWidgetImpl::NotifyThemeColorsChanged(bool chrome_theme,
if (window_view_) { if (window_view_) {
window_view_->OnThemeColorsChanged(chrome_theme); window_view_->OnThemeColorsChanged(chrome_theme);
if (call_theme_changed) { if (call_theme_changed) {
ThemeChanged(); // Call ThemeChanged() asynchronously to avoid possible reentrancy.
CEF_POST_TASK(TID_UI, base::BindOnce(&CefWidgetImpl::ThemeChanged,
weak_ptr_factory_.GetWeakPtr()));
} }
} }
} }

View File

@ -11,6 +11,7 @@
#include "libcef/browser/views/color_provider_tracker.h" #include "libcef/browser/views/color_provider_tracker.h"
#include "libcef/browser/views/widget.h" #include "libcef/browser/views/widget.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/themes/theme_service_observer.h" #include "chrome/browser/themes/theme_service_observer.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
@ -91,6 +92,8 @@ class CefWidgetImpl : public views::Widget,
ProfileMap associated_profiles_; ProfileMap associated_profiles_;
CefColorProviderTracker color_provider_tracker_{this}; CefColorProviderTracker color_provider_tracker_{this};
base::WeakPtrFactory<CefWidgetImpl> weak_ptr_factory_{this};
}; };
#endif // CEF_LIBCEF_BROWSER_VIEWS_WIDGET_IMPL_H_ #endif // CEF_LIBCEF_BROWSER_VIEWS_WIDGET_IMPL_H_