New issue
Advanced search Search tips

Issue 591535 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Spinners stay on-screen if browser toolbar hides while page is still loading

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

Issue description

Version: 49.0.2623.59
OS: ChromeOS

What steps will reproduce the problem?
1. Open a window.
2. Press the full-screen button to fullscreen the window. This also switches the browser toolbar to switch to an auto-hiding mode.
3. Type in a URL and let Chrome start navigating to it, so there is a spinner displayed in the tab.
4. Move the cursor, so that the toolbar auto-hides.

What is the expected output? What do you see instead?

Expect that the spinner disappears, since the toolbar is no longer visible.

Instead there is a lingering static spinner overlaid on whatever content is in the window. If you re-show the toolbar, or create a new tab in the window and switch to it, then the spinner stays overload.

See attached screenshots and notice the faint grey spinner in each case.
 
Screenshot 2016-03-02 at 13.35.07.png
4.9 KB View Download
Screenshot 2016-03-02 at 13.34.27.png
2.0 KB View Download
Screenshot 2016-03-02 at 13.34.22.png
1.6 KB View Download

Comment 1 by w...@chromium.org, Mar 2 2016

Labels: M-49
Cc: tapted@chromium.org
Components: -Internals>Compositing>Ubercomp Internals>Views>Desktop
Owner: sky@chromium.org
Cc: -tapted@chromium.org sky@chromium.org
Labels: -Pri-2 Pri-1
Owner: tapted@chromium.org
Status: Assigned (was: Untriaged)
Doh - this is probably me, I'll take a look
Status: Started (was: Assigned)
https://codereview.chromium.org/1758183002/
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 3 2016

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

commit fde5de1bc63b656dabaafef3e5b59badcc613a22
Author: tapted <tapted@chromium.org>
Date: Thu Mar 03 23:50:55 2016

CrOS: Don't paint tab-loading spinners in layers while in immersive fullscreen.

This regressed in r362878 which didn't handle a case where animations
occur in immersive fullscreen.

To fix, disable layer-painting of tab spinners while in fullscreen.
In most cases the tab is hidden or animating; layers shouldn't
be used. When the tab strip is fully revealed we could use layers,
but the benefit isn't worth the extra logic.

BUG= 591535 
TEST=On CrOS, enter immersive fullscreen, reload a page, reveal the
tab strip while the page is still loading, then move the mouse down
to dismiss the tab strip. Ensure a frozen throbber doesn't stick
around, floating over the page.

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

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

[modify] https://crrev.com/fde5de1bc63b656dabaafef3e5b59badcc613a22/chrome/browser/ui/views/frame/immersive_mode_controller_ash_unittest.cc
[modify] https://crrev.com/fde5de1bc63b656dabaafef3e5b59badcc613a22/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/fde5de1bc63b656dabaafef3e5b59badcc613a22/chrome/browser/ui/views/tabs/tab_strip.cc

Components: -Internals>Views>Desktop UI>Browser>FullScreen Internals>Views
Labels: Merge-Request-50 Merge-Request-49
Ideally we would merge this CrOS-only fix to m49 and m50, but it might not be possible to verify on an official CrOS release until next week. This landed in 51.0.2667.0, but there are still no CrOS official builds on any channel for m51 -- per go/ilczc . My canary-channel daisy CrOS device is still at 50.0.2657.0 (and isn't lagging per go/ilczc ).

I've poked around on the 51.0.2670.0 Canary looking for any issues, and haven't discovered anything.

I've verified on a local linux-chromeos build, and the patch is quite small.

Adding the merge-requested labels for m49 and m50, to get this on the release maintainers' radars. This is a pretty annoying UI glitch for users of immersive fullscreen on CrOS.

Comment 7 by tin...@google.com, Mar 7 2016

Labels: -Merge-Request-49 Merge-Review-49 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M49, manual review required.

Comment 8 by tin...@google.com, Mar 7 2016

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

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

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

commit 5196d5924deff29d6a4bbe4c6ccdc272a2686397
Author: Trent Apted <tapted@chromium.org>
Date: Wed Mar 09 08:12:35 2016

[merge] CrOS: Don't paint tab-loading spinners in layers while in immersive fullscreen.

This regressed in r362878 which didn't handle a case where animations
occur in immersive fullscreen.

To fix, disable layer-painting of tab spinners while in fullscreen.
In most cases the tab is hidden or animating; layers shouldn't
be used. When the tab strip is fully revealed we could use layers,
but the benefit isn't worth the extra logic.

TBR=tapted@chromium.org
BUG= 591535 
TEST=On CrOS, enter immersive fullscreen, reload a page, reveal the
tab strip while the page is still loading, then move the mouse down
to dismiss the tab strip. Ensure a frozen throbber doesn't stick
around, floating over the page.

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

Cr-Commit-Position: refs/heads/master@{#379140}
(cherry picked from commit fde5de1bc63b656dabaafef3e5b59badcc613a22)

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

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

[modify] https://crrev.com/5196d5924deff29d6a4bbe4c6ccdc272a2686397/chrome/browser/ui/views/frame/immersive_mode_controller_ash_unittest.cc
[modify] https://crrev.com/5196d5924deff29d6a4bbe4c6ccdc272a2686397/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/5196d5924deff29d6a4bbe4c6ccdc272a2686397/chrome/browser/ui/views/tabs/tab_strip.cc

Project Member

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

Labels: -merge-approved-49 merge-merged-2623
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/449279b9ecb1f2b2ff1d20f4a296fcc880badc7c

commit 449279b9ecb1f2b2ff1d20f4a296fcc880badc7c
Author: Trent Apted <tapted@chromium.org>
Date: Wed Mar 09 08:21:11 2016

[merge-m49] CrOS: Don't paint tab-loading spinners in layers while in immersive fullscreen.

This regressed in r362878 which didn't handle a case where animations
occur in immersive fullscreen.

To fix, disable layer-painting of tab spinners while in fullscreen.
In most cases the tab is hidden or animating; layers shouldn't
be used. When the tab strip is fully revealed we could use layers,
but the benefit isn't worth the extra logic.

TBR=tapted@chromium.org
BUG= 591535 
TEST=On CrOS, enter immersive fullscreen, reload a page, reveal the
tab strip while the page is still loading, then move the mouse down
to dismiss the tab strip. Ensure a frozen throbber doesn't stick
around, floating over the page.

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

Cr-Commit-Position: refs/heads/master@{#379140}
(cherry picked from commit fde5de1bc63b656dabaafef3e5b59badcc613a22)

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

Cr-Commit-Position: refs/branch-heads/2623@{#604}
Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907}

[modify] https://crrev.com/449279b9ecb1f2b2ff1d20f4a296fcc880badc7c/chrome/browser/ui/views/frame/immersive_mode_controller_ash_unittest.cc
[modify] https://crrev.com/449279b9ecb1f2b2ff1d20f4a296fcc880badc7c/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/449279b9ecb1f2b2ff1d20f4a296fcc880badc7c/chrome/browser/ui/views/tabs/tab_strip.cc

Status: Fixed (was: Started)
Project Member

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

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/bling/chromium.git/+/449279b9ecb1f2b2ff1d20f4a296fcc880badc7c

commit 449279b9ecb1f2b2ff1d20f4a296fcc880badc7c
Author: Trent Apted <tapted@chromium.org>
Date: Wed Mar 09 08:21:11 2016

Labels: VerifyIn-52
Labels: VerifyIn-53
Labels: VerifyIn-54

Comment 17 by ka...@chromium.org, Aug 31 2016

Labels: Bulk-Verified
Status: Verified (was: Fixed)

Sign in to add a comment