New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 721646 link

Starred by 1 user
Status: Archived
Owner:
osh...@chromium.org
Closed: May 2017
Cc:
domlasko...@chromium.org
█ skuhne@chromium.org
█ reve...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Refactor backdrop code in maximize mode

Project Member Reported by osh...@chromium.org, May 12 2017 
Refactor backdrop code in maximize mode and use it for exo.
This will have following benefits:

1) The accessibility feature added to exo can be used for
   non exo scenario.
2) Exo's minmize animation for maximized window will be better.
3) We can add fade in/out animation for backdrop for exo windows.
4) It'll reduce ash dependency in exo.

Comment 1 by osh...@chromium.org, May 12 2017

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, May 17 2017 

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

commit 04936c54ed2396ae54cd824e24f11151e0e11948
Author: oshima <oshima@chromium.org>
Date: Wed May 17 09:44:32 2017

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.

BUG= 721646 
TEST=coverted by unit tests

Review-Url: https://codereview.chromium.org/2884623002
Cr-Commit-Position: refs/heads/master@{#472401}

[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/BUILD.gn
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/test/workspace_controller_test_api.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/test/workspace_controller_test_api.h
[add] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/maximize_mode/maximize_mode_backdrop_delegate_impl.cc
[add] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/maximize_mode/maximize_mode_backdrop_delegate_impl.h
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/maximize_mode/maximize_mode_window_manager.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/maximize_mode/maximize_mode_window_manager.h
[delete] https://crrev.com/4c356ae265cc3276e7a3fb89c07bcbb13d8bf5c0/ash/wm/maximize_mode/workspace_backdrop_delegate.cc
[delete] https://crrev.com/4c356ae265cc3276e7a3fb89c07bcbb13d8bf5c0/ash/wm/maximize_mode/workspace_backdrop_delegate.h
[add] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/workspace/backdrop_controller.cc
[add] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/workspace/backdrop_controller.h
[add] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/workspace/backdrop_delegate.h
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/workspace/workspace_layout_manager.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/workspace/workspace_layout_manager.h
[delete] https://crrev.com/4c356ae265cc3276e7a3fb89c07bcbb13d8bf5c0/ash/wm/workspace/workspace_layout_manager_backdrop_delegate.h
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/workspace/workspace_layout_manager_unittest.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/workspace_controller.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ash/wm/workspace_controller.h
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_unittest.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/BUILD.gn
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/shell_surface.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/shell_surface.h
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/shell_surface_unittest.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/wm_helper.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/wm_helper.h
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/wm_helper_ash.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/wm_helper_ash.h
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/wm_helper_mus.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/components/exo/wm_helper_mus.h
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/04936c54ed2396ae54cd824e24f11151e0e11948/ui/aura/client/aura_constants.h

Comment 3 by osh...@chromium.org, May 17 2017

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, May 17 2017 

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

commit b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3
Author: kolos <kolos@chromium.org>
Date: Wed May 17 13:17:08 2017

Revert of Refactor backdrop  (patchset #6 id:220001 of https://codereview.chromium.org/2884623002/ )

Reason for revert:
FindIt detected this CL as culprit.
https://findit-for-me.appspot.com/waterfall/build-failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/21215

Original issue's description:
> 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.
>
> 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

TBR=reveman@chromium.org,jamescook@chromium.org,sky@chromium.org,oshima@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 721646 

Review-Url: https://codereview.chromium.org/2887103002
Cr-Commit-Position: refs/heads/master@{#472436}

[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/BUILD.gn
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/test/workspace_controller_test_api.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/test/workspace_controller_test_api.h
[delete] https://crrev.com/12f4051195bd555d7449b331db556bbff0aae1cd/ash/wm/maximize_mode/maximize_mode_backdrop_delegate_impl.cc
[delete] https://crrev.com/12f4051195bd555d7449b331db556bbff0aae1cd/ash/wm/maximize_mode/maximize_mode_backdrop_delegate_impl.h
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/maximize_mode/maximize_mode_window_manager.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/maximize_mode/maximize_mode_window_manager.h
[add] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/maximize_mode/workspace_backdrop_delegate.cc
[add] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/maximize_mode/workspace_backdrop_delegate.h
[delete] https://crrev.com/12f4051195bd555d7449b331db556bbff0aae1cd/ash/wm/workspace/backdrop_controller.cc
[delete] https://crrev.com/12f4051195bd555d7449b331db556bbff0aae1cd/ash/wm/workspace/backdrop_controller.h
[delete] https://crrev.com/12f4051195bd555d7449b331db556bbff0aae1cd/ash/wm/workspace/backdrop_delegate.h
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/workspace/workspace_layout_manager.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/workspace/workspace_layout_manager.h
[add] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/workspace/workspace_layout_manager_backdrop_delegate.h
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/workspace/workspace_layout_manager_unittest.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/workspace_controller.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ash/wm/workspace_controller.h
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_unittest.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/BUILD.gn
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/shell_surface.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/shell_surface.h
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/shell_surface_unittest.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/wm_helper.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/wm_helper.h
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/wm_helper_ash.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/wm_helper_ash.h
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/wm_helper_mus.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/components/exo/wm_helper_mus.h
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3/ui/aura/client/aura_constants.h

Comment 5 by osh...@chromium.org, May 17 2017 

Looks like my test found an existing memory bug in accessibility code :)
I'll fix it and reland.

Comment 6 by sdantul...@chromium.org, May 22 2017

Status: Assigned (was: Fixed)
Re-opening as per c#5

Comment 7 by osh...@chromium.org, May 22 2017

Status: Fixed (was: Assigned)
It's landed https://codereview.chromium.org/2890733005/. Not sure why it didn't update this bug.

Comment 8 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
►

Sign in to add a comment