Commit a2d0668d authored by Jiaqi Han's avatar Jiaqi Han Committed by Commit Bot

[Chromecast] Fix Lifetime issue between CastWindowManager and CastBrowserProcess

CastWindowManager are used in chromecast code with the assumption
that it outlives the browser process code, meanwhile
CastWindowManager is reset before CastBrowserProcess, causing
lifetime issues.

Since CastWindowManager is owned by CastBrowserMainPart and is
created before CastBrowserProcess, the reasonable fix is to swap
the order to reset CastWindowManager after CastBrowserProcess.

Because CastScreen is used by WindowManager during tear down,
its ownership is moved to CastBrowserMainPart. This fixes
cast_shell_browsertests.

Bug: b/144778077, b/131312670
Test: CQ
Change-Id: Iee03600138c3d7d9324ba3971d1aad816239115b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928897Reviewed-by: default avatarSean Topping <seantopping@chromium.org>
Reviewed-by: default avatarDaniel Nicoara <dnicoara@chromium.org>
Commit-Queue: Jiaqi Han <jiaqih@google.com>
Cr-Commit-Position: refs/heads/master@{#720282}
parent 188aea8a
...@@ -478,12 +478,12 @@ int CastBrowserMainParts::PreCreateThreads() { ...@@ -478,12 +478,12 @@ int CastBrowserMainParts::PreCreateThreads() {
->TakePrefService()); ->TakePrefService());
#if defined(USE_AURA) #if defined(USE_AURA)
cast_browser_process_->SetCastScreen(std::make_unique<CastScreen>()); cast_screen_ = std::make_unique<CastScreen>();
cast_browser_process_->SetCastScreen(cast_screen_.get());
DCHECK(!display::Screen::GetScreen()); DCHECK(!display::Screen::GetScreen());
display::Screen::SetScreenInstance(cast_browser_process_->cast_screen()); display::Screen::SetScreenInstance(cast_screen_.get());
cast_browser_process_->SetDisplayConfigurator( cast_browser_process_->SetDisplayConfigurator(
std::make_unique<CastDisplayConfigurator>( std::make_unique<CastDisplayConfigurator>(cast_screen_.get()));
cast_browser_process_->cast_screen()));
#endif // defined(USE_AURA) #endif // defined(USE_AURA)
content::ChildProcessSecurityPolicy::GetInstance()->RegisterWebSafeScheme( content::ChildProcessSecurityPolicy::GetInstance()->RegisterWebSafeScheme(
...@@ -724,12 +724,16 @@ void CastBrowserMainParts::PostMainMessageLoopRun() { ...@@ -724,12 +724,16 @@ void CastBrowserMainParts::PostMainMessageLoopRun() {
// Android does not use native main MessageLoop. // Android does not use native main MessageLoop.
NOTREACHED(); NOTREACHED();
#else #else
window_manager_.reset();
cast_browser_process_->cast_service()->Finalize(); cast_browser_process_->cast_service()->Finalize();
cast_browser_process_->cast_browser_metrics()->Finalize(); cast_browser_process_->cast_browser_metrics()->Finalize();
cast_browser_process_.reset(); cast_browser_process_.reset();
window_manager_.reset();
#if defined(USE_AURA)
display::Screen::SetScreenInstance(nullptr);
cast_screen_.reset();
#endif
#if !defined(OS_FUCHSIA) #if !defined(OS_FUCHSIA)
DeregisterKillOnAlarm(); DeregisterKillOnAlarm();
#endif // !defined(OS_FUCHSIA) #endif // !defined(OS_FUCHSIA)
......
...@@ -40,6 +40,7 @@ class WaylandServerController; ...@@ -40,6 +40,7 @@ class WaylandServerController;
#if defined(USE_AURA) #if defined(USE_AURA)
class CastWindowManagerAura; class CastWindowManagerAura;
class CastScreen;
#else #else
class CastWindowManager; class CastWindowManager;
#endif // #if defined(USE_AURA) #endif // #if defined(USE_AURA)
...@@ -96,6 +97,7 @@ class CastBrowserMainParts : public content::BrowserMainParts { ...@@ -96,6 +97,7 @@ class CastBrowserMainParts : public content::BrowserMainParts {
#if defined(USE_AURA) #if defined(USE_AURA)
std::unique_ptr<views::ViewsDelegate> views_delegate_; std::unique_ptr<views::ViewsDelegate> views_delegate_;
std::unique_ptr<CastScreen> cast_screen_;
std::unique_ptr<CastWindowManagerAura> window_manager_; std::unique_ptr<CastWindowManagerAura> window_manager_;
#else #else
std::unique_ptr<CastWindowManager> window_manager_; std::unique_ptr<CastWindowManager> window_manager_;
......
...@@ -43,7 +43,12 @@ CastBrowserProcess* CastBrowserProcess::GetInstance() { ...@@ -43,7 +43,12 @@ CastBrowserProcess* CastBrowserProcess::GetInstance() {
} }
CastBrowserProcess::CastBrowserProcess() CastBrowserProcess::CastBrowserProcess()
: cast_content_browser_client_(nullptr), :
#if defined(USE_AURA)
cast_screen_(nullptr),
#endif
web_view_factory_(nullptr),
cast_content_browser_client_(nullptr),
net_log_(nullptr) { net_log_(nullptr) {
DCHECK(!g_instance); DCHECK(!g_instance);
g_instance = this; g_instance = this;
...@@ -84,10 +89,9 @@ void CastBrowserProcess::SetCastService( ...@@ -84,10 +89,9 @@ void CastBrowserProcess::SetCastService(
} }
#if defined(USE_AURA) #if defined(USE_AURA)
void CastBrowserProcess::SetCastScreen( void CastBrowserProcess::SetCastScreen(CastScreen* cast_screen) {
std::unique_ptr<CastScreen> cast_screen) {
DCHECK(!cast_screen_); DCHECK(!cast_screen_);
cast_screen_ = std::move(cast_screen); cast_screen_ = cast_screen;
} }
void CastBrowserProcess::SetDisplayConfigurator( void CastBrowserProcess::SetDisplayConfigurator(
......
...@@ -63,7 +63,7 @@ class CastBrowserProcess { ...@@ -63,7 +63,7 @@ class CastBrowserProcess {
void AccessibilityStateChanged(bool enabled); void AccessibilityStateChanged(bool enabled);
#endif // BUILDFLAG(ENABLE_CHROMECAST_EXTENSIONS) #endif // BUILDFLAG(ENABLE_CHROMECAST_EXTENSIONS)
void SetCastScreen(std::unique_ptr<CastScreen> cast_screen); void SetCastScreen(CastScreen* cast_screen);
void SetDisplayConfigurator( void SetDisplayConfigurator(
std::unique_ptr<CastDisplayConfigurator> display_configurator); std::unique_ptr<CastDisplayConfigurator> display_configurator);
#endif // defined(USE_AURA) #endif // defined(USE_AURA)
...@@ -85,7 +85,7 @@ class CastBrowserProcess { ...@@ -85,7 +85,7 @@ class CastBrowserProcess {
CastBrowserContext* browser_context() const { return browser_context_.get(); } CastBrowserContext* browser_context() const { return browser_context_.get(); }
CastService* cast_service() const { return cast_service_.get(); } CastService* cast_service() const { return cast_service_.get(); }
#if defined(USE_AURA) #if defined(USE_AURA)
CastScreen* cast_screen() const { return cast_screen_.get(); } CastScreen* cast_screen() const { return cast_screen_; }
CastDisplayConfigurator* display_configurator() const { CastDisplayConfigurator* display_configurator() const {
return display_configurator_.get(); return display_configurator_.get();
} }
...@@ -115,7 +115,7 @@ class CastBrowserProcess { ...@@ -115,7 +115,7 @@ class CastBrowserProcess {
// Note: The following order should match the order they are set in // Note: The following order should match the order they are set in
// CastBrowserMainParts. // CastBrowserMainParts.
#if defined(USE_AURA) #if defined(USE_AURA)
std::unique_ptr<CastScreen> cast_screen_; CastScreen* cast_screen_;
std::unique_ptr<CastDisplayConfigurator> display_configurator_; std::unique_ptr<CastDisplayConfigurator> display_configurator_;
#if BUILDFLAG(ENABLE_CHROMECAST_EXTENSIONS) #if BUILDFLAG(ENABLE_CHROMECAST_EXTENSIONS)
...@@ -129,7 +129,7 @@ class CastBrowserProcess { ...@@ -129,7 +129,7 @@ class CastBrowserProcess {
std::unique_ptr<metrics::CastBrowserMetrics> cast_browser_metrics_; std::unique_ptr<metrics::CastBrowserMetrics> cast_browser_metrics_;
std::unique_ptr<RemoteDebuggingServer> remote_debugging_server_; std::unique_ptr<RemoteDebuggingServer> remote_debugging_server_;
CastWebViewFactory* web_view_factory_ = nullptr; CastWebViewFactory* web_view_factory_;
CastContentBrowserClient* cast_content_browser_client_; CastContentBrowserClient* cast_content_browser_client_;
net::NetLog* net_log_; net::NetLog* net_log_;
std::unique_ptr<TtsController> tts_controller_; std::unique_ptr<TtsController> tts_controller_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment