Fix app icon flicker in loading animation

Fix app icon flicker in loading animation

The flicker is caused by setting window title icon while a loading
animation is in progress. Fix the problem by skipping window icon update
when loading animation is running.

BUG=15782
TEST=Verify no flicker while web app loads per Ben's comments.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=32982

[Ben Goodger (Google), 2009-11-24 20:54:31]

http://codereview.chromium.org/436028/diff/1/2
File chrome/browser/views/frame/browser_view.cc (right):

http://codereview.chromium.org/436028/diff/1/2#newcode795
chrome/browser/views/frame/browser_view.cc:795: if (ShouldShowWindowIcon() &&
!loading_animation_timer_.IsRunning())
Does this mean no tab can update its favicon while any other tab is loading?

[xiyuan, 2009-11-24 21:05:05]

http://codereview.chromium.org/436028/diff/1/2
File chrome/browser/views/frame/browser_view.cc (right):

http://codereview.chromium.org/436028/diff/1/2#newcode795
chrome/browser/views/frame/browser_view.cc:795: if (ShouldShowWindowIcon() &&
!loading_animation_timer_.IsRunning())
On 2009/11/24 20:54:31, Ben Goodger wrote:
> Does this mean no tab can update its favicon while any other tab is loading?
Tab's favicon update in tab strip should not be affected. ShouldShowWindowIcon()
is true only for chrome frame window with title bar (i.e. app and pop-up) and
UpdateWindowIcon belows updates the top-left icon for the frame window. The fix
is to skip the window icon set when frame window (GlassBrowserFrameView) uses it
for loading animation.

[Ben Goodger (Google), 2009-11-24 21:12:49]

Ah, OK.

LGTM then.

On Tue, Nov 24, 2009 at 1:05 PM,  <xiyuan@chromium.org> wrote:
>
> http://codereview.chromium.org/436028/diff/1/2
> File chrome/browser/views/frame/browser_view.cc (right):
>
> http://codereview.chromium.org/436028/diff/1/2#newcode795
> chrome/browser/views/frame/browser_view.cc:795: if
> (ShouldShowWindowIcon() && !loading_animation_timer_.IsRunning())
> On 2009/11/24 20:54:31, Ben Goodger wrote:
>>
>> Does this mean no tab can update its favicon while any other tab is
>
> loading?
> Tab's favicon update in tab strip should not be affected.
> ShouldShowWindowIcon() is true only for chrome frame window with title
> bar (i.e. app and pop-up) and UpdateWindowIcon belows updates the
> top-left icon for the frame window. The fix is to skip the window icon
> set when frame window (GlassBrowserFrameView) uses it for loading
> animation.
>
> http://codereview.chromium.org/436028
>