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

Issue 734104 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Bug

Blocked on:
issue 752326



Sign in to add a comment

Reduce flicker in UI while loading

Project Member Reported by est...@chromium.org, Jun 16 2017

Issue description

In these two places, when navigating there's too much flicker between loading and non-loading states.

- tab throbber/favicon
- stop/reload icon

We should investigate delaying switching between states to reduce this.
 

Comment 1 by est...@chromium.org, Jun 16 2017

see go/btyim for more context.

Comment 2 by est...@chromium.org, Jun 16 2017

Example sites where the favicon noticeably flickers post-load:

- yahoo.com
- cyclingnews.com
- metacritic.com

(the pattern seems to be that advertisements are lazily loaded)

If we fix this you'll also be able to see an effect for sites with infinite scroll, like FB's newsfeed. If your connection is fast, then when your scrolling causes more loading, the favicon will remain instead of switching to the throbber for a short time.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 27 2017

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

commit 486f3ed63d1ce98f71b5ca391d0762e3f1425e2d
Author: Scott Violet <sky@chromium.org>
Date: Tue Jun 27 04:53:26 2017

Revert "Views: Don't show throbber for very brief resource loads in tabs, post"

This reverts commit 99c2df715bdf733d259528f9f5f15a35c89a81de.

Reason for revert: as mentioned over IM this makes lots of form submissions feel slow. For example, on codereview.chromium.org when mailing a review comment there is a noticeable delay as compared to before this patch between the time when you click the send button and the throbber appears. What's equally bad is that the stop button changes immediately, but not the throbber.

Original change's description:
> Views: Don't show throbber for very brief resource loads in tabs, post
> initial load.
> 
> Bug:  734104 
> Change-Id: I69bd97a025c3510dd32948ded8ba13e8a620fc38
> Reviewed-on: https://chromium-review.googlesource.com/538922
> Commit-Queue: Evan Stade <estade@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#480903}

TBR=sky@chromium.org,estade@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  734104 
Change-Id: If9af1b087b268bb3ea10f484d8167887c6e2f70d
Reviewed-on: https://chromium-review.googlesource.com/549583
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482540}
[modify] https://crrev.com/486f3ed63d1ce98f71b5ca391d0762e3f1425e2d/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/486f3ed63d1ce98f71b5ca391d0762e3f1425e2d/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/486f3ed63d1ce98f71b5ca391d0762e3f1425e2d/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/486f3ed63d1ce98f71b5ca391d0762e3f1425e2d/chrome/browser/ui/views/tabs/tab_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 12 2017

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

commit b508c72d8fdcc8879989a63d31c2d639009b3d48
Author: Evan Stade <estade@chromium.org>
Date: Wed Jul 12 02:19:08 2017

Delay switching reload button from reload appearance to stop appearance.

This implements ainslie's suggestion "delay the transition from reload
to stop for N seconds", for N = 1.35 simply because that constant is already
in use for a similar UI update delay. I put this one behind a flag because
it is more of a behavioral change (since the button actually does something)
and the correct N value seems unsettled.

A good site to test this on is the Chrome Web Store. When I open a new tab
to the page for a particular in extension/theme/etc., I see 3-4 switches
back and forth between load states. With the flag enabled, I see 0-1 toggles
(depending on how long the initial load takes).

Bug:  734104 
Change-Id: Ib32ebbdc161b75b37fefe2c4ca94fd5af4b56f5f
Reviewed-on: https://chromium-review.googlesource.com/540672
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485797}
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/browser/about_flags.cc
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/browser/ui/views/toolbar/reload_button.cc
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/browser/ui/views/toolbar/reload_button.h
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/browser/ui/views/toolbar/reload_button_unittest.cc
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/common/chrome_switches.cc
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/chrome/common/chrome_switches.h
[modify] https://crrev.com/b508c72d8fdcc8879989a63d31c2d639009b3d48/tools/metrics/histograms/enums.xml

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 12 2017

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

commit 896032108c97ec8e014f5b221afd5b945a4c0389
Author: Evan Stade <estade@chromium.org>
Date: Wed Jul 12 16:18:38 2017

Reland "Views: Don't show throbber for very brief resource loads in tabs
post initial load."

This time, land behind a command line flag (the same one that controls
the stop/reload icon toggles). Also, it was noted that this change could
make certain actions feel sluggish, such as pressing "Save Changes" in
monorail, because the throbber feedback was no longer immediate. To
address this, only suppress switches to the throbber that occur right
after the throbber has stopped showing. That way, as long as you press
"Save Changes" a few seconds after the page has finished loading, the
throbber feedback will be immediate, but we'll still get the suppression
of flicker at the end of a page load (visible, for example, on yahoo.com,
cyclingnews.com, or when navigating to an extension/app/etc. in the
Chrome Web Store).

