Skip to content

Commit

Permalink
Revert "Display mirroring persistency changes"
Browse files Browse the repository at this point in the history
This reverts commit fa8dfaa.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 524627 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2ZhOGRmYWE2MjFjMTcwMWRjZTg0MWY3NmVmZmVjZDA2NGZmMzc1M2UM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/3252

Original change's description:
> Display mirroring persistency changes
> 
> Changes:
> 1. Change mirroring persistency rule to:
> If previous mirror mode is not set (e.g. no external display in previous
> configuration), turn on mirror mode if at least one of the external
> displays was in mirror mode before and turn off mirror mode otherwise.
> If previous mirror mode is set, mirror mode remains its previous value
> as long as there are more than 1 displays.
> For example:
> Case 1: Connect display A; Turn on mirror mode; Connect display B. B
> should be in mirror mode as well.
> Case 2: Connect display A; Turn on mirror mode; Remove A; Reconnect A; A
> should be in mirror mode.
> Case 3: Connect display A; Turn on mirror mode; Remove A; Connect
> display B. B should not be in mirror mode.
> Case 4: Connect display A; Turn on mirror mode; Connect display B;
> Remove B; Remove A; Reconnect B; B should be in mirror mode.
> 
> 2. Fix a bug in MirrorWindowController: mirroring source display may be
> changed in mirror mode (e.g. Connect two external displays; Turn on
> mirror mode; Close internal display lid; Then the internal display will
> be replaced with one external display as the mirroring source.),
> reflector and mirror windows should be updated in this case.
> 
> 3. Fix broken test.
> 
> Bug: 785416,792207
> Test: MultiMirroringTest.*
> Change-Id: Ia1da9f25df2bcc04f98ab087478a14685fff7708
> Reviewed-on: https://chromium-review.googlesource.com/804647
> Commit-Queue: Weidong Guo <weidongg@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524627}

