New issue
Advanced search Search tips

Issue 907642 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 911327



Sign in to add a comment

[Tab loading animation] Avoid showing favicon during waiting stage

Project Member Reported by pbos@chromium.org, Nov 21

Issue description

Showing favicon of the new page navigation before page commit may be a privacy and security concern.

We should make sure the favicon fade-in timer doesn't start before we're in the loading state. I'm not 100% it can happen right now, but we should make sure that there's no cached favicon that starts showing in the connecting state.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 3

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

commit b0f1c90971a60660d0f0dbbeaa97602ac70c5eac
Author: Peter Boström <pbos@chromium.org>
Date: Mon Dec 03 19:24:08 2018

Do not fade in favicon before page commit

Prevents showing cached favicons for the previous page during a timing
bug while it's connecting to the new page.

Bug:  chromium:907642 
Change-Id: Ifb1cfa75ed8fbc6842ec9811b2b92bd10d2442b5
Reviewed-on: https://chromium-review.googlesource.com/c/1358921
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613189}
[modify] https://crrev.com/b0f1c90971a60660d0f0dbbeaa97602ac70c5eac/chrome/browser/ui/views/tabs/tab_icon.cc
[modify] https://crrev.com/b0f1c90971a60660d0f0dbbeaa97602ac70c5eac/chrome/browser/ui/views/tabs/tab_icon.h

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 4

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

commit 5964e2682f6d392d82752c0760deb30633059a47
Author: Peter Boström <pbos@chromium.org>
Date: Tue Dec 04 19:19:49 2018

Reset tab-icon animations for loading -> waiting

Prevents showing earlier favicon from the earlier page load that did not
finish (was still at loading). Also allows the favicon of the new page
load to fade in.

Bug:  chromium:907642 ,  chromium:910848 
Change-Id: I65fb44b7f432d5a0f4cd484f5a497f9b8a327c62
Reviewed-on: https://chromium-review.googlesource.com/c/1361282
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613631}
[modify] https://crrev.com/5964e2682f6d392d82752c0760deb30633059a47/chrome/browser/ui/views/tabs/tab_icon.cc

Labels: Merge-Request-72
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 6

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

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

Comment 5 by bugdroid1@chromium.org, Dec 6

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b2b4a5dac549f17b124051260c65bdedc6701c9

commit 2b2b4a5dac549f17b124051260c65bdedc6701c9
Author: Peter Boström <pbos@chromium.org>
Date: Thu Dec 06 17:37:56 2018

Merge tab-loading-animation refactor to M72

Prerequisite for other approved merges.

Bug:  chromium:907642 ,  chromium:908920 

Refactor the tab-loading animation

This change instead updates the animation progress in delta-time
increments, which prevents a bunch of clock math and rewinding of start
timestamps.

Bug: None
Change-Id: I6848e3c3ca2d7c844f56f126d72c9b8420364b2a
Reviewed-on: https://chromium-review.googlesource.com/c/1356261
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613161}(cherry picked from commit 910efca58c0cc9089c771d5f2ccdf12de7732678)
Reviewed-on: https://chromium-review.googlesource.com/c/1365875
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#112}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/2b2b4a5dac549f17b124051260c65bdedc6701c9/chrome/browser/ui/views/tabs/tab_icon.cc
[modify] https://crrev.com/2b2b4a5dac549f17b124051260c65bdedc6701c9/chrome/browser/ui/views/tabs/tab_icon.h
[modify] https://crrev.com/2b2b4a5dac549f17b124051260c65bdedc6701c9/chrome/browser/ui/views/tabs/tab_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 6

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

commit 4ee260858855d2f78c0100f090d0ef80665003c2
Author: Peter Boström <pbos@chromium.org>
Date: Thu Dec 06 17:43:52 2018

Do not fade in favicon before page commit

Prevents showing cached favicons for the previous page during a timing
bug while it's connecting to the new page.

Bug:  chromium:907642 
Change-Id: Ifb1cfa75ed8fbc6842ec9811b2b92bd10d2442b5
Reviewed-on: https://chromium-review.googlesource.com/c/1358921
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613189}(cherry picked from commit b0f1c90971a60660d0f0dbbeaa97602ac70c5eac)
Reviewed-on: https://chromium-review.googlesource.com/c/1365876
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#113}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/4ee260858855d2f78c0100f090d0ef80665003c2/chrome/browser/ui/views/tabs/tab_icon.cc
[modify] https://crrev.com/4ee260858855d2f78c0100f090d0ef80665003c2/chrome/browser/ui/views/tabs/tab_icon.h

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 6

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

commit 72709886da48858f030a966bb286c00782029fb3
Author: Peter Boström <pbos@chromium.org>
Date: Thu Dec 06 17:50:19 2018

Reset tab-icon animations for loading -> waiting

Prevents showing earlier favicon from the earlier page load that did not
finish (was still at loading). Also allows the favicon of the new page
load to fade in.

Bug:  chromium:907642 ,  chromium:910848 
Change-Id: I65fb44b7f432d5a0f4cd484f5a497f9b8a327c62
Reviewed-on: https://chromium-review.googlesource.com/c/1361282
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613631}(cherry picked from commit 5964e2682f6d392d82752c0760deb30633059a47)
Reviewed-on: https://chromium-review.googlesource.com/c/1365975
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#115}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/72709886da48858f030a966bb286c00782029fb3/chrome/browser/ui/views/tabs/tab_icon.cc

Status: Fixed (was: Assigned)
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/2b2b4a5dac549f17b124051260c65bdedc6701c9

Commit: 2b2b4a5dac549f17b124051260c65bdedc6701c9
Author: pbos@chromium.org
Commiter: pbos@chromium.org
Date: 2018-12-06 17:37:56 +0000 UTC

Merge tab-loading-animation refactor to M72

Prerequisite for other approved merges.

Bug:  chromium:907642 ,  chromium:908920 

Refactor the tab-loading animation

This change instead updates the animation progress in delta-time
increments, which prevents a bunch of clock math and rewinding of start
timestamps.

Bug: None
Change-Id: I6848e3c3ca2d7c844f56f126d72c9b8420364b2a
Reviewed-on: https://chromium-review.googlesource.com/c/1356261
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613161}(cherry picked from commit 910efca58c0cc9089c771d5f2ccdf12de7732678)
Reviewed-on: https://chromium-review.googlesource.com/c/1365875
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#112}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/72709886da48858f030a966bb286c00782029fb3

Commit: 72709886da48858f030a966bb286c00782029fb3
Author: pbos@chromium.org
Commiter: pbos@chromium.org
Date: 2018-12-06 17:50:19 +0000 UTC

Reset tab-icon animations for loading -> waiting

Prevents showing earlier favicon from the earlier page load that did not
finish (was still at loading). Also allows the favicon of the new page
load to fade in.

Bug:  chromium:907642 ,  chromium:910848 
Change-Id: I65fb44b7f432d5a0f4cd484f5a497f9b8a327c62
Reviewed-on: https://chromium-review.googlesource.com/c/1361282
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613631}(cherry picked from commit 5964e2682f6d392d82752c0760deb30633059a47)
Reviewed-on: https://chromium-review.googlesource.com/c/1365975
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#115}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment