Issue metadata
Sign in to add a comment
|
[M64] Webpage favicons are not being shown in most visited tiles |
||||||||||||||||||||||
Issue descriptionSeparating Issue 788708 to track fix on M64 iOS Version: 11.2 beta 4, 11.1.2, 10.3.3 Device: iPhone and iPad URL: bbc.com or any other web page having favicons. Steps to reproduce: 1. Fresh install and launch chrome 2. Load bbc.com web page 3. Once the page loads completely, wait for few seconds or a minute 4. Open a new tab page( if bbc.com tile is not displayed open another new tab page) Observed results: Notice that bbc.com tile is displayed without favicon Note 1: Go back to step 2 tab page and reload the page, now open another new tab page notice the favicon is now displayed. Note 2: Tested on multiple webpages like cnn.com, ebay.com observe the same behavior Expected results: Webpage favicons should be displayed in most visited tiles, when the webpage is loaded for the first time
,
Nov 29 2017
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/19825f69ff27c5e99fb0a230ce730d8cc0561830 commit 19825f69ff27c5e99fb0a230ce730d8cc0561830 Author: Eugene But <eugenebut@google.com> Date: Tue Dec 05 22:50:21 2017 Call WebStateObserver::FaviconUrlUpdated for same-document navigations. WebStateImpl will cache favicon urls in OnFaviconUrlUpdated and will call WebStateObserver::FaviconUrlUpdated in OnNavigationFinished for same document navigations. Also removed favicon urls caching in WebFaviconDriver introduced in crrev.com/c/695761. This CL is not exactly a revert of crrev.com/c/695761, because deprecated NavigationItemCommitted is replaced with DidFinishNavigation. NavigationItemCommitted is not called for push/replace state same-document navigaiton, which may cause other bugs for favicons caching. Bug: 789581 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I42922ab4d7812380787e350713640efe4d599680 Reviewed-on: https://chromium-review.googlesource.com/797072 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Commit-Queue: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#521876} [modify] https://crrev.com/19825f69ff27c5e99fb0a230ce730d8cc0561830/components/favicon/ios/web_favicon_driver.h [modify] https://crrev.com/19825f69ff27c5e99fb0a230ce730d8cc0561830/components/favicon/ios/web_favicon_driver.mm [modify] https://crrev.com/19825f69ff27c5e99fb0a230ce730d8cc0561830/ios/web/web_state/web_state_impl.h [modify] https://crrev.com/19825f69ff27c5e99fb0a230ce730d8cc0561830/ios/web/web_state/web_state_impl.mm [modify] https://crrev.com/19825f69ff27c5e99fb0a230ce730d8cc0561830/ios/web/web_state/web_state_impl_unittest.mm
,
Dec 5 2017
,
Dec 6 2017
Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d51db23f06cd4da8a968e96cf455f14e42788d38 commit d51db23f06cd4da8a968e96cf455f14e42788d38 Author: Eugene But <eugenebut@google.com> Date: Wed Dec 06 23:28:14 2017 Call WebStateObserver::FaviconUrlUpdated for same-document navigations. WebStateImpl will cache favicon urls in OnFaviconUrlUpdated and will call WebStateObserver::FaviconUrlUpdated in OnNavigationFinished for same document navigations. Also removed favicon urls caching in WebFaviconDriver introduced in crrev.com/c/695761. This CL is not exactly a revert of crrev.com/c/695761, because deprecated NavigationItemCommitted is replaced with DidFinishNavigation. NavigationItemCommitted is not called for push/replace state same-document navigaiton, which may cause other bugs for favicons caching. TBR=eugenebut@google.com (cherry picked from commit 19825f69ff27c5e99fb0a230ce730d8cc0561830) Bug: 789581 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I42922ab4d7812380787e350713640efe4d599680 Reviewed-on: https://chromium-review.googlesource.com/797072 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Commit-Queue: Eugene But <eugenebut@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#521876} Reviewed-on: https://chromium-review.googlesource.com/812468 Reviewed-by: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/branch-heads/3282@{#66} Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840} [modify] https://crrev.com/d51db23f06cd4da8a968e96cf455f14e42788d38/components/favicon/ios/web_favicon_driver.h [modify] https://crrev.com/d51db23f06cd4da8a968e96cf455f14e42788d38/components/favicon/ios/web_favicon_driver.mm [modify] https://crrev.com/d51db23f06cd4da8a968e96cf455f14e42788d38/ios/web/web_state/web_state_impl.h [modify] https://crrev.com/d51db23f06cd4da8a968e96cf455f14e42788d38/ios/web/web_state/web_state_impl.mm [modify] https://crrev.com/d51db23f06cd4da8a968e96cf455f14e42788d38/ios/web/web_state/web_state_impl_unittest.mm
,
Dec 7 2017
Verified the fix. NTP favicon is displayed immediately. Verified on M64.0.3282.14 beta Device: iPhone7 plus, iOS: 11.2 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by sczs@chromium.org
, Nov 29 2017Owner: eugene...@chromium.org
Status: Assigned (was: Untriaged)