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

Issue 591474 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

When dragging tab out of window, dark border around new window is missing

Project Member Reported by tbuck...@chromium.org, Mar 2 2016

Issue description

Chrome Version: 50.0.2657.0
OS Version: 7956.1.0

What steps will reproduce the problem?
1. Create a window (A) with multiple tabs
2. Drag a tab out of the window (A) to create a new window (B)
3. Observe the shadow/border around the new window (B)

What is the expected result?
There should be a dark border around the window

What happens instead of that?
The border is missing, and only a shadow is shown.

Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (X11; CrOS x86_64 7956.1.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2657.0 Safari/537.36



 
Cc: -tdander...@chromium.org sgabr...@chromium.org kuscher@chromium.org
Labels: ReleaseBlock-Stable M-50
Owner: tdander...@chromium.org
Yeah, it only seems to affect focused windows. This seems like a blocker
Labels: Needs-Feedback
If I'm understanding the bug description correctly, I can also reproduce this when --top-chrome-md is set to non-material. Tom, can you please verify that this is not MD-specific?
Status: Started (was: Assigned)
It looks like this isn't a super-recent regression. I'm performing a bisect.
Cc: tdander...@chromium.org
Labels: -M-50 -Proj-MaterialDesign-NativeUI -Needs-Feedback M-49
Owner: loyso@chromium.org
Status: Assigned (was: Started)
This regression isn't specific to material design, and was caused by https://codereview.chromium.org/1536833004. It's also present in M-49, though I'm not sure whether this should be considered an M-49 stable blocker.

Attached are expected and actual images for reference (window (A) on the left, window (B) on the right). 

Note that the border and shadow of window (B) will appear after clicking on window (A), i.e., once window (B) becomes inactive.
ash-frame-actual.png
55.9 KB View Download
ash-frame-expected.png
57.4 KB View Download

Comment 5 by loyso@chromium.org, Mar 4 2016

Status: Started (was: Assigned)
JFYI, You can disable  https://codereview.chromium.org/1536833004 CL
with "Disable UI compositor animation timelines" flag in chrome://flags.

It should remove the bug.

Comment 6 by loyso@chromium.org, Mar 4 2016

Just random observation: ash-frame-actual.png - the shadow is still there, but it's very light.
re c#5, is this flag enabled by default?

Comment 8 by loyso@chromium.org, Mar 8 2016

No, it's not enabled by default.
Unrelated to flag, here is the fix: https://codereview.chromium.org/1776513002/
Labels: -ReleaseBlock-Stable M-50
ok, let's merge in ToT/M50. I won;t block initial stable on this but we can consider for a later M49 (please merge-request) once validated that problem is resolved and no side effects in dev/beta
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 9 2016

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

commit 3338dfbd7f61e4ebb5a1e1519e0ef930cd7499d8
Author: loyso <loyso@chromium.org>
Date: Wed Mar 09 03:07:32 2016

UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines inside same Compositor(AnimationHost) without any animation losses.

See  http://crbug.com/592873  (To be designed separately)

BUG= 591474 

Review URL: https://codereview.chromium.org/1776513002

Cr-Commit-Position: refs/heads/master@{#380047}

[modify] https://crrev.com/3338dfbd7f61e4ebb5a1e1519e0ef930cd7499d8/ui/compositor/layer_animator.cc
[modify] https://crrev.com/3338dfbd7f61e4ebb5a1e1519e0ef930cd7499d8/ui/compositor/layer_animator.h
[modify] https://crrev.com/3338dfbd7f61e4ebb5a1e1519e0ef930cd7499d8/ui/compositor/layer_animator_unittest.cc

Status: Fixed (was: Started)
Verify it in next canary.
Labels: Merge-Request-50 Merge-Request-49

Comment 13 by tin...@google.com, Mar 10 2016

Labels: -Merge-Request-49 Merge-Review-49 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M49), manual review required.

Comment 14 by tin...@google.com, Mar 10 2016

Labels: -Merge-Request-50 Merge-Approved-50 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M50 (branch: 2661)

Comment 15 by tin...@google.com, Mar 10 2016

Labels: -Merge-Request-49 Merge-Review-49 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M49), manual review required.
Project Member

Comment 16 by bugdroid1@chromium.org, Mar 10 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aacac20280640e01e261b003078041e211f250dc

commit aacac20280640e01e261b003078041e211f250dc
Author: Alexey Baskakov <loyso@chromium.org>
Date: Thu Mar 10 04:10:06 2016

UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines inside same Compositor(AnimationHost) without any animation losses.

See  http://crbug.com/592873  (To be designed separately)

BUG= 591474 

Review URL: https://codereview.chromium.org/1776513002

Cr-Commit-Position: refs/heads/master@{#380047}
(cherry picked from commit 3338dfbd7f61e4ebb5a1e1519e0ef930cd7499d8)

Review URL: https://codereview.chromium.org/1778213003 .

Cr-Commit-Position: refs/branch-heads/2661@{#170}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/aacac20280640e01e261b003078041e211f250dc/ui/compositor/layer_animator.cc
[modify] https://crrev.com/aacac20280640e01e261b003078041e211f250dc/ui/compositor/layer_animator.h
[modify] https://crrev.com/aacac20280640e01e261b003078041e211f250dc/ui/compositor/layer_animator_unittest.cc

Comment 17 by loyso@chromium.org, Mar 17 2016

Labels: -Hotlist-Merge-Approved Merge-Request-49

Comment 18 by tin...@google.com, Mar 17 2016

Labels: -Merge-Request-49
[Automated comment] Request affecting a post-stable build (M49), manual review required.
Labels: -Merge-Review-49 Merge-Approved-49

Comment 20 by loyso@chromium.org, Apr 18 2016

It was too late. Reject M49 merge?

Comment 21 by loyso@chromium.org, Apr 21 2016

Labels: Merge-Rejected-49
Project Member

Comment 22 by sheriffbot@chromium.org, Apr 29 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 23 by sheriffbot@chromium.org, May 3 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-49
Status: Verified (was: Fixed)
Verified on ChromeOS TOT (8314.0.0, 52.0.2733.0) and M50 (7978.74.0, 50.0.2661.103)

Sign in to add a comment