Change-Id: Ie18b8dc47d031984590ed98af331fdb659b17941
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 785416,792207
Reviewed-on: https://chromium-review.googlesource.com/831290
Cr-Commit-Position: refs/heads/master@{#524629}
  • Loading branch information
Findit committed Dec 17, 2017
1 parent f410ff6 commit ba99d19
Show file tree
Hide file tree
Showing 21 changed files with 223 additions and 707 deletions.
322 changes: 127 additions & 195 deletions ash/display/display_manager_unittest.cc

Large diffs are not rendered by default.

43 changes: 16 additions & 27 deletions ash/display/mirror_window_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,11 @@ display::DisplayManager::MultiDisplayMode GetCurrentMultiDisplayMode() {
display::DisplayManager* display_manager = Shell::Get()->display_manager();
return display_manager->IsInUnifiedMode()
? display::DisplayManager::UNIFIED
: (display_manager->IsInSoftwareMirrorMode()
: (display_manager->IsInMirrorMode()
? display::DisplayManager::MIRRORING
: display::DisplayManager::EXTENDED);
}

int64_t GetCurrentReflectingSourceId() {
display::DisplayManager* display_manager = Shell::Get()->display_manager();
if (display_manager->IsInUnifiedMode())
return display::Screen::GetScreen()->GetPrimaryDisplay().id();
if (display_manager->IsInSoftwareMirrorMode())
return display_manager->mirroring_source_id();
return display::kInvalidDisplayId;
}

} // namespace

struct MirrorWindowController::MirroringHostInfo {
Expand All @@ -146,26 +137,27 @@ MirrorWindowController::~MirrorWindowController() {

void MirrorWindowController::UpdateWindow(
const std::vector<display::ManagedDisplayInfo>& display_info_list) {
display::DisplayManager* display_manager = Shell::Get()->display_manager();
DCHECK(display_manager->IsInSoftwareMirrorMode() ||
display_manager->IsInUnifiedMode());
static int mirror_host_count = 0;
display::DisplayManager* display_manager = Shell::Get()->display_manager();
const display::Display& primary =
display::Screen::GetScreen()->GetPrimaryDisplay();
const display::ManagedDisplayInfo& source_display_info =
display_manager->GetDisplayInfo(primary.id());

multi_display_mode_ = GetCurrentMultiDisplayMode();
reflecting_source_id_ = GetCurrentReflectingSourceId();

for (const display::ManagedDisplayInfo& display_info : display_info_list) {
std::unique_ptr<RootWindowTransformer> transformer;
if (display_manager->IsInSoftwareMirrorMode()) {
if (display_manager->IsInMirrorMode()) {
transformer.reset(CreateRootWindowTransformerForMirroredDisplay(
display_manager->GetDisplayInfo(reflecting_source_id_),
display_info));
} else {
DCHECK(display_manager->IsInUnifiedMode());
source_display_info, display_info));
} else if (display_manager->IsInUnifiedMode()) {
display::Display display =
display_manager->GetMirroringDisplayById(display_info.id());
transformer.reset(CreateRootWindowTransformerForUnifiedDesktop(
display::Screen::GetScreen()->GetPrimaryDisplay().bounds(), display));
primary.bounds(), display));
} else {
NOTREACHED();
}

if (mirroring_host_info_map_.find(display_info.id()) ==
Expand Down Expand Up @@ -205,7 +197,7 @@ void MirrorWindowController::UpdateWindow(
AshWindowTreeHost* unified_ash_host =
Shell::Get()
->window_tree_host_manager()
->GetAshWindowTreeHostForDisplayId(reflecting_source_id_);
->GetAshWindowTreeHostForDisplayId(primary.id());
unified_ash_host->RegisterMirroringHost(host_info->ash_host.get());
aura::client::SetScreenPositionClient(host->window(),
screen_position_client_.get());
Expand All @@ -232,9 +224,7 @@ void MirrorWindowController::UpdateWindow(
aura::Env::GetInstance()
->context_factory_private()
->CreateReflector(
Shell::GetRootWindowForDisplayId(reflecting_source_id_)
->GetHost()
->compositor(),
Shell::GetPrimaryRootWindow()->GetHost()->compositor(),
mirror_window->layer());
}
}
Expand Down Expand Up @@ -288,10 +278,9 @@ void MirrorWindowController::UpdateWindow() {
void MirrorWindowController::CloseIfNotNecessary() {
display::DisplayManager::MultiDisplayMode new_mode =
GetCurrentMultiDisplayMode();
int64_t new_reflecting_source_id = GetCurrentReflectingSourceId();
if (multi_display_mode_ != new_mode ||
reflecting_source_id_ != new_reflecting_source_id) {
if (multi_display_mode_ != new_mode) {
Close(true);
multi_display_mode_ = new_mode;
} else {
UpdateWindow();
}
Expand Down
3 changes: 0 additions & 3 deletions ash/display/mirror_window_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,6 @@ class ASH_EXPORT MirrorWindowController

display::DisplayManager::MultiDisplayMode multi_display_mode_;

// The id of the display being mirrored in the reflector.
int64_t reflecting_source_id_ = display::kInvalidDisplayId;

std::unique_ptr<aura::client::ScreenPositionClient> screen_position_client_;

std::unique_ptr<ui::Reflector> reflector_;
Expand Down
20 changes: 9 additions & 11 deletions ash/display/mirror_window_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,8 @@ TEST_F(MirrorWindowControllerTest, MirrorCursorBasic) {
aura::test::TestWindowDelegate test_window_delegate;
test_window_delegate.set_window_component(HTTOP);

display_manager()->SetMultiDisplayMode(display::DisplayManager::MIRRORING);
UpdateDisplay("400x400,400x400");
display_manager()->SetMirrorMode(true);
RunAllPendingInMessageLoop();
aura::Window* root = Shell::Get()->GetPrimaryRootWindow();
std::unique_ptr<aura::Window> window(aura::test::CreateTestWindowWithDelegate(
&test_window_delegate, 0, gfx::Rect(50, 50, 100, 100), root));
Expand Down Expand Up @@ -119,9 +118,8 @@ TEST_F(MirrorWindowControllerTest, MirrorCursorRotate) {
aura::test::TestWindowDelegate test_window_delegate;
test_window_delegate.set_window_component(HTTOP);

display_manager()->SetMultiDisplayMode(display::DisplayManager::MIRRORING);
UpdateDisplay("400x400,400x400");
display_manager()->SetMirrorMode(true);
RunAllPendingInMessageLoop();
aura::Window* root = Shell::Get()->GetPrimaryRootWindow();
std::unique_ptr<aura::Window> window(aura::test::CreateTestWindowWithDelegate(
&test_window_delegate, 0, gfx::Rect(50, 50, 100, 100), root));
Expand Down Expand Up @@ -177,11 +175,10 @@ TEST_F(MirrorWindowControllerTest, MirrorCursorLocations) {
return;

MirrorWindowTestApi test_api;
display_manager()->SetMultiDisplayMode(display::DisplayManager::MIRRORING);

// Test with device scale factor.
UpdateDisplay("400x600*2,400x600");
display_manager()->SetMirrorMode(true);
RunAllPendingInMessageLoop();

aura::Window* root = Shell::Get()->GetPrimaryRootWindow();
ui::test::EventGenerator generator(root);
Expand Down Expand Up @@ -239,9 +236,8 @@ TEST_F(MirrorWindowControllerTest, MirrorCursorMoveOnEnter) {
EXPECT_EQ(display::Display::ROTATE_0,
cursor_test_api.GetCurrentCursorRotation());

display_manager()->SetMultiDisplayMode(display::DisplayManager::MIRRORING);
UpdateDisplay("400x400*2/r,400x400");
display_manager()->SetMirrorMode(true);
RunAllPendingInMessageLoop();

// Entering mirror mode should have centered the cursor on the primary display
// because the cursor's previous position is out of bounds.
Expand Down Expand Up @@ -275,6 +271,8 @@ TEST_F(MirrorWindowControllerTest, DockMode) {
CreateDisplayInfo(external_id, gfx::Rect(1, 1, 100, 100));
std::vector<display::ManagedDisplayInfo> display_info_list;

display_manager()->SetMultiDisplayMode(display::DisplayManager::MIRRORING);

// software mirroring.
display_info_list.push_back(internal_display_info);
display_info_list.push_back(external_display_info);
Expand All @@ -284,16 +282,15 @@ TEST_F(MirrorWindowControllerTest, DockMode) {
.SetFirstDisplayAsInternalDisplay();
EXPECT_EQ(internal_id, internal_display_id);

display_manager()->SetMirrorMode(true);
RunAllPendingInMessageLoop();
EXPECT_EQ(1U, display_manager()->GetNumDisplays());
EXPECT_TRUE(display_manager()->IsInSoftwareMirrorMode());
EXPECT_TRUE(display_manager()->IsInMirrorMode());
EXPECT_EQ(external_id,
display_manager()->GetMirroringDestinationDisplayIdList()[0]);

// dock mode.
display_info_list.clear();
display_info_list.push_back(external_display_info);
display_manager()->SetMultiDisplayMode(display::DisplayManager::MIRRORING);
display_manager()->OnNativeDisplaysChanged(display_info_list);
EXPECT_EQ(1U, display_manager()->GetNumDisplays());
EXPECT_FALSE(display_manager()->IsInMirrorMode());
Expand All @@ -302,6 +299,7 @@ TEST_F(MirrorWindowControllerTest, DockMode) {
display_info_list.clear();
display_info_list.push_back(internal_display_info);
display_info_list.push_back(external_display_info);
display_manager()->SetMultiDisplayMode(display::DisplayManager::MIRRORING);
display_manager()->OnNativeDisplaysChanged(display_info_list);
EXPECT_EQ(1U, display_manager()->GetNumDisplays());
EXPECT_TRUE(display_manager()->IsInMirrorMode());
Expand Down
2 changes: 1 addition & 1 deletion ash/display/root_window_transformers_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,8 @@ TEST_F(RootWindowTransformersTest, ConvertHostToRootCoords) {

TEST_F(RootWindowTransformersTest, LetterBoxPillarBox) {
MirrorWindowTestApi test_api;
display_manager()->SetMultiDisplayMode(display::DisplayManager::MIRRORING);
UpdateDisplay("400x200,500x500");
display_manager()->SetMirrorMode(true);
std::unique_ptr<RootWindowTransformer> transformer(
CreateCurrentRootWindowTransformerForMirroring());
// Y margin must be margin is (500 - 500/400 * 200) / 2 = 125.
Expand Down
Loading

0 comments on commit ba99d19

Please sign in to comment.