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

Issue 831035 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

ClientControlledShellSurface should not just ignore the bounds even if ash needs to adjust the bounds

Project Member Reported by osh...@chromium.org, Apr 10 2018

Issue description

The CL https://chromium-review.googlesource.com/c/chromium/src/+/930661/15/components/exo/client_controlled_shell_surface.cc may ignore the bounds, so therefore the widget size may be in inconsistent state. We should first set the bounds, then request the bounds change.


Experimental cl:
https://chromium-review.googlesource.com/c/chromium/src/+/1002252
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 11 2018

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

commit f8f9b2ef64b225e7b6fdb21ec17a7a260eb06038
Author: Qiang Xu <warx@google.com>
Date: Fri May 11 22:22:06 2018

cros: bounds request to client when ash adjusts bounds

changes:
In crrev.com/c/930661, chrome changes the bounds locally which is backed
by ClientControlledShellSurface without requesting Android for bounds
change. This could lead to widget size inconsistent state.

Bug:  831035 
Test: test coverage
Change-Id: I2d3c0fbc6b39f9637ac55d7f0292c2cc8a08c78e
Reviewed-on: https://chromium-review.googlesource.com/1010906
Commit-Queue: Qiang Xu <warx@google.com>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558039}
[modify] https://crrev.com/f8f9b2ef64b225e7b6fdb21ec17a7a260eb06038/components/exo/client_controlled_shell_surface.cc
[modify] https://crrev.com/f8f9b2ef64b225e7b6fdb21ec17a7a260eb06038/components/exo/client_controlled_shell_surface_unittest.cc

Comment 2 by warx@chromium.org, May 11 2018

Labels: Merge-Request-67
Project Member

Comment 3 by sheriffbot@chromium.org, May 12 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 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), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Is this a M67 regression or a feature enhancement?  Has the modification been tested extensively?

It's a rather large change this late in M67 so I need some context on risk.

Comment 5 by warx@chromium.org, May 14 2018

For bugs in issue 786231 comment #13. It has been tested by oshima and me.
Labels: -Merge-Review-67 Merge-Rejected-67
crbug/786231 is fairly old; certainly not a M67 regression.   I don't see that the changes in #13 were merged to previous milestones.   Rejecting; please reflag and provide more detail if it puts M67 at risk.

Comment 7 by warx@chromium.org, May 15 2018

Cc: abodenha@chromium.org osh...@chromium.org
Status: Fixed (was: Assigned)
shrug.. I think oshima wanted the fix in M67.

Sign in to add a comment