New issue
Advanced search Search tips

Issue 798857 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 771178



Sign in to add a comment

OverviewButtonTrayTest.SecondaryTrayCreatedVisible is incorrect

Project Member Reported by steve...@chromium.org, Jan 3 2018

Issue description

The test OverviewButtonTrayTest.SecondaryTrayCreatedVisible fails when DisplayConfigurationObserver is running (part of  issue 678949 ). This test will need to be disabled with that change until it is fixed.

The correct fix is not entirely obvious. The following code breaks the test:

void DisplayConfigurationObserver::OnTabletModeStarted() {
  ...
  // TODO(oshima): Mirroring won't work with 3 displays. crbug.com/737667.
  display::DisplayManager* display_manager =
      ash::Shell::Get()->display_manager();
  was_in_mirror_mode_ = display_manager->IsInMirrorMode();
  display_manager->SetMirrorMode(true);
  display_manager->layout_store()->set_forced_mirror_mode(true);
}


 
Cc: jonr...@chromium.org
Labels: OS-Chrome
Note: OverviewButtonTrayTest.DisplaysOnBothDisplays has a similar problem.

I'm unfamiliar with the change that is breaking these tests. But is it that the tests are no longer written correctly. Or is this change actually breaking the feature?
Currently DisplayConfigurationObserver is owned by ChromeShellDelegate due to some dependency issues that we are attempting to resolve.

AshTestBase uses test ShellDelegate that does not create DisplayConfigurationObserver.

DisplayConfigurationObserver is very much Ash specific and it seems to me that its behavior should be taken into account in this test.

In the short term I am adding a mechanism for this and other tests to disable the DisplayConfigurationObserver functionality as a workaround.

Blockedon: 771178
This was due to that dependency issue. I'll fix it once your change is landed.
Components: UI>Shell>OverviewMode
Update: SecondaryTrayCreatedVisible fails on ToT if base::RunLoop().RunUntilIdle() is called after UpdateDisplay().

See https://chromium-review.googlesource.com/c/chromium/src/+/1006226/ which disables the test for more details.

Sign in to add a comment