From 16bc5f811ed1a927730e24d3fc73ac956face8fe Mon Sep 17 00:00:00 2001 From: oshima Date: Wed, 13 May 2015 17:25:37 -0700 Subject: Close mirroring window only when necessary always recreating was causing extra XRANR event on X11 system. Delete WindowTreeHost asynchronously when the change can be made via input event. BUG=484580,486190 TEST=tested manually. no flicker. Review URL: https://codereview.chromium.org/1133323002 Cr-Commit-Position: refs/heads/master@{#329761} --- ash/display/display_controller.cc | 4 ++-- ash/display/display_controller.h | 2 +- ash/display/display_manager.cc | 9 ++++---- ash/display/display_manager.h | 6 ++--- ash/display/mirror_window_controller.cc | 39 +++++++++++++++++++++++++-------- ash/display/mirror_window_controller.h | 15 ++++++++++--- ash/shell.cc | 1 - 7 files changed, 53 insertions(+), 23 deletions(-) diff --git a/ash/display/display_controller.cc b/ash/display/display_controller.cc index 7d8e549..ec9e478 100644 --- a/ash/display/display_controller.cc +++ b/ash/display/display_controller.cc @@ -805,8 +805,8 @@ void DisplayController::CreateOrUpdateMirroringDisplay( } } -void DisplayController::CloseMirroringDisplay() { - mirror_window_controller_->Close(); +void DisplayController::CloseMirroringDisplayIfNotNecessary() { + mirror_window_controller_->CloseIfNotNecessary(); // If cursor_compositing is enabled for large cursor, the cursor window is // always on the desktop display (the visible cursor on the non-desktop // display is drawn through compositor mirroring). Therefore, it's unnecessary diff --git a/ash/display/display_controller.h b/ash/display/display_controller.h index a46b8d1..bb476ef 100644 --- a/ash/display/display_controller.h +++ b/ash/display/display_controller.h @@ -159,7 +159,7 @@ class ASH_EXPORT DisplayController : public gfx::DisplayObserver, // aura::DisplayManager::Delegate overrides: void CreateOrUpdateMirroringDisplay( const DisplayInfoList& info_list) override; - void CloseMirroringDisplay() override; + void CloseMirroringDisplayIfNotNecessary() override; void PreDisplayConfigurationChange(bool clear_focus) override; void PostDisplayConfigurationChange() override; diff --git a/ash/display/display_manager.cc b/ash/display/display_manager.cc index c04264b..3a42d1a 100644 --- a/ash/display/display_manager.cc +++ b/ash/display/display_manager.cc @@ -673,12 +673,13 @@ void DisplayManager::UpdateDisplays( std::sort(new_display_info_list.begin(), new_display_info_list.end(), DisplayInfoSortFunctor()); + + CreateSoftwareMirroringDisplayInfo(&new_display_info_list); + // Close the mirroring window if any here to avoid creating two compositor on // one display. if (delegate_) - delegate_->CloseMirroringDisplay(); - - CreateSoftwareMirroringDisplay(&new_display_info_list); + delegate_->CloseMirroringDisplayIfNotNecessary(); DisplayList new_displays; DisplayList removed_displays; @@ -1081,7 +1082,7 @@ void DisplayManager::UpdateInternalDisplayModeListForTest() { SetInternalDisplayModeList(info); } -void DisplayManager::CreateSoftwareMirroringDisplay( +void DisplayManager::CreateSoftwareMirroringDisplayInfo( DisplayInfoList* display_info_list) { // Use the internal display or 1st as the mirror source, then scale // the root window so that it matches the external display's diff --git a/ash/display/display_manager.h b/ash/display/display_manager.h index e7711a0..b6ac092 100644 --- a/ash/display/display_manager.h +++ b/ash/display/display_manager.h @@ -65,8 +65,8 @@ class ASH_EXPORT DisplayManager virtual void CreateOrUpdateMirroringDisplay( const DisplayInfoList& display_info_list) = 0; - // Closes the mirror window if exists. - virtual void CloseMirroringDisplay() = 0; + // Closes the mirror window if not necessary. + virtual void CloseMirroringDisplayIfNotNecessary() = 0; // Called before and after the display configuration changes. // When |clear_focus| is true, the implementation should @@ -343,7 +343,7 @@ private: // Creates software mirroring display related information. The display // used to mirror the content is removed from the |display_info_list|. - void CreateSoftwareMirroringDisplay(DisplayInfoList* display_info_list); + void CreateSoftwareMirroringDisplayInfo(DisplayInfoList* display_info_list); gfx::Display* FindDisplayForId(int64 id); diff --git a/ash/display/mirror_window_controller.cc b/ash/display/mirror_window_controller.cc index a4d70a6..5293537 100644 --- a/ash/display/mirror_window_controller.cc +++ b/ash/display/mirror_window_controller.cc @@ -136,12 +136,13 @@ MirrorWindowController::MirroringHostInfo::~MirroringHostInfo() { } MirrorWindowController::MirrorWindowController() - : screen_position_client_(new MirroringScreenPositionClient(this)) { + : multi_display_mode_(DisplayManager::EXTENDED), + screen_position_client_(new MirroringScreenPositionClient(this)) { } MirrorWindowController::~MirrorWindowController() { // Make sure the root window gets deleted before cursor_window_delegate. - Close(); + Close(false); } void MirrorWindowController::UpdateWindow( @@ -247,7 +248,7 @@ void MirrorWindowController::UpdateWindow( [iter](const DisplayInfo& info) { return info.id() == iter->first; }) == display_info_list.end()) { - CloseAndDeleteHost(iter->second); + CloseAndDeleteHost(iter->second, true); iter = mirroring_host_info_map_.erase(iter); } else { ++iter; @@ -266,10 +267,23 @@ void MirrorWindowController::UpdateWindow() { UpdateWindow(display_info_list); } -void MirrorWindowController::Close() { - for (auto& info : mirroring_host_info_map_) { - CloseAndDeleteHost(info.second); - } +void MirrorWindowController::CloseIfNotNecessary() { + DisplayManager* display_manager = Shell::GetInstance()->display_manager(); + + DisplayManager::MultiDisplayMode new_mode = + display_manager->IsInUnifiedMode() + ? DisplayManager::UNIFIED + : (display_manager->IsInMirrorMode() ? DisplayManager::MIRRORING + : DisplayManager::EXTENDED); + if (multi_display_mode_ != new_mode) + Close(true); + multi_display_mode_ = new_mode; +} + +void MirrorWindowController::Close(bool delay_host_deletion) { + for (auto& info : mirroring_host_info_map_) + CloseAndDeleteHost(info.second, delay_host_deletion); + mirroring_host_info_map_.clear(); if (reflector_) { aura::Env::GetInstance()->context_factory()->RemoveReflector( @@ -342,7 +356,8 @@ aura::Window::Windows MirrorWindowController::GetAllRootWindows() const { return root_windows; } -void MirrorWindowController::CloseAndDeleteHost(MirroringHostInfo* host_info) { +void MirrorWindowController::CloseAndDeleteHost(MirroringHostInfo* host_info, + bool delay_host_deletion) { aura::WindowTreeHost* host = host_info->ash_host->AsWindowTreeHost(); aura::client::SetScreenPositionClient(host->window(), nullptr); @@ -357,7 +372,13 @@ void MirrorWindowController::CloseAndDeleteHost(MirroringHostInfo* host_info) { host_info->ash_host->PrepareForShutdown(); reflector_->RemoveMirroringLayer(host_info->mirror_window->layer()); - delete host_info; + // EventProcessor may be accessed after this call if the mirroring window + // was deleted as a result of input event (e.g. shortcut), so don't delete + // now. + if (delay_host_deletion) + base::MessageLoop::current()->DeleteSoon(FROM_HERE, host_info); + else + delete host_info; } scoped_ptr diff --git a/ash/display/mirror_window_controller.h b/ash/display/mirror_window_controller.h index 8bef88a..9f38102 100644 --- a/ash/display/mirror_window_controller.h +++ b/ash/display/mirror_window_controller.h @@ -9,6 +9,7 @@ #include #include "ash/ash_export.h" +#include "ash/display/display_manager.h" #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" @@ -57,8 +58,8 @@ class ASH_EXPORT MirrorWindowController : public aura::WindowTreeHostObserver { // for the mirrored display. void UpdateWindow(); - // Close the mirror window. - void Close(); + // Close the mirror window if they're not necessary any longer. + void CloseIfNotNecessary(); // aura::WindowTreeHostObserver overrides: void OnHostResized(const aura::WindowTreeHost* host) override; @@ -81,7 +82,13 @@ class ASH_EXPORT MirrorWindowController : public aura::WindowTreeHostObserver { struct MirroringHostInfo; - void CloseAndDeleteHost(MirroringHostInfo* host_info); + // Close the mirror window. When |delay_host_deletion| is true, the window + // tree host will be deleted in an another task on UI thread. This is + // necessary to safely delete the WTH that is currently handling input events. + void Close(bool delay_host_deletion); + + void CloseAndDeleteHost(MirroringHostInfo* host_info, + bool delay_host_deletion); // Creates a RootWindowTransformer for current display // configuration. @@ -90,6 +97,8 @@ class ASH_EXPORT MirrorWindowController : public aura::WindowTreeHostObserver { typedef std::map MirroringHostInfoMap; MirroringHostInfoMap mirroring_host_info_map_; + DisplayManager::MultiDisplayMode multi_display_mode_; + scoped_ptr screen_position_client_; scoped_ptr reflector_; diff --git a/ash/shell.cc b/ash/shell.cc index a6f455b..d7dc9b5 100644 --- a/ash/shell.cc +++ b/ash/shell.cc @@ -758,7 +758,6 @@ Shell::~Shell() { // Destroy all child windows including widgets. display_controller_->CloseChildWindows(); - display_controller_->CloseMirroringDisplay(); // Chrome implementation of shelf delegate depends on FocusClient, // so must be deleted before |focus_client_|. -- cgit v1.1