Change WebContents to report favicons early |
||
Issue descriptionCurrently WebContentsObserver::DidUpdateFaviconURL() gets called very late during the page load. This should be changed to happen as early as possible. The immediate benefit would be that the coverage of the favicon cache (as in mappings between page URLs and icon URLs) would improve, since this would no longer wait until the page has been fully loaded. This is expected to improve the likelihood that NTP tiles (and other UIs) have a nice icon. Somewhat orthogonally, we'd need to decide whether the actual downloading of favicon images should wait until the pages is loaded. One could argue that favicons are unimportant compared to the actual content of the page, so we should make sure we don't slow down the page load.
,
Jun 20 2017
A second look suggests this change wouldn't actually fix crbug.com/498436 but might fix cases like Google Maps, which currently behaves as follows: 1. Type maps.google.com in the omnibox. 2. DidStartNavigation() is triggered for http://maps.google.com/. 3. DidFinishNavigation() is triggered for https://www.google.com/maps/@?nogmmr=1. 4. DidStartNavigation() for https://www.google.com/maps/@48.142803,11.5411858,17z. 5. DidFinishNavigation() for the same URL. 6. DidUpdateFaviconURL() receives two favicons for the same URL. 7. History backend associates the favicons to a single page URL (no redirect information exists). Reporting favicons early seems to fix this issue because the same list of favicons would be provided for the first page URL (https://www.google.com/maps/@). An alternative approach would be to extend HistoryBackend's recent_redirects_ to track these client-side redirects, possibly by better handling did_replace_entry in HistoryBackend::AddPage().
,
Jul 4 2017
I just confirmed that improved handling of client-side redirects (WIP/prototype in https://codereview.chromium.org/2949023002/) fixes the Google Maps case, but not the gmail case (crbug.com/498436), at least in the current form. If the patches lands, this bug would be obsolete.
,
Jul 5 2017
Thanks for following up on this. :)
,
Sep 13 2017
The mentioned bugs have been fixed by improving the handling of client-side redirects, no I see no need to address this bug anymore. Closing. |
||
►
Sign in to add a comment |
||
Comment 1 by mastiz@chromium.org
, Apr 7 2017Owner: mastiz@chromium.org
Status: Assigned (was: Untriaged)