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

Issue 762816 link

Starred by 4 users
Status: Fixed
Owner:
osh...@chromium.org
Closed: Dec 2017
Cc:
x...@chromium.org
█ mtomasz@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Support for client side controller bounds and states of windows in ash.

Project Member Reported by mtomasz@chromium.org, Sep 7 2017 
Ash controls window states and bounds, which doesn't work well with ARC. It required us to add some workarounds, such as:
1. Temporary setting window state and reverting it later when ARC disagrees.
2. Temporary setting incorrect geometry until ARC overwrites it in ShellSurface::OnSurfaceCommit.
3. Disabling some of the bounds changes in ash::wm::DefaultState.
4. Implementing custom cross-fade for window states in exo.

By adding native support for out-of-ash controller bounds and states, we should be able to remove the above hacks.

Comment 1 by osh...@chromium.org, Oct 18 2017

Cc: x...@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 8 2017 

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

commit a0a7001231e7b651bc64c9641b1c3a80d0525df2
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Wed Nov 08 19:44:00 2017

Introduce ClientControlledState

* This will be used to implement the window state management for ARC++ windows.
 The state of the ARC++ is controlled by the Android side, so we first need to delegate
 the request and wait for the final state. Bounds is also controlled by Android side,
 so we need to send the request and wait for actual bounds change from Android side.
* Refactored DefaultState and introduced BaseState to share the common implementation.

BUG= 762816 
TEST=Covered by unit tests

Change-Id: Ic963c04fb712902ba68f532a03fc94fff4bbe770
Reviewed-on: https://chromium-review.googlesource.com/756154
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514910}
[modify] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/BUILD.gn
[add] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/base_state.cc
[add] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/base_state.h
[add] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/client_controlled_state.cc
[add] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/client_controlled_state.h
[add] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/client_controlled_state_unittest.cc
[modify] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/default_state.cc
[modify] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/default_state.h
[modify] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/window_state.h
[modify] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/wm_event.cc
[modify] https://crrev.com/a0a7001231e7b651bc64c9641b1c3a80d0525df2/ash/wm/wm_event.h
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 9 2017 

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

commit cbdd097c79995b5162019cfa5ab0597f83933e78
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Thu Nov 09 16:49:44 2017

Allow client to change the pinned window's bounds.

BUG= 762816 
TEST=covered by unit test

Change-Id: I240054a306f67762b4cfb072dbd83c61b2a7b0d2
Reviewed-on: https://chromium-review.googlesource.com/760196
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515183}
[modify] https://crrev.com/cbdd097c79995b5162019cfa5ab0597f83933e78/ash/wm/base_state.cc
[modify] https://crrev.com/cbdd097c79995b5162019cfa5ab0597f83933e78/ash/wm/client_controlled_state.cc
[modify] https://crrev.com/cbdd097c79995b5162019cfa5ab0597f83933e78/ash/wm/client_controlled_state_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 16 2017 

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

commit 87778279b1f9c5cbd0f2a3f650ef73ca477c34d3
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Thu Nov 16 00:51:19 2017

Keep the order of method bodies in sync with header.

BUG= 762816 
TEST=None

Change-Id: Ic194617b04cc318208fad09bb5b10580c13009e3
Reviewed-on: https://chromium-review.googlesource.com/773461
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516926}
[modify] https://crrev.com/87778279b1f9c5cbd0f2a3f650ef73ca477c34d3/ash/wm/default_state.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 1 2017 

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

commit 21b079d1d8b5712fde801a2219c43751121fc6fc
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Fri Dec 01 02:38:15 2017

Use the CustomFrameViewAsh for client controlled shell surface.

This will not change the functionality now, but will be used
when caption is moved to chrome for ARC++ apps.

BUG= 762816 

