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

Issue 713560 link

Starred by 20 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug



Sign in to add a comment

ARC is unusable with broken WM input and browser crash

Project Member Reported by nya@chromium.org, Apr 20 2017

Issue description

Chrome Version:
  M60: ToT
  M59: 59.0.3071.16
  M58: 58.0.3029.78
OS: Chrome OS
Board: samus

What steps will reproduce the problem?
1. Login
2. Opt-in to ARC. This will succeed.
3. Play Store is automatically opened with ToS confirmation dialog. User input to this window is ignored (no click, no touch)
4. Click the Play Store icon on the shelf to trigger minimize. Then the browser crash.

This is due to a change in  Issue 711514 .

Internal bug: b/37503897

 

Comment 1 by uekawa@google.com, Apr 20 2017

Labels: ReleaseBlock-Stable ReleaseBlock-Beta Merge-Request-58 Merge-Request-59 M-59 M-58

Comment 2 by nya@chromium.org, Apr 20 2017

The revert is under review: https://codereview.chromium.org/2827233002/

(Is it okay to set Merge-Request-* before landing changes?)

Project Member

Comment 3 by sheriffbot@chromium.org, Apr 20 2017

Labels: -Merge-Request-58 Merge-Review-58 Hotlist-Merge-Review
This bug requires manual review: Only 4 days from stable, we might already have a stable candidate build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-58 -Merge-Request-59 Merge-Approved-59 Merge-Approved-58
Merge approved for this revert.

Comment 5 by nya@chromium.org, Apr 20 2017

PST people: Please feel free to land the revert and cherry-pick to release branches while I'm sleeping.
Sent to CQ
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 20 2017

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

commit b650cb773adbf72f3f731e0bc56e22d55c521391
Author: nya <nya@chromium.org>
Date: Thu Apr 20 17:55:22 2017

Revert of Do not hide shadow underlay for max/fullscreen state even if the client disables the shadow. (patchset #3 id:40001 of https://codereview.chromium.org/2820493004/ )

Reason for revert:
This change broke basic WM interaction of ARC and caused browser crash.  https://crbug.com/713560 

Original issue's description:
> Do not hide shadow underlay for max/fullscreen state even if the client disables the shadow.
>
> During the state transition, ARC preserves windows being deleted,
> however the main window is deleted (thus, window frame is empty) which disables the shadow.
>
> BUG= 711514 
> TEST=covered by unit tests.
>  manual. Instal & start "Clash Royal", then F4 to toggle fullscreen.
>
> Review-Url: https://codereview.chromium.org/2820493004
> Cr-Commit-Position: refs/heads/master@{#464819}
> Committed: https://chromium.googlesource.com/chromium/src/+/ae72f92d7caba6d32ba2a11f32943f29af15c3d8

TBR=reveman@chromium.org,oshima@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 713560 

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

[modify] https://crrev.com/b650cb773adbf72f3f731e0bc56e22d55c521391/components/exo/shell_surface.cc
[modify] https://crrev.com/b650cb773adbf72f3f731e0bc56e22d55c521391/components/exo/shell_surface_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 20 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29b3bfe7c7371600d717d66502307b1e676d7c2c

commit 29b3bfe7c7371600d717d66502307b1e676d7c2c
Author: Victor Hsieh <victorhsieh@google.com>
Date: Thu Apr 20 18:23:49 2017

Revert of Do not hide shadow underlay for max/fullscreen state even if the client disables the shadow. (patchset #3 id:40001 of https://codereview.chromium.org/2820493004/ )

Reason for revert:
This change broke basic WM interaction of ARC and caused browser crash.  https://crbug.com/713560 

Original issue's description:
> Do not hide shadow underlay for max/fullscreen state even if the client disables the shadow.
>
> During the state transition, ARC preserves windows being deleted,
> however the main window is deleted (thus, window frame is empty) which disables the shadow.
>
> BUG= 711514 
> TEST=covered by unit tests.
>  manual. Instal & start "Clash Royal", then F4 to toggle fullscreen.
>
> Review-Url: https://codereview.chromium.org/2820493004
> Cr-Commit-Position: refs/heads/master@{#464819}
> Committed: https://chromium.googlesource.com/chromium/src/+/ae72f92d7caba6d32ba2a11f32943f29af15c3d8

TBR=reveman@chromium.org,oshima@chromium.org
BUG= 713560 

Review-Url: https://codereview.chromium.org/2827233002
Cr-Commit-Position: refs/heads/master@{#466063}
(cherry picked from commit b650cb773adbf72f3f731e0bc56e22d55c521391)

Review-Url: https://codereview.chromium.org/2829723004 .
Cr-Commit-Position: refs/branch-heads/3071@{#90}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/29b3bfe7c7371600d717d66502307b1e676d7c2c/components/exo/shell_surface.cc
[modify] https://crrev.com/29b3bfe7c7371600d717d66502307b1e676d7c2c/components/exo/shell_surface_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 20 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f053a5346cc243578179e26b0331f5e0f6fe4316

commit f053a5346cc243578179e26b0331f5e0f6fe4316
Author: Victor Hsieh <victorhsieh@google.com>
Date: Thu Apr 20 19:59:40 2017

Revert of Do not hide shadow underlay for max/fullscreen state even if the client disables the shadow. (patchset #3 id:40001 of https://codereview.chromium.org/2820493004/ )

Reason for revert:
This change broke basic WM interaction of ARC and caused browser crash.  https://crbug.com/713560 

Original issue's description:
> Do not hide shadow underlay for max/fullscreen state even if the client disables the shadow.
>
> During the state transition, ARC preserves windows being deleted,
> however the main window is deleted (thus, window frame is empty) which disables the shadow.
>
> BUG= 711514 
> TEST=covered by unit tests.
>  manual. Instal & start "Clash Royal", then F4 to toggle fullscreen.
>
> Review-Url: https://codereview.chromium.org/2820493004
> Cr-Commit-Position: refs/heads/master@{#464819}
> Committed: https://chromium.googlesource.com/chromium/src/+/ae72f92d7caba6d32ba2a11f32943f29af15c3d8

TBR=reveman@chromium.org,oshima@chromium.org
BUG= 713560 

Review-Url: https://codereview.chromium.org/2827233002
Cr-Commit-Position: refs/heads/master@{#466063}
(cherry picked from commit b650cb773adbf72f3f731e0bc56e22d55c521391)

Review-Url: https://codereview.chromium.org/2836433002 .
Cr-Commit-Position: refs/branch-heads/3029@{#758}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/f053a5346cc243578179e26b0331f5e0f6fe4316/components/exo/shell_surface.cc
[modify] https://crrev.com/f053a5346cc243578179e26b0331f5e0f6fe4316/components/exo/shell_surface_unittest.cc

Comment 10 by nya@chromium.org, Apr 21 2017

Status: Fixed (was: Started)
Thanks Victor for merging!

Comment 11 by uekawa@google.com, Apr 21 2017

Cc: rjahagir@chromium.org pucchakayala@chromium.org songsuk@chromium.org ajha@chromium.org kavvaru@chromium.org brajkumar@chromium.org
 Issue 714077  has been merged into this issue.
Cc: elijahtaylor@chromium.org skuhne@chromium.org lpique@chromium.org denniskempin@chromium.org
 Issue 714362  has been merged into this issue.
 Issue 714376  has been merged into this issue.
Status: Verified (was: Fixed)
9334.69.0, 58.0.3029.112

Sign in to add a comment