Skip to content

Commit

Permalink
Remove WORKSPACE_WINDOW_STATE_WINDOW_OVERLAPS_SHELF and related code
Browse files Browse the repository at this point in the history
Shelf no longer becomes transparent when on overlapped windows,
so we can remove this.

Bug: 912191
Test: updated existing tests to reflect the change.
Change-Id: I5bd31fb131703e4ba779b2444838e87d31358192
Reviewed-on: https://chromium-review.googlesource.com/c/1399047
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621070}
  • Loading branch information
mitoshima authored and Commit Bot committed Jan 9, 2019
1 parent 5b3b135 commit 2f56955
Show file tree
Hide file tree
Showing 16 changed files with 30 additions and 102 deletions.
3 changes: 0 additions & 3 deletions ash/app_list/app_list_presenter_delegate_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "ash/shell.h"
#include "ash/system/status_area_widget.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "ash/wm/window_state.h"
#include "base/command_line.h"
#include "chromeos/constants/chromeos_switches.h"
#include "ui/aura/window.h"
Expand Down Expand Up @@ -95,8 +94,6 @@ void AppListPresenterDelegateImpl::Init(app_list::AppListView* view,
view->Initialize(params);

SnapAppListBoundsToDisplayEdge();
wm::GetWindowState(view->GetWidget()->GetNativeWindow())
->set_ignored_by_shelf(true);
Shell::Get()->AddPreTargetHandler(this);

// By setting us as DnD recipient, the app list knows that we can
Expand Down
2 changes: 0 additions & 2 deletions ash/shelf/shelf_layout_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ void ShelfLayoutManager::UpdateVisibilityState() {
? SHELF_AUTO_HIDE
: CalculateShelfVisibility());
break;
case wm::WORKSPACE_WINDOW_STATE_WINDOW_OVERLAPS_SHELF:
case wm::WORKSPACE_WINDOW_STATE_DEFAULT:
SetState(CalculateShelfVisibility());
break;
Expand Down Expand Up @@ -1515,7 +1514,6 @@ void ShelfLayoutManager::UpdateWorkspaceMask(
case wm::WORKSPACE_WINDOW_STATE_FULL_SCREEN:
container->layer()->SetMasksToBounds(false);
break;
case wm::WORKSPACE_WINDOW_STATE_WINDOW_OVERLAPS_SHELF:
case wm::WORKSPACE_WINDOW_STATE_DEFAULT:
container->layer()->SetMasksToBounds(true);
break;
Expand Down
24 changes: 15 additions & 9 deletions ash/shelf/shelf_layout_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "ash/wm/lock_state_controller.h"
#include "ash/wm/overview/window_selector_controller.h"
#include "ash/wm/splitview/split_view_controller.h"
#include "ash/wm/splitview/split_view_divider.h"
#include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
#include "ash/wm/window_state.h"
#include "ash/wm/window_util.h"
Expand Down Expand Up @@ -2249,10 +2250,9 @@ TEST_F(ShelfLayoutManagerTest, WorkspaceMask) {
EXPECT_EQ(wm::WORKSPACE_WINDOW_STATE_DEFAULT, GetWorkspaceWindowState());
EXPECT_TRUE(GetNonLockScreenContainersContainerLayer()->GetMasksToBounds());

// Overlaps with shelf.
// Overlaps with shelf should not cause any specific behavior.
w1->SetBounds(GetShelfLayoutManager()->GetIdealBounds());
EXPECT_EQ(wm::WORKSPACE_WINDOW_STATE_WINDOW_OVERLAPS_SHELF,
GetWorkspaceWindowState());
EXPECT_EQ(wm::WORKSPACE_WINDOW_STATE_DEFAULT, GetWorkspaceWindowState());
EXPECT_TRUE(GetNonLockScreenContainersContainerLayer()->GetMasksToBounds());

w1->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
Expand All @@ -2265,17 +2265,15 @@ TEST_F(ShelfLayoutManagerTest, WorkspaceMask) {

std::unique_ptr<aura::Window> w2(CreateTestWindow());
w2->Show();
EXPECT_EQ(wm::WORKSPACE_WINDOW_STATE_WINDOW_OVERLAPS_SHELF,
GetWorkspaceWindowState());
EXPECT_EQ(wm::WORKSPACE_WINDOW_STATE_DEFAULT, GetWorkspaceWindowState());
EXPECT_TRUE(GetNonLockScreenContainersContainerLayer()->GetMasksToBounds());

w2.reset();
EXPECT_EQ(wm::WORKSPACE_WINDOW_STATE_FULL_SCREEN, GetWorkspaceWindowState());
EXPECT_FALSE(GetNonLockScreenContainersContainerLayer()->GetMasksToBounds());

w1->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
EXPECT_EQ(wm::WORKSPACE_WINDOW_STATE_WINDOW_OVERLAPS_SHELF,
GetWorkspaceWindowState());
EXPECT_EQ(wm::WORKSPACE_WINDOW_STATE_DEFAULT, GetWorkspaceWindowState());
EXPECT_TRUE(GetNonLockScreenContainersContainerLayer()->GetMasksToBounds());
}

Expand Down Expand Up @@ -2380,8 +2378,16 @@ TEST_F(ShelfLayoutManagerTest, ShelfBackgroundColorInSplitView) {
EXPECT_TRUE(Shell::Get()->window_selector_controller()->IsSelecting());
EXPECT_EQ(SHELF_BACKGROUND_SPLIT_VIEW, GetShelfWidget()->GetBackgroundType());

// Ending split view mode will maximize the two windows.
split_view_controller->EndSplitView();
// Drag the divider to left to end split view mode, which will maximize the
// the right snapped window.
gfx::Point drag_point = split_view_controller->split_view_divider()
->GetDividerBoundsInScreen(false)
.CenterPoint();
split_view_controller->StartResize(drag_point);
drag_point.set_x(0);
split_view_controller->EndResize(drag_point);

EXPECT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
EXPECT_EQ(SHELF_BACKGROUND_MAXIMIZED, GetShelfWidget()->GetBackgroundType());
}

Expand Down
7 changes: 2 additions & 5 deletions ash/shelf/shelf_window_watcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "ash/shelf/shelf_constants.h"
#include "ash/shelf/shelf_window_watcher_item_delegate.h"
#include "ash/shell.h"
#include "ash/wm/window_state.h"
#include "ash/wm/window_util.h"
#include "base/strings/string_util.h"
#include "ui/aura/client/aura_constants.h"
Expand All @@ -32,8 +31,7 @@ namespace {
ShelfItemType GetShelfItemType(aura::Window* window) {
if ((features::IsMultiProcessMash() || features::IsSingleProcessMash()) &&
window->GetProperty(kShelfItemTypeKey) == TYPE_UNDEFINED &&
window->type() == aura::client::WINDOW_TYPE_NORMAL &&
!wm::GetWindowState(window)->ignored_by_shelf()) {
window->type() == aura::client::WINDOW_TYPE_NORMAL) {
return TYPE_DIALOG;
}
return static_cast<ShelfItemType>(window->GetProperty(kShelfItemTypeKey));
Expand All @@ -43,8 +41,7 @@ ShelfItemType GetShelfItemType(aura::Window* window) {
// Mash sets and returns an initial default shelf id for unidentified windows.
// TODO(msw): Extend this Mash behavior to all Ash configs.
ShelfID GetShelfID(aura::Window* window) {
if (features::IsUsingWindowService() && !window->GetProperty(kShelfIDKey) &&
!wm::GetWindowState(window)->ignored_by_shelf()) {
if (features::IsUsingWindowService() && !window->GetProperty(kShelfIDKey)) {
static int id = 0;
const ash::ShelfID shelf_id(ShelfWindowWatcher::kDefaultShelfIdPrefix +
std::to_string(id++));
Expand Down
9 changes: 0 additions & 9 deletions ash/shelf/shelf_window_watcher_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,6 @@ TEST_F(ShelfWindowWatcherTest, OpenAndCloseMash) {
EXPECT_EQ(type == aura::client::WINDOW_TYPE_NORMAL ? 3 : 2,
model_->item_count());
}

// Windows with WindowState::ignored_by_shelf set do not get shelf items.
widget1 =
CreateTestWidget(nullptr, kShellWindowId_DefaultContainer, gfx::Rect());
wm::GetWindowState(widget1->GetNativeWindow())->set_ignored_by_shelf(true);
// TODO(msw): Make the flag a window property and remove this workaround.
widget1->GetNativeWindow()->SetProperty(aura::client::kDrawAttentionKey,
true);
EXPECT_EQ(2, model_->item_count());
}

TEST_F(ShelfWindowWatcherTest, CreateAndRemoveShelfItemProperties) {
Expand Down
2 changes: 0 additions & 2 deletions ash/wm/overview/overview_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,6 @@ std::unique_ptr<views::Widget> CreateBackgroundWidget(aura::Window* root_window,
// Disable the "bounce in" animation when showing the window.
::wm::SetWindowVisibilityAnimationTransition(widget_window,
::wm::ANIMATE_NONE);
// The background widget should not activate the shelf when passing under it.
wm::GetWindowState(widget_window)->set_ignored_by_shelf(true);
if (params.layer_type == ui::LAYER_SOLID_COLOR) {
widget_window->layer()->SetColor(background_color);
} else if (params.layer_type == ui::LAYER_TEXTURED) {
Expand Down
3 changes: 0 additions & 3 deletions ash/wm/overview/scoped_transform_overview_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ ScopedTransformOverviewWindow::ScopedTransformOverviewWindow(
aura::Window* window)
: selector_item_(selector_item),
window_(window),
ignored_by_shelf_(wm::GetWindowState(window)->ignored_by_shelf()),
original_opacity_(window->layer()->GetTargetOpacity()),
original_mask_layer_(window_->layer()->layer_mask_layer()),
weak_ptr_factory_(this) {
Expand Down Expand Up @@ -217,7 +216,6 @@ void ScopedTransformOverviewWindow::RestoreWindow(bool reset_transform,
// Shadow controller may be null on shutdown.
if (Shell::Get()->shadow_controller())
Shell::Get()->shadow_controller()->UpdateShadowForWindow(window_);
wm::GetWindowState(window_)->set_ignored_by_shelf(ignored_by_shelf_);
if (minimized_widget_) {
// Fade out the minimized widget. This animation continues past the
// lifetime of |this|.
Expand Down Expand Up @@ -414,7 +412,6 @@ void ScopedTransformOverviewWindow::PrepareForOverview() {

DCHECK(!overview_started_);
overview_started_ = true;
wm::GetWindowState(window_)->set_ignored_by_shelf(true);
if (window_->GetProperty(aura::client::kShowStateKey) ==
ui::SHOW_STATE_MINIMIZED) {
CreateMirrorWindowForMinimizedState();
Expand Down
6 changes: 1 addition & 5 deletions ash/wm/overview/window_selector_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1036,26 +1036,22 @@ TEST_F(WindowSelectorTest, SelectingHidesAppList) {
}

// Tests that a minimized window's visibility and layer visibility
// stay invisible (A minimized window is cloned during overview),
// and ignored_by_shelf state is restored upon exit.
// stay invisible (A minimized window is cloned during overview).
TEST_F(WindowSelectorTest, MinimizedWindowState) {
const gfx::Rect bounds(400, 400);
std::unique_ptr<aura::Window> window1(CreateWindow(bounds));
wm::WindowState* window_state = wm::GetWindowState(window1.get());
window_state->Minimize();
EXPECT_FALSE(window1->IsVisible());
EXPECT_FALSE(window1->layer()->GetTargetVisibility());
EXPECT_FALSE(window_state->ignored_by_shelf());

ToggleOverview();
EXPECT_FALSE(window1->IsVisible());
EXPECT_FALSE(window1->layer()->GetTargetVisibility());
EXPECT_TRUE(window_state->ignored_by_shelf());

ToggleOverview();
EXPECT_FALSE(window1->IsVisible());
EXPECT_FALSE(window1->layer()->GetTargetVisibility());
EXPECT_FALSE(window_state->ignored_by_shelf());
}

// Tests that a bounds change during overview is corrected for.
Expand Down
10 changes: 0 additions & 10 deletions ash/wm/top_level_window_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,16 +187,6 @@ aura::Window* CreateAndParentTopLevelWindow(
root_window_controller, window_type, property_converter, properties);
DisconnectedAppHandler::Create(window);

auto ignored_by_shelf_iter = properties->find(
ws::mojom::WindowManager::kWindowIgnoredByShelf_InitProperty);
if (ignored_by_shelf_iter != properties->end()) {
wm::WindowState* window_state = wm::GetWindowState(window);
window_state->set_ignored_by_shelf(
mojo::ConvertTo<bool>(ignored_by_shelf_iter->second));
// No need to persist this value.
properties->erase(ignored_by_shelf_iter);
}

// TODO: kFocusable_InitProperty should be removed. http://crbug.com/837713.
auto focusable_iter =
properties->find(ws::mojom::WindowManager::kFocusable_InitProperty);
Expand Down
1 change: 0 additions & 1 deletion ash/wm/window_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,6 @@ void WindowState::SetAndClearRestoreBounds() {
WindowState::WindowState(aura::Window* window)
: window_(window),
bounds_changed_by_user_(false),
ignored_by_shelf_(false),
can_consume_system_keys_(false),
unminimize_to_restore_bounds_(false),
hide_shelf_when_fullscreen_(true),
Expand Down
8 changes: 0 additions & 8 deletions ash/wm/window_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,13 +279,6 @@ class ASH_EXPORT WindowState : public aura::WindowObserver {
bool bounds_changed_by_user() const { return bounds_changed_by_user_; }
void set_bounds_changed_by_user(bool bounds_changed_by_user);

// True if the window is ignored by the shelf layout manager for
// purposes of darkening the shelf.
bool ignored_by_shelf() const { return ignored_by_shelf_; }
void set_ignored_by_shelf(bool ignored_by_shelf) {
ignored_by_shelf_ = ignored_by_shelf;
}

// True if the window should be offered a chance to consume special system
// keys such as brightness, volume, etc. that are usually handled by the
// shell.
Expand Down Expand Up @@ -438,7 +431,6 @@ class ASH_EXPORT WindowState : public aura::WindowObserver {
std::unique_ptr<WindowStateDelegate> delegate_;

bool bounds_changed_by_user_;
bool ignored_by_shelf_;
bool can_consume_system_keys_;
std::unique_ptr<DragDetails> drag_details_;

Expand Down
3 changes: 0 additions & 3 deletions ash/wm/workspace/workspace_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ enum WorkspaceWindowState {
// There's a maximized window.
WORKSPACE_WINDOW_STATE_MAXIMIZED,

// At least one window overlaps the shelf.
WORKSPACE_WINDOW_STATE_WINDOW_OVERLAPS_SHELF,

// None of the windows are fullscreen, maximized or touch the shelf.
WORKSPACE_WINDOW_STATE_DEFAULT,
};
Expand Down
23 changes: 12 additions & 11 deletions ash/wm/workspace_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ash/shell.h"
#include "ash/wm/fullscreen_window_finder.h"
#include "ash/wm/mru_window_tracker.h"
#include "ash/wm/overview/window_selector_controller.h"
#include "ash/wm/window_state.h"
#include "ash/wm/wm_window_animations.h"
#include "ash/wm/workspace/backdrop_controller.h"
Expand Down Expand Up @@ -56,31 +57,31 @@ wm::WorkspaceWindowState WorkspaceController::GetWindowState() const {
if (!viewport_)
return wm::WORKSPACE_WINDOW_STATE_DEFAULT;

// Always use DEFAULT state in overview mode so that work area stays
// the same regardles of the window we have.
// The |window_selector_controller| can be null during shutdown.
if (Shell::Get()->window_selector_controller() &&
Shell::Get()->window_selector_controller()->IsSelecting()) {
return wm::WORKSPACE_WINDOW_STATE_DEFAULT;
}

const aura::Window* fullscreen = wm::GetWindowForFullscreenMode(viewport_);
if (fullscreen && !wm::GetWindowState(fullscreen)->ignored_by_shelf())
if (fullscreen)
return wm::WORKSPACE_WINDOW_STATE_FULL_SCREEN;

const gfx::Rect shelf_bounds(Shelf::ForWindow(viewport_)->GetIdealBounds());
bool window_overlaps_launcher = false;
auto mru_list =
Shell::Get()->mru_window_tracker()->BuildWindowListIgnoreModal();

for (aura::Window* window : mru_list) {
if (window->GetRootWindow() != viewport_->GetRootWindow())
continue;
wm::WindowState* window_state = wm::GetWindowState(window);
if (window_state->ignored_by_shelf() ||
(window->layer() && !window->layer()->GetTargetVisibility())) {
if (window->layer() && !window->layer()->GetTargetVisibility())
continue;
}
if (window_state->IsMaximized())
return wm::WORKSPACE_WINDOW_STATE_MAXIMIZED;
window_overlaps_launcher |= window->bounds().Intersects(shelf_bounds);
}

return window_overlaps_launcher
? wm::WORKSPACE_WINDOW_STATE_WINDOW_OVERLAPS_SHELF
: wm::WORKSPACE_WINDOW_STATE_DEFAULT;
return wm::WORKSPACE_WINDOW_STATE_DEFAULT;
}

void WorkspaceController::DoInitialAnimation() {
Expand Down
8 changes: 0 additions & 8 deletions ash/wm/workspace_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -353,14 +353,6 @@ TEST_F(WorkspaceControllerTest, ShelfStateUpdated) {
0, shelf_layout_manager()->GetIdealBounds().y() - 10, 101, 102);
// Move |w1| to overlap the shelf.
w1->SetBounds(touches_shelf_bounds);

// Add a visible ignored window
std::unique_ptr<Window> w_ignored(CreateTestWindow());
w_ignored->SetBounds(touches_shelf_bounds);
wm::GetWindowState(&(*w_ignored))->set_ignored_by_shelf(true);
w_ignored->Show();

// Then make it visible
w1->Show();

wm::ActivateWindow(w1.get());
Expand Down
19 changes: 0 additions & 19 deletions chrome/browser/ui/views/status_bubble_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,6 @@
#include "ui/views/widget/widget.h"
#include "url/gurl.h"

#if defined(OS_CHROMEOS)
#include "ash/shell.h" // mash-ok
#include "ash/wm/window_state.h" // mash-ok
#include "services/ws/public/cpp/property_type_converters.h" // nogncheck
#include "services/ws/public/mojom/window_manager.mojom.h" // nogncheck
#endif

namespace {

// The alpha and color of the bubble's shadow.
Expand Down Expand Up @@ -661,23 +654,11 @@ void StatusBubbleViews::Init() {
params.parent = frame->GetNativeView();
params.context = frame->GetNativeWindow();
params.name = "StatusBubble";
#if defined(OS_CHROMEOS)
params.mus_properties
[ws::mojom::WindowManager::kWindowIgnoredByShelf_InitProperty] =
mojo::ConvertTo<std::vector<uint8_t>>(true);
#endif
popup_->Init(params);
// We do our own animation and don't want any from the system.
popup_->SetVisibilityChangedAnimationsEnabled(false);
popup_->SetOpacity(0.f);
popup_->SetContentsView(view_);
#if defined(OS_CHROMEOS)
// Mash is handled via mus_properties.
if (ash::Shell::HasInstance()) {
ash::wm::GetWindowState(popup_->GetNativeWindow())
->set_ignored_by_shelf(true);
}
#endif
RepositionPopup();
}
}
Expand Down
4 changes: 0 additions & 4 deletions services/ws/public/mojom/window_manager.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ interface WindowManager {
// and the client area should be equivalent to the window area. Type: bool
const string kRemoveStandardFrame_InitProperty = "init:remove-standard-frame";

// A flag controlling the window's presence on the mash shelf. Type: bool
const string kWindowIgnoredByShelf_InitProperty =
"init:window-ignored-by-shelf";

// The window type. This maps to aura::client::kWindowTypeKey as well as
// Window::type(). This mapping is only done for top-level windows that are
// created by the window manager at the request of a client.
Expand Down

0 comments on commit 2f56955

Please sign in to comment.