Change-Id: Ia25d13eba37b31677a265b9f3e8b09953a5dd8e5
Reviewed-on: https://chromium-review.googlesource.com/786545
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520811}
[modify] https://crrev.com/21b079d1d8b5712fde801a2219c43751121fc6fc/ash/frame/custom_frame_view_ash.cc
[modify] https://crrev.com/21b079d1d8b5712fde801a2219c43751121fc6fc/ash/frame/custom_frame_view_ash.h
[modify] https://crrev.com/21b079d1d8b5712fde801a2219c43751121fc6fc/ash/wm/client_controlled_state.cc
[modify] https://crrev.com/21b079d1d8b5712fde801a2219c43751121fc6fc/ash/wm/client_controlled_state.h
[modify] https://crrev.com/21b079d1d8b5712fde801a2219c43751121fc6fc/ash/wm/client_controlled_state_unittest.cc
[modify] https://crrev.com/21b079d1d8b5712fde801a2219c43751121fc6fc/components/exo/shell_surface.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 12 2017 

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

commit 2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Tue Dec 12 00:23:41 2017

Delegates state change to Android

Modified ClientControlledStateSurface so that
 - State change are first sent to client (Android) except for PIN/TRUSTED_PIN
 - PIN/TRUSTED_PIN are still handled on server side per security requirement.
   It may leave the the state inconsistent and the same thing can happen in
   the old implmenetation.
 - the bounds change request from chrome side is ignored
   for now and only the geometry change from client is applied.
* Emulate the state change by client in ShellSurface unit tests.

  Animation isn't enabled yet in this CL. It'll be addressed in a separate CL.

BUG= 762816 
TEST=manually tested on device (maximize/fullscreen/minimix/pin/unpin etc)
Existing test should pass with new implementation (with test impl).

Change-Id: Idc8001876f4b1c50d375b1090ce63516f3998ab0
Reviewed-on: https://chromium-review.googlesource.com/809488
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523266}
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/ash/wm/client_controlled_state_unittest.cc
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/ash/wm/window_state.h
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/components/exo/BUILD.gn
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/components/exo/client_controlled_shell_surface.cc
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/components/exo/client_controlled_shell_surface.h
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/components/exo/shell_surface_base.cc
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/components/exo/shell_surface_base.h
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/components/exo/test/exo_test_base.cc
[add] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/components/exo/test/test_client_controlled_state_delegate.cc
[add] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/components/exo/test/test_client_controlled_state_delegate.h
[modify] https://crrev.com/2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8/ui/wm/core/window_util.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 14 2017 

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

commit 1143977a5034b82fb71fb7a932cfafbc560337cd
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Thu Dec 14 02:06:59 2017

Add cross fade animation for maximize/fullscreen/restore state transition.

* Add bounds_change_animation_type to ClientControlledState.
* Use cross fade if the window is maximized/fullscreen-ed or restored.
* Moved WindowStateObserver to ShellSurface because it's not necessary for
  ClientControlledShellSurface.

* Cleanups:
 - Remove UpdateBackdrop virtual method and moved to ClientControlledShellSurface
 - Moved ScopedAnimationsDisabled to ShellSurface.
 - Removed CanAnimateWindowStateTransition.

BUG=b:29406058,  crbug.com/762816 
TEST=updated unit tests. tested on device.

Change-Id: I87a0d27d0edb7a7ce1d8fbefc35a584692cf159c
Reviewed-on: https://chromium-review.googlesource.com/820972
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523983}
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/ash/wm/client_controlled_state.cc
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/ash/wm/client_controlled_state.h
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/ash/wm/client_controlled_state_unittest.cc
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/components/exo/client_controlled_shell_surface.cc
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/components/exo/client_controlled_shell_surface.h
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/components/exo/client_controlled_shell_surface_unittest.cc
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/components/exo/shell_surface.cc
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/components/exo/shell_surface.h
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/components/exo/shell_surface_base.cc
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/components/exo/shell_surface_base.h
[modify] https://crrev.com/1143977a5034b82fb71fb7a932cfafbc560337cd/components/exo/test/test_client_controlled_state_delegate.cc

Comment 8 by osh...@chromium.org, Dec 19 2017

Labels: -M-63 M-64 ReleaseBlock-Stable Merge-Request-64
Project Member

