desks: Pause occlusion update while starting desk's screenshot is taken
The fix for crbug.com/40100714 has made the ending desk's container shown (with opacity 0.0f) before the desk switching animation. Although the container is invisible, this might affect occlusion states of the starting desk's windows. Also, since this happens before the starting desk screenshot is taken, the side effect of occluded window might be captured by the screenshot and can be observed by user. This CL fixes the issue by pausing the occlusion tracker while the starting desk screenshot is taken. Bug: b:398287318 Change-Id: I104ba7cb7eba893e929d8cccc6592a87a369f8d9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6538912 Commit-Queue: Jun Ishiguro <junis@google.com> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/main@{#1461138}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
e1586c86c4
commit
f01a08e89c
@ -46,6 +46,14 @@ void DeskAnimationBase::Launch() {
|
||||
throughput_tracker_->Start(GetSmoothnessReportCallback());
|
||||
}
|
||||
|
||||
// Pause occlusion tracking while taking starting desk screenshot.
|
||||
//
|
||||
// Occlusion tracking should be paused prior to PrepareForActivationAnimation
|
||||
// since it can update the occlusion state of the starting desk windows.
|
||||
// See crbug.com/417088506 for the context.
|
||||
pauser_for_screenshot_ =
|
||||
std::make_unique<aura::WindowOcclusionTracker::ScopedPause>();
|
||||
|
||||
// This step makes sure that the containers of the target desk are shown at
|
||||
// the beginning of the animation (but not actually visible to the user yet,
|
||||
// until the desk is actually activated at a later step of the animation).
|
||||
@ -107,6 +115,9 @@ void DeskAnimationBase::OnStartingDeskScreenshotTaken(int ending_desk_index) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Now each display is covered by starting desk screenshot.
|
||||
pauser_for_screenshot_.reset();
|
||||
|
||||
// Extend the compositors' timeouts in order to prevents any repaints until
|
||||
// the desks are switched and overview mode exits.
|
||||
const auto roots = Shell::GetAllRootWindows();
|
||||
|
@ -13,6 +13,7 @@
|
||||
#include "base/functional/callback.h"
|
||||
#include "base/memory/raw_ptr.h"
|
||||
#include "base/time/time.h"
|
||||
#include "ui/aura/window_occlusion_tracker.h"
|
||||
#include "ui/compositor/compositor_metrics_tracker.h"
|
||||
|
||||
namespace ash {
|
||||
@ -155,6 +156,10 @@ class ASH_EXPORT DeskAnimationBase
|
||||
// DeskController are tied together in production code, but may not be in a
|
||||
// test scenario.
|
||||
bool skip_notify_controller_on_animation_finished_for_testing_ = false;
|
||||
|
||||
// Used to pause occlusion updates while taking starting desk screenshot.
|
||||
std::unique_ptr<aura::WindowOcclusionTracker::ScopedPause>
|
||||
pauser_for_screenshot_;
|
||||
};
|
||||
|
||||
} // namespace ash
|
||||
|
@ -2613,6 +2613,89 @@ TEST_P(DesksTest, AddDeskWhileExitingOverview) {
|
||||
NewDesk();
|
||||
}
|
||||
|
||||
// Verify that the starting desk is not occluded by the ending desk when
|
||||
// activation is switching. Regression test for crbug.com/398287318.
|
||||
TEST_P(DesksTest, EndingDeskShouldNotOccludeStartingDesk) {
|
||||
UpdateDisplay("600x400");
|
||||
|
||||
auto* root = Shell::GetPrimaryRootWindow();
|
||||
auto* controller = DesksController::Get();
|
||||
|
||||
// Prepare two desks.
|
||||
NewDesk();
|
||||
ASSERT_EQ(2u, controller->desks().size());
|
||||
|
||||
const Desk* desk1 = controller->GetDeskAtIndex(0);
|
||||
const Desk* desk2 = controller->GetDeskAtIndex(1);
|
||||
auto* desk1_container = desk1->GetDeskContainerForRoot(root);
|
||||
auto* desk2_container = desk2->GetDeskContainerForRoot(root);
|
||||
|
||||
// desk1 is active.
|
||||
ASSERT_TRUE(desk1_container->IsVisible());
|
||||
ASSERT_FALSE(desk2_container->IsVisible());
|
||||
ASSERT_EQ(desks_util::GetActiveDeskContainerForRoot(root), desk1_container);
|
||||
|
||||
// Create two windows so that win2 can completely cover win1.
|
||||
const auto win1_bounds = gfx::Rect{20, 20, 100, 100};
|
||||
const auto win2_bounds = gfx::Rect{10, 10, 200, 200};
|
||||
std::unique_ptr<aura::Window> win1 = CreateAppWindow(win1_bounds);
|
||||
std::unique_ptr<aura::Window> win2 = CreateAppWindow(win2_bounds);
|
||||
win1->TrackOcclusionState();
|
||||
win2->TrackOcclusionState();
|
||||
win1->Show();
|
||||
win2->Show();
|
||||
// Check that win1 is occluded when win2 is active.
|
||||
wm::ActivateWindow(win2.get());
|
||||
ASSERT_EQ(win1->GetOcclusionState(), aura::Window::OcclusionState::OCCLUDED);
|
||||
ASSERT_EQ(win2->GetOcclusionState(), aura::Window::OcclusionState::VISIBLE);
|
||||
wm::ActivateWindow(win1.get());
|
||||
ASSERT_EQ(win1->GetOcclusionState(), aura::Window::OcclusionState::VISIBLE);
|
||||
ASSERT_EQ(win2->GetOcclusionState(), aura::Window::OcclusionState::VISIBLE);
|
||||
|
||||
// Move win2 to desk2.
|
||||
controller->SendToDeskAtIndex(win2.get(), 1);
|
||||
|
||||
// Now win1 is only visible.
|
||||
ASSERT_EQ(win1->GetOcclusionState(), aura::Window::OcclusionState::VISIBLE);
|
||||
ASSERT_EQ(win2->GetOcclusionState(), aura::Window::OcclusionState::HIDDEN);
|
||||
|
||||
// Activate desk2.
|
||||
DeskSwitchAnimationWaiter animation_waiter;
|
||||
controller->ActivateDesk(desk2, DesksSwitchSource::kDeskSwitchShortcut);
|
||||
|
||||
// `ActivateDesk` does not activate desk2 immediately.
|
||||
// Although the fix for crbug.com/40100714 makes win2 shown (with opacity 0),
|
||||
// win1 should not be occluded.
|
||||
// Note: Occlusion state update is paused until the starting desk screenshot
|
||||
// is taken.
|
||||
EXPECT_TRUE(desk1_container->IsVisible());
|
||||
EXPECT_TRUE(desk2_container->IsVisible());
|
||||
EXPECT_EQ(win1->GetOcclusionState(), aura::Window::OcclusionState::VISIBLE);
|
||||
EXPECT_EQ(win2->GetOcclusionState(), aura::Window::OcclusionState::HIDDEN);
|
||||
|
||||
// Wait until the starting desk screenshot has been taken.
|
||||
base::RunLoop run_loop;
|
||||
DeskAnimationBase* animation = DesksController::Get()->animation();
|
||||
ASSERT_TRUE(animation);
|
||||
auto* desk_switch_animator =
|
||||
animation->GetDeskSwitchAnimatorAtIndexForTesting(0);
|
||||
ASSERT_TRUE(desk_switch_animator);
|
||||
RootWindowDeskSwitchAnimatorTestApi(desk_switch_animator)
|
||||
.SetOnStartingScreenshotTakenCallback(run_loop.QuitClosure());
|
||||
run_loop.Run();
|
||||
|
||||
// Activation is done while the ending desk screenshot is taken.
|
||||
EXPECT_EQ(desks_util::GetActiveDeskContainerForRoot(root), desk2_container);
|
||||
EXPECT_FALSE(desk1_container->IsVisible());
|
||||
EXPECT_TRUE(desk2_container->IsVisible());
|
||||
// Occlusion tracking is resumed before the ending desk screenshot is
|
||||
// taken, so occlusion stat should be updated.
|
||||
EXPECT_EQ(win1->GetOcclusionState(), aura::Window::OcclusionState::HIDDEN);
|
||||
EXPECT_EQ(win2->GetOcclusionState(), aura::Window::OcclusionState::VISIBLE);
|
||||
|
||||
animation_waiter.Wait();
|
||||
}
|
||||
|
||||
class DesksWithMultiDisplayOverview : public AshTestBase {
|
||||
public:
|
||||
DesksWithMultiDisplayOverview() = default;
|
||||
|
Reference in New Issue
Block a user