Bug:  734104 
Change-Id: I5a59fc16785e3a5f30b40a72f03c5c6a67e66337
Reviewed-on: https://chromium-review.googlesource.com/567800
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485985}
[modify] https://crrev.com/896032108c97ec8e014f5b221afd5b945a4c0389/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/896032108c97ec8e014f5b221afd5b945a4c0389/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/896032108c97ec8e014f5b221afd5b945a4c0389/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/896032108c97ec8e014f5b221afd5b945a4c0389/chrome/browser/ui/views/tabs/tab_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 14 2017

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

commit c3b2602af580e7c3ce433fce432d55ef50a4f5e5
Author: Evan Stade <estade@chromium.org>
Date: Fri Jul 14 19:50:24 2017

Change meaning of "IsLoadingToDifferentDocument" in WebContents to only
apply to loads in the main frame.

The distinction between "IsLoading" and "IsLoadingToDifferentDocument"
appears to only be relevant in UI contexts (e.g. TabRendererData, which
controls whether to show the throbber in the tabstrip on desktop
Chrome). I think the distinction only makes sense in the context of the
main frame and a load in a subframe should always be lumped in with
non-top-level document loads. For example, on Android it's sometimes
used to conditionally call ToolbarLayout.onNavigatedToDifferentPage(),
which from other callsites definitely appears to only care about main
frame navigations.

Making this change means the throbber won't flicker when content is
loaded in an iframe right after the main frame finishes loading, which
happens a lot (see bug).

With this change, we can probably remove the timer added in
896032108c97ec8e014f which attempts to smooth out favicon/throbber
flickering.

Bug:  734104 
Change-Id: Ie14b1c54956479021fd9aa6c4c8aa690be52f193
Reviewed-on: https://chromium-review.googlesource.com/569060
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486854}
[modify] https://crrev.com/c3b2602af580e7c3ce433fce432d55ef50a4f5e5/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/c3b2602af580e7c3ce433fce432d55ef50a4f5e5/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/c3b2602af580e7c3ce433fce432d55ef50a4f5e5/chrome/browser/ui/views/tabs/tab_renderer_data.cc
[modify] https://crrev.com/c3b2602af580e7c3ce433fce432d55ef50a4f5e5/chrome/browser/ui/views/tabs/tab_renderer_data.h
[modify] https://crrev.com/c3b2602af580e7c3ce433fce432d55ef50a4f5e5/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/c3b2602af580e7c3ce433fce432d55ef50a4f5e5/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/c3b2602af580e7c3ce433fce432d55ef50a4f5e5/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/c3b2602af580e7c3ce433fce432d55ef50a4f5e5/content/public/browser/web_contents.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 18 2017

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

commit 2009897ef7d9825f61b904acbf684cdd1a1357ef
Author: Evan Stade <estade@chromium.org>
Date: Tue Jul 18 19:27:00 2017

Fix default setting of a flag in about:flags

Specifically, --delay-reload-stop-button-change

Bug:  734104 
Change-Id: Ie923a0da9511a663c9a63efaf3d8ab01671a768a
Reviewed-on: https://chromium-review.googlesource.com/576250
Reviewed-by: Terry Anderson <tdanderson@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487555}
[modify] https://crrev.com/2009897ef7d9825f61b904acbf684cdd1a1357ef/chrome/browser/about_flags.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 2 2017

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

commit 84dfc127963de8c2e0821a621da83debf893a22a
Author: Evan Stade <estade@chromium.org>
Date: Wed Aug 02 01:52:00 2017

Remove heuristic suppression of UI indicators of loading state changes.

With c3b2602af580e7c3ce433 these heuristics are deemed no longer useful/
necessary. They were only enabled behind a flag, so there should be no
effective user-visible change.

Bug:  734104 
Change-Id: I4c22e7a638dad8d2ed36494663276ef47dbbb495
Reviewed-on: https://chromium-review.googlesource.com/596830
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491199}
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/browser/about_flags.cc
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/browser/ui/views/tabs/tab_unittest.cc
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/browser/ui/views/toolbar/reload_button.cc
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/browser/ui/views/toolbar/reload_button.h
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/common/chrome_switches.cc
[modify] https://crrev.com/84dfc127963de8c2e0821a621da83debf893a22a/chrome/common/chrome_switches.h

Comment 9 by creis@chromium.org, Aug 2 2017

Cc: a...@chromium.org nick@chromium.org nasko@chromium.org creis@chromium.org
In https://chromium-review.googlesource.com/c/578297, we're discussing some of the tradeoffs in showing the old page's URL after the new page commits (while we wait for the title).  Here's a video of how it looks when the stale title sticks around for quite some time (with the CL patched in), which I'm concerned about.
stale-title.webm
5.1 MB View Download
Blockedon: 752326
today I was able to repro the behavior shown in the video above (sporadically), but I believe it's a separate problem, so I filed  bug 752326 .
Status: Fixed (was: Assigned)

Sign in to add a comment