Comment 9 by sheriffbot@chromium.org, Dec 19 2017

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by osh...@chromium.org, Dec 19 2017 

This is required for b/67384524

Comment 11 by kbleicher@google.com, Dec 20 2017

Labels: -Merge-Review-64 Merge-Approved-64
Approving the M-64 merge to support b/67384524
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 21 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0

commit c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Thu Dec 21 01:24:34 2017

Delegates state change to Android

Modified ClientControlledStateSurface so that
 - State change are first sent to client (Android) except for PIN/TRUSTED_PIN
 - PIN/TRUSTED_PIN are still handled on server side per security requirement.
   It may leave the the state inconsistent and the same thing can happen in
   the old implmenetation.
 - the bounds change request from chrome side is ignored
   for now and only the geometry change from client is applied.
* Emulate the state change by client in ShellSurface unit tests.

  Animation isn't enabled yet in this CL. It'll be addressed in a separate CL.

BUG= 762816 
TEST=manually tested on device (maximize/fullscreen/minimix/pin/unpin etc)
Existing test should pass with new implementation (with test impl).

Change-Id: Idc8001876f4b1c50d375b1090ce63516f3998ab0
Reviewed-on: https://chromium-review.googlesource.com/809488
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#523266}(cherry picked from commit 2ad2c1b1fe3056cd621a585e9e3fb1b44d4341d8)
Reviewed-on: https://chromium-review.googlesource.com/838422
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#312}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/ash/wm/client_controlled_state_unittest.cc
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/ash/wm/window_state.h
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/components/exo/BUILD.gn
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/components/exo/client_controlled_shell_surface.cc
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/components/exo/client_controlled_shell_surface.h
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/components/exo/shell_surface_base.cc
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/components/exo/shell_surface_base.h
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/components/exo/test/exo_test_base.cc
[add] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/components/exo/test/test_client_controlled_state_delegate.cc
[add] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/components/exo/test/test_client_controlled_state_delegate.h
[modify] https://crrev.com/c33a6bd6b5ebc32c0f9e10a6f97afdca4eff53a0/ui/wm/core/window_util.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 21 2017 

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

commit 9065ccfff347a6301d36d4b2a0fee6aa57cb5a61
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Thu Dec 21 02:29:50 2017

Add cross fade animation for maximize/fullscreen/restore state transition.

* Add bounds_change_animation_type to ClientControlledState.
* Use cross fade if the window is maximized/fullscreen-ed or restored.
* Moved WindowStateObserver to ShellSurface because it's not necessary for
  ClientControlledShellSurface.

* Cleanups:
 - Remove UpdateBackdrop virtual method and moved to ClientControlledShellSurface
 - Moved ScopedAnimationsDisabled to ShellSurface.
 - Removed CanAnimateWindowStateTransition.

BUG=b:29406058,  crbug.com/762816 
TEST=updated unit tests. tested on device.

Change-Id: I87a0d27d0edb7a7ce1d8fbefc35a584692cf159c
Reviewed-on: https://chromium-review.googlesource.com/820972
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#523983}(cherry picked from commit 1143977a5034b82fb71fb7a932cfafbc560337cd)
Reviewed-on: https://chromium-review.googlesource.com/838561
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#317}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/ash/wm/client_controlled_state.cc
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/ash/wm/client_controlled_state.h
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/ash/wm/client_controlled_state_unittest.cc
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/components/exo/client_controlled_shell_surface.cc
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/components/exo/client_controlled_shell_surface.h
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/components/exo/client_controlled_shell_surface_unittest.cc
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/components/exo/shell_surface.cc
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/components/exo/shell_surface.h
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/components/exo/shell_surface_base.cc
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/components/exo/shell_surface_base.h
[modify] https://crrev.com/9065ccfff347a6301d36d4b2a0fee6aa57cb5a61/components/exo/test/test_client_controlled_state_delegate.cc

Comment 14 by osh...@chromium.org, Dec 21 2017

Status: Fixed (was: Started)
►

Sign in to add a comment