New issue
Advanced search Search tips

Issue 826456 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 678705



Sign in to add a comment

mash: Remove ash::wm::GetWindowState from ChromeViewsDelegate (set_minimum_visibility)

Project Member Reported by jamescook@chromium.org, Mar 27 2018

Issue description

See //chrome/browser/ui/views/chrome_views_delegate_chromeos.cc

It's used to flag the window so it always has a minimum area on screen (so you can always see/drag it).

Probably this could become an ash window property under mus.

 
Labels: -Proj-Mustash-Mash

Comment 2 by est...@chromium.org, Jun 19 2018

Cc: osh...@chromium.org
Owner: est...@chromium.org
Status: Started (was: Untriaged)
This was added here: https://codereview.chromium.org/55303006

To the best of my ability to determine, it is redundant with the other code that was added in that CL, i.e. the bounds adjustment right above where the minimum_visibility flag is set. I can remove it completely and I still get the same behavior when I follow the testing steps in the above CL:

  "open TaskManager on 2nd display, close it, disconnect display, then open task manager again. minimum visibility is covered by test."

Debugging statements verify that the Chrome-side bounds adjustment is adjusting for the missing display and that Ash doesn't make further adjustments in the minimum_visibility codepath in WindowPositioner.

Comment 3 by osh...@chromium.org, Jun 20 2018

The minimum visibility logic is implemented inside window state (default state) now, so removing it should be fine. (I wouldn't be also surprised if the similar logic exists on chrome side as well for desktop)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f86cb2b3c43f1d3b5716d4030a7379490d36ddb

commit 6f86cb2b3c43f1d3b5716d4030a7379490d36ddb
Author: Evan Stade <estade@chromium.org>
Date: Wed Jun 20 23:13:42 2018

Remove "minimum visibility" flag from ash::wm::WindowState.

The original purpose of this code was to make sure the task manager
(among other windows) wouldn't be restored off-screen even if the
saved coordinates were off-screen, e.g. due to the last display it
was on having been removed. However, the rest of
AdjustSavedWindowPlacementChromeOS is sufficient to accomplish that
and the minimum visibility flag/functionality in Ash is currently
redundant.

Bug:  826456 
Change-Id: Icd922ff244eab2c8ff1b12179a123917a0f7245b
Reviewed-on: https://chromium-review.googlesource.com/1107102
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569075}
[modify] https://crrev.com/6f86cb2b3c43f1d3b5716d4030a7379490d36ddb/ash/wm/window_positioner.cc
[modify] https://crrev.com/6f86cb2b3c43f1d3b5716d4030a7379490d36ddb/ash/wm/window_positioner_unittest.cc
[modify] https://crrev.com/6f86cb2b3c43f1d3b5716d4030a7379490d36ddb/ash/wm/window_state.cc
[modify] https://crrev.com/6f86cb2b3c43f1d3b5716d4030a7379490d36ddb/ash/wm/window_state.h
[modify] https://crrev.com/6f86cb2b3c43f1d3b5716d4030a7379490d36ddb/chrome/browser/ui/views/chrome_views_delegate_chromeos.cc
[modify] https://crrev.com/6f86cb2b3c43f1d3b5716d4030a7379490d36ddb/chrome/browser/ui/views/task_manager_view_browsertest.cc

Comment 5 by est...@chromium.org, Jun 24 2018

Status: Fixed (was: Started)
Labels: Proj-Mustash

Sign in to add a comment