Allow site settings to query the favicon database by host name |
||
Issue descriptionThe favicon database associates exact URLs with favicons. This is problematic when Site Settings queries the database since it will be sending origins or domains, which don't necessarily correspond directly to history items. For example, on desktop: 1. Visit https://permission.site 2. Go to chrome://settings/content/cookies 3. Add a block or allow exception for "permission.site" 4. Notice that the icon next to the new exception is a page (i.e. no favicon). This is because the favicon is associated with "https://permission.site", but we queried with "permission.site". 5. Edit the exception to be "https://permission.site" 6. Notice that permission.site's favicon now appears. On Android: 1. Visit https://www.flipkart.com/offers 2. Open the 3 dot menu, Settings, Site Settings, All Sites 3. Notice that www.flipkart.com has a generated "F" favicon rather than its proper favicon. This is because www.flipkart.com has domain-scoped cookie storage, so we query the favicon service with "www.flipkart.com", which doesn't match "https://www.flipkart.com/offers" in the favicon database. We should implement host name matching as an optional fallback in the favicon database. Site settings can opt into this fallback since it will often be querying the favicon service with URLs that the user hasn't directly visited. This should help address the friendliness of these services (and reduce the what-the factor of seeing these familiar URLs without their familiar icons)
,
Jan 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832 commit 5cb9e8d52ba6f729e36c4ab3447c4a76ad250832 Author: Dominick Ng <dominickn@chromium.org> Date: Sat Jan 13 06:08:05 2018 Allow site settings to query favicons by host name. The favicon database associates exact URLs with favicons, which is problematic for Site Settings since it queries for favicons using origins or URLs with different schemes to those stored in the database. One example is domain-scoped data, where Site Settings queries for a favicon with a HTTP scheme (e.g. http://www.google.com). If the site redirects users to HTTPS, there will be no favicon available, since all favicons will be associated with https://www.google.com. Another example is paths. If a user visits https://www.google.com/maps, and then grants location permission, the Site Settings Location Content Settings page associates the permission with the origin https://www.google.com/. Querying for a favicon with the origin will fail unless the user has visited the origin exactly and created a mapping for the origin to an icon. This CL implements a hostname matching backoff in the thumbnail database maintained by the history service. Site Settings on Android and the chrome://favicon URL used by WebUI Site Settings on Desktop are both updated to use the fallback. The backoff is only triggered by an explicit |fallback_to_host| parameter passed in the call querying the favicon database, and it only triggers if no match is found with the exact URL passed in (i.e. existing search behaviour is maintained). New tests are added to ensure the backoff functions as expected. The effect can be manually observed by following the examples on the attached bug. BUG=798029 Change-Id: I11316a0939f7c553f497f3f1b7fb42298f86d4e8 Reviewed-on: https://chromium-review.googlesource.com/846319 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Dominick Ng <dominickn@chromium.org> Cr-Commit-Position: refs/heads/master@{#529168} [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/chrome/browser/android/favicon_helper.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/chrome/browser/bookmarks/bookmark_html_writer.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/chrome/browser/sync/test/integration/bookmarks_helper.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/chrome/browser/ui/webui/extensions/extension_icon_source.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/chrome/browser/ui/webui/favicon_source.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/favicon/core/favicon_service.h [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/favicon/core/favicon_service_impl.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/favicon/core/favicon_service_impl.h [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/favicon/core/favicon_util.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/favicon/core/test/mock_favicon_service.h [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/history/core/browser/history_backend.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/history/core/browser/history_backend.h [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/history/core/browser/history_backend_unittest.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/history/core/browser/history_service.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/history/core/browser/history_service.h [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/history/core/browser/thumbnail_database.cc [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/history/core/browser/thumbnail_database.h [modify] https://crrev.com/5cb9e8d52ba6f729e36c4ab3447c4a76ad250832/components/history/core/browser/thumbnail_database_unittest.cc
,
Sep 4
Is this issue fixed?
,
Sep 4
Not fully. There are still edge cases that don't work - the most prominent one being that we don't search for an icon for the ETLD+1 (e.g. google.com) in the case where we have an origin (maps.google.com) that doesn't have a favicon. That's much harder to solve because of the way the history database is designed. |
||
►
Sign in to add a comment |
||
Comment 1 by dominickn@chromium.org
, Dec 29 2017