Skip to content

Commit

Permalink
This is reland of crrev.com/2884623002.
Browse files Browse the repository at this point in the history
Added test uncovered a stack-use-after-scope issue in AshTouchExplorationManager. Patch #2 fixes the issue.

Refactor backdrop that is currently used in the maximized mode.

The maximized mode creates a backdrop window so that a user will not see the content of windows behind the top window,
in case it doesn't cover the entire window. (can happen if the maximize size is specified for example)
This CL generalizes the backdrop code used in maximize mode as to create the backdrop in the following scenarios:

1) Has a aura::client::kHasBackdrop property = true.
2) BackdropDelegate::HasBackdrop(aura::Window* window) returns true.
3) Active ARC window when the spoken feedback is enabled.

* Added delegate to check if the window should have a backdrop.
  Maximized mode always puts a backdrop.

* Added kHasBackdrop property for a window that needs a backdrop even in clamshell.

* Move the accessibility feature implemented in exo's backbround. This
  is useful and should be there even for non-arc/exo case.

TBR=jamescook@chromium.org,reveman@chromium.org,sky@chromium.org
BUG=721646
TEST=coverted by unit tests

Review-Url: https://codereview.chromium.org/2884623002
Cr-Commit-Position: refs/heads/master@{#472401}
Committed: https://chromium.googlesource.com/chromium/src/+/04936c54ed2396ae54cd824e24f11151e0e11948

patch from issue 2884623002 at patchset 220001 (http://crrev.com/2884623002#ps220001)

Review-Url: https://codereview.chromium.org/2890733005
Cr-Commit-Position: refs/heads/master@{#472479}
  • Loading branch information
mitoshima authored and Commit bot committed May 17, 2017
1 parent 8876859 commit 512e6cd
Show file tree
Hide file tree
Showing 32 changed files with 783 additions and 570 deletions.
8 changes: 5 additions & 3 deletions ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,8 @@ component("ash") {
"wm/lock_state_observer.h",
"wm/lock_window_state.cc",
"wm/lock_window_state.h",
"wm/maximize_mode/maximize_mode_backdrop_delegate_impl.cc",
"wm/maximize_mode/maximize_mode_backdrop_delegate_impl.h",
"wm/maximize_mode/maximize_mode_controller.cc",
"wm/maximize_mode/maximize_mode_controller.h",
"wm/maximize_mode/maximize_mode_event_handler.cc",
Expand All @@ -673,8 +675,6 @@ component("ash") {
"wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_ozone.h",
"wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_x11.cc",
"wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_x11.h",
"wm/maximize_mode/workspace_backdrop_delegate.cc",
"wm/maximize_mode/workspace_backdrop_delegate.h",
"wm/mru_window_tracker.cc",
"wm/mru_window_tracker.h",
"wm/overlay_event_filter.cc",
Expand Down Expand Up @@ -794,6 +794,9 @@ component("ash") {
"wm/wm_types.h",
"wm/wm_window_animations.cc",
"wm/wm_window_animations.h",
"wm/workspace/backdrop_controller.cc",
"wm/workspace/backdrop_controller.h",
"wm/workspace/backdrop_delegate.h",
"wm/workspace/magnetism_matcher.cc",
"wm/workspace/magnetism_matcher.h",
"wm/workspace/multi_window_resize_controller.cc",
Expand All @@ -808,7 +811,6 @@ component("ash") {
"wm/workspace/workspace_event_handler_aura.h",
"wm/workspace/workspace_layout_manager.cc",
"wm/workspace/workspace_layout_manager.h",
"wm/workspace/workspace_layout_manager_backdrop_delegate.h",
"wm/workspace/workspace_types.h",
"wm/workspace/workspace_window_resizer.cc",
"wm/workspace/workspace_window_resizer.h",
Expand Down
6 changes: 3 additions & 3 deletions ash/ash_touch_exploration_manager_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ void AshTouchExplorationManager::UpdateTouchExplorationState() {
touch_accessibility_enabler_.get());
}
if (pass_through_surface) {
const gfx::Rect& work_area = root_window_controller_->GetWindow()
->GetDisplayNearestWindow()
.work_area();
const gfx::Rect work_area = root_window_controller_->GetWindow()
->GetDisplayNearestWindow()
.work_area();
touch_exploration_controller_->SetExcludeBounds(work_area);
SilenceSpokenFeedback();
Shell::Get()->accessibility_delegate()->ClearFocusHighlight();
Expand Down
6 changes: 6 additions & 0 deletions ash/test/workspace_controller_test_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "ash/test/workspace_controller_test_api.h"

#include "ash/test/workspace_event_handler_test_helper.h"
#include "ash/wm/workspace/backdrop_controller.h"
#include "ash/wm/workspace/workspace_layout_manager.h"
#include "ash/wm/workspace_controller.h"
#include "ui/aura/window.h"

Expand All @@ -26,5 +28,9 @@ WorkspaceControllerTestApi::GetMultiWindowResizeController() {
return WorkspaceEventHandlerTestHelper(GetEventHandler()).resize_controller();
}

aura::Window* WorkspaceControllerTestApi::GetBackdropWindow() {
return controller_->layout_manager_->backdrop_controller_->backdrop_window_;
}

} // namespace test
} // namespace ash
1 change: 1 addition & 0 deletions ash/test/workspace_controller_test_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class ASH_EXPORT WorkspaceControllerTestApi {

WorkspaceEventHandler* GetEventHandler();
MultiWindowResizeController* GetMultiWindowResizeController();
aura::Window* GetBackdropWindow();

private:
WorkspaceController* controller_;
Expand Down
17 changes: 17 additions & 0 deletions ash/wm/maximize_mode/maximize_mode_backdrop_delegate_impl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ash/wm/maximize_mode/maximize_mode_backdrop_delegate_impl.h"

namespace ash {

MaximizeModeBackdropDelegateImpl::MaximizeModeBackdropDelegateImpl() = default;

MaximizeModeBackdropDelegateImpl::~MaximizeModeBackdropDelegateImpl() = default;

bool MaximizeModeBackdropDelegateImpl::HasBackdrop(aura::Window* window) {
return true;
}

} // namespace ash
27 changes: 27 additions & 0 deletions ash/wm/maximize_mode/maximize_mode_backdrop_delegate_impl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ash/wm/workspace/backdrop_delegate.h"

#include "ash/ash_export.h"
#include "base/macros.h"

namespace ash {

// A backdrop delegate for MaximizedMode, which always creates a backdrop.
// This is also used in the WorkspaceLayoutManagerBackdropTest, hence
// is public.
class ASH_EXPORT MaximizeModeBackdropDelegateImpl : public BackdropDelegate {
public:
MaximizeModeBackdropDelegateImpl();
~MaximizeModeBackdropDelegateImpl() override;

protected:
bool HasBackdrop(aura::Window* window) override;

private:
DISALLOW_COPY_AND_ASSIGN(MaximizeModeBackdropDelegateImpl);
};

} // namespace ash
24 changes: 4 additions & 20 deletions ash/wm/maximize_mode/maximize_mode_window_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
#include "ash/session/session_state_delegate.h"
#include "ash/shell.h"
#include "ash/shell_port.h"
#include "ash/wm/maximize_mode/maximize_mode_backdrop_delegate_impl.h"
#include "ash/wm/maximize_mode/maximize_mode_event_handler.h"
#include "ash/wm/maximize_mode/maximize_mode_window_state.h"
#include "ash/wm/maximize_mode/workspace_backdrop_delegate.h"
#include "ash/wm/mru_window_tracker.h"
#include "ash/wm/overview/window_selector_controller.h"
#include "ash/wm/window_state.h"
Expand Down Expand Up @@ -82,20 +82,10 @@ void MaximizeModeWindowManager::WindowStateDestroyed(WmWindow* window) {
}

void MaximizeModeWindowManager::OnOverviewModeStarting() {
if (backdrops_hidden_)
return;

EnableBackdropBehindTopWindowOnEachDisplay(false);
SetDeferBoundsUpdates(true);
backdrops_hidden_ = true;
}

void MaximizeModeWindowManager::OnOverviewModeEnded() {
if (!backdrops_hidden_)
return;

backdrops_hidden_ = false;
EnableBackdropBehindTopWindowOnEachDisplay(true);
SetDeferBoundsUpdates(false);
}

Expand Down Expand Up @@ -202,8 +192,7 @@ void MaximizeModeWindowManager::SetIgnoreWmEventsForExit() {
}
}

MaximizeModeWindowManager::MaximizeModeWindowManager()
: backdrops_hidden_(false) {
MaximizeModeWindowManager::MaximizeModeWindowManager() {
// The overview mode needs to be ended before the maximize mode is started. To
// guarantee the proper order, it will be turned off from here.
CancelOverview();
Expand Down Expand Up @@ -310,17 +299,12 @@ bool MaximizeModeWindowManager::IsContainerWindow(aura::Window* window) {

void MaximizeModeWindowManager::EnableBackdropBehindTopWindowOnEachDisplay(
bool enable) {
if (backdrops_hidden_)
return;

// Inform the WorkspaceLayoutManager that we want to show a backdrop behind
// the topmost window of its container.
for (WmWindow* root : ShellPort::Get()->GetAllRootWindows()) {
RootWindowController* controller = root->GetRootWindowController();
WmWindow* default_container =
root->GetChildByShellWindowId(kShellWindowId_DefaultContainer);
controller->workspace_controller()->SetMaximizeBackdropDelegate(
enable ? base::MakeUnique<WorkspaceBackdropDelegate>(default_container)
controller->workspace_controller()->SetBackdropDelegate(
enable ? base::MakeUnique<MaximizeModeBackdropDelegateImpl>()
: nullptr);
}
}
Expand Down
3 changes: 0 additions & 3 deletions ash/wm/maximize_mode/maximize_mode_window_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,6 @@ class ASH_EXPORT MaximizeModeWindowManager : public aura::WindowObserver,
// Windows added to the container, but not yet shown.
std::unordered_set<aura::Window*> added_windows_;

// True if all backdrops are hidden.
bool backdrops_hidden_;

std::unique_ptr<wm::MaximizeModeEventHandler> event_handler_;

DISALLOW_COPY_AND_ASSIGN(MaximizeModeWindowManager);
Expand Down
133 changes: 0 additions & 133 deletions ash/wm/maximize_mode/workspace_backdrop_delegate.cc

This file was deleted.

65 changes: 0 additions & 65 deletions ash/wm/maximize_mode/workspace_backdrop_delegate.h

This file was deleted.

Loading

0 comments on commit 512e6cd

Please sign in to comment.