New issue
Advanced search Search tips

Issue 843690 link

Starred by 2 users
Status: Fixed
Owner:
thegreenfrog@chromium.org
Closed: Jun 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Refactor FavcionLoader

Project Member Reported by thegreenfrog@chromium.org, May 16 2018 
Use an NSCache instead of an NSMutableDicitonary to take advantage of an LRU eviction logic that is a part of NSCache in addition to the in-memory access of recently retrieved favicons that both provide.

Implement favicon retrieval logic using LargeIconService static method GetLargeIconOrFallbackStyle(). Currently it has a dependency on TabSwitcher, which is the wrong approach.
Project Member

Comment 1 by bugdroid1@chromium.org, May 23 2018 

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

commit 7957692183ad09abdd37717e333048a102890239
Author: Chris Lu <thegreenfrog@chromium.org>
Date: Wed May 23 16:45:26 2018

[ios] use NSCache for FaviconLoader

Previously used NSMutableDictionary

Bug:  843690 
Change-Id: Id315cd56d3fe132aa5c6d48f88939b182f6fcb75
Reviewed-on: https://chromium-review.googlesource.com/1063263
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561120}
[modify] https://crrev.com/7957692183ad09abdd37717e333048a102890239/ios/chrome/browser/favicon/favicon_loader.h
[modify] https://crrev.com/7957692183ad09abdd37717e333048a102890239/ios/chrome/browser/favicon/favicon_loader.mm
[modify] https://crrev.com/7957692183ad09abdd37717e333048a102890239/ios/chrome/browser/ui/browser_view_controller.mm
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 1 2018 

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

commit 09b31be1ebe2052fe9b63183327e6a72f8b449ed
Author: Chris Lu <thegreenfrog@chromium.org>
Date: Fri Jun 01 04:24:40 2018

[ios] refactor FaviconLoader to use LargeIconService

FaviconLoader fetch method now returns FaviconAttributes instead of images to accommodate for fallback styles. Updates all use cases of FaviconLoader as needed.

Bug:  843690 

Change-Id: I913a130f137b92cfde39af3444f9fe2b33b4677c
Reviewed-on: https://chromium-review.googlesource.com/1065205
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563533}
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/favicon/BUILD.gn
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/favicon/favicon_loader.h
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/favicon/favicon_loader.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/favicon/ios_chrome_favicon_loader_factory.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/bookmarks/BUILD.gn
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/history/history_entry_item.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/ntp/recent_tabs/views/BUILD.gn
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/ntp/recent_tabs/views/session_tab_data_view.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/recent_tabs/BUILD.gn
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/recent_tabs/recent_tabs_image_data_source.h
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/recent_tabs/recent_tabs_mediator.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/tab_switcher/BUILD.gn
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_cell.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/tab_switcher/tab_switcher_utils.h
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/tab_switcher/tab_switcher_utils.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/table_view/cells/BUILD.gn
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/table_view/cells/table_view_url_item.h
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/table_view/cells/table_view_url_item.mm
[modify] https://crrev.com/09b31be1ebe2052fe9b63183327e6a72f8b449ed/ios/chrome/browser/ui/table_view/cells/table_view_url_item_unittest.mm

Comment 3 by thegreenfrog@chromium.org, Jun 6 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment