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

Issue 681525 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression:Incognito window 'minimize', 'restore down' and close button shift upwards after toggling out of fullscreen mode.

Reported by vku...@etouch.net, Jan 16 2017

Issue description

Chrome Version:57.0.2983.0 (Official Build) (64-bit)266a86c94d34f3c7c3d22ee00624636b801f80d3-refs/heads/master@{#443819}-32/64 bit 
OS:Windows(7,8,8.1,10)
URL:https://chrome.google.com/webstore/detail/galaxy-view/dcbeddldohkakodfncjnkkjfojggbahp?hl=en

What steps will reproduce the problem?
(1)Launch chrome and install "Galaxy-View" theme from above mentioned url
(2)Click on wrench > New incognito window, press 'F11' to toggle to fullscreen mode and click on 'import bookmarks now..' button
(3)Click on 'undo' button of theme infobar from original window, switch to incognito window and again press 'F11' to toggle out from fullscreen mode.
(4)Observe the 'minimize', 'restore down' and close button of incognito window.

Actual: Incognito window 'minimize', 'restore down' and close button shift upwards after toggling out of fullscreen mode.

Expected: Incognito window 'minimize', 'restore down' and close button should not shift upward after toggling out of fullscreen mode.

This is a regression issue broken in 'M56' and below is the manual regression range:
Good Build: 56.0.2920.0
Bad Build:  56.0.2922.0

Note: Issue not seen on Mac & Linux OS.


 
ActualTheme.png
36.6 KB View Download
Expected_Theme.mp4
1.0 MB View Download

Comment 1 by vku...@etouch.net, Jan 16 2017

Actual_Theme.mp4
1012 KB View Download

Comment 2 by hdodda@chromium.org, Jan 16 2017

Labels: Needs-Bisect
Status: Untriaged (was: Unconfirmed)
Labels: -Needs-Bisect hasbisect-per-revision
Owner: sky@chromium.org
Status: Assigned (was: Untriaged)
Able to reproduce the issue on Windows 10 using chrome reported version #57.0.2983.0.
Issue is not seen on Mac and Linux.

Bisect Information:
=====================
Good build: 56.0.2920.0	 Revision(432057)

Bad Build : 56.0.2922.0	 Revision(432511)

Change Log URL: 
https://chromium.googlesource.com/chromium/src/+log/0d2133839bd06c22c31bfc1695035a13811163ab..306338a1ac90527e74f9604cd5b202d1cdf2cb63

From the above change log suspecting below change

Review url: https://codereview.chromium.org/2488993002

sky@ - Could you please check whether this is caused with respect to author(atimoxin) change, if not please help us in assigning it to the right owner.

Note: Assigning to one of the reviewer, as the author doesn't have a chromium id.

Thanks...!!
Labels: ReleaseBlock-Stable
Adding blocker label, please undo if not the case.
Your bug is labelled as Stable Release Block, please make sure to land the fix and get it merged into the release branch ASAP so we can take it for next week's Beta release for Desktop. Thank you!
Cc: rbasuvula@chromium.org
Still able to reproduce the issue on Win 7 using latest chrome version #57.0.2989.0.

sky@ Could you please look into this issue.

Thanks!
Labels: -M-56 M-57
We are cutting stable RC today 01/24 .
Punting to M57 since the testcases doesn't seem to be a blocker for now.
sky@ Gentle Ping! can we get any latest update on this issue since this is marked as RB-Stable.
A friendly reminder that M57 Stable is launch is coming soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!

Just to update the latest behavior of this issue, Able to reproduce it on Windows-10 using chrome latest canary #58.0.3012.0. 

sky@ Since this issue is marked as RB-Stable can we get any latest update on this issue? we're getting closer to M57 early stable launch.

Thanks!

Comment 11 by sky@chromium.org, Feb 15 2017

Owner: ananta@chromium.org
Ananta, I don't have a win10 machine. Could you try reverting the identified patch locally and see if it fixes the issue?
A friendly reminder that M57 Stable is launch is coming VERY soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch (2987) ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!
Project Member

Comment 13 by bugdroid1@chromium.org, Feb 19 2017

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

commit f7c7217dd882eac40d5d07e441db5ef9fe3c94a4
Author: ananta <ananta@chromium.org>
Date: Sun Feb 19 02:43:45 2017

Fix a crash due to reentrancy in the widget while processing a theme changed notification after coming out of fullscreen mode

The crash occurs as views are removed/added while we are propagating theme changed notifications to the view hierarchy.

Proposed fix is to add a recursion guard flag processing_theme_changed_ which is set in
Widget::OnNativeThemeUpdated before propagating the theme changed notification and reset after
the function returns. We check the flag in Widget::FrameTypeChanged() and bail. This function is called becaue
Widget is registered as an observer on the BrowserView.

BUG= 681525 

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

[modify] https://crrev.com/f7c7217dd882eac40d5d07e441db5ef9fe3c94a4/ui/views/widget/widget.cc
[modify] https://crrev.com/f7c7217dd882eac40d5d07e441db5ef9fe3c94a4/ui/views/widget/widget.h

Labels: TE-Verified-M58 TE-Verified-58.0.3019.0
Rechecked this on chrome canary version 58.0.3019.0 on Windows 10 and Windows 7 following the steps provided and fix is working as intended. 'Minimize', 'restore down' and close buttons remain the desired position.

Adding TE-Verified labels.
Thank you Ranjit.

Ananta, please request a merge to M57. Thank you.

URGENT - PTAL ASAP.

We're getting VERY close to M57 Stable promotion. And 
this issue is marked as M57 stable release blocker. Pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion).

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label or move to M58.

Thank you.
Labels: Merge-Request-57
Project Member

Comment 18 by sheriffbot@chromium.org, Feb 22 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
If possible, could you please merge your change to M57 branch 2987 by 5:00 PM PT today, Wednesday (02/22) so we can pick it up for next beta release. 
Thank you.
Project Member

Comment 20 by bugdroid1@chromium.org, Feb 23 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/feff536f7394156055e3c86df0dd5901e2f49bb2

commit feff536f7394156055e3c86df0dd5901e2f49bb2
Author: ananta <ananta@chromium.org>
Date: Thu Feb 23 22:27:59 2017

Merging to M57

Fix a crash due to reentrancy in the widget while processing a theme changed notification after coming out of fullscreen mode

The crash occurs as views are removed/added while we are propagating theme changed notifications to the view hierarchy.

Proposed fix is to add a recursion guard flag processing_theme_changed_ which is set in
Widget::OnNativeThemeUpdated before propagating the theme changed notification and reset after
the function returns. We check the flag in Widget::FrameTypeChanged() and bail. This function is called becaue
Widget is registered as an observer on the BrowserView.

BUG= 681525 
TBR=sky
NOTRY=true
NOPRESUBMIT=true

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

Review-Url: https://codereview.chromium.org/2711853004
Cr-Commit-Position: refs/branch-heads/2987@{#670}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/feff536f7394156055e3c86df0dd5901e2f49bb2/ui/views/widget/widget.cc
[modify] https://crrev.com/feff536f7394156055e3c86df0dd5901e2f49bb2/ui/views/widget/widget.h

Status: Fixed (was: Assigned)
Labels: TE-Verified-57.0.2987.88 TE-Verified-M57
Verified the issue on Win 10 using 57.0.2987.88 and its working fine now.
681525_Mar_1.mp4
9.5 MB View Download

Sign in to add a comment