New issue
Advanced search Search tips

Issue 839428 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-23
OS: iOS
Pri: 0
Type: Task
Q2

Blocking:
issue 805166



Sign in to add a comment

Bookmarks: Move favicon loading out of BookmarkTableView

Project Member Reported by rohitrao@chromium.org, May 3 2018

Issue description

BookmarkTableView currently contains a large amount of code for loading favicons.  I'd like to move this out of the table view and into a helper class instead.

Favicon loading is also keyed by indexPath, but index paths can change if bookmarks are added or removed.  We should instead key by BookmarkNodeItem (or maybe by BookmarkNode).  It's easy enough to get the current indexPath for a given item.

Moving this code out of BookmarkTableView will make it easier to switch back to a stock UITableView if that is needed in the future.  It also helps move functionality out of the existing large class.
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, May 10 2018

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

commit 9096df43addf9de72c4039e676e2261d28249642
Author: Rohit Rao <rohitrao@chromium.org>
Date: Thu May 10 17:07:42 2018

[ios] Moves favicon loading into BookmarkHomeViewController.

BUG= 840381 , 839428 

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Id20bd04c30c03bb98dbb9ac9c8c6013fc245b950
Reviewed-on: https://chromium-review.googlesource.com/1052904
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557561}
[modify] https://crrev.com/9096df43addf9de72c4039e676e2261d28249642/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm
[modify] https://crrev.com/9096df43addf9de72c4039e676e2261d28249642/ios/chrome/browser/ui/bookmarks/bookmark_table_view.h
[modify] https://crrev.com/9096df43addf9de72c4039e676e2261d28249642/ios/chrome/browser/ui/bookmarks/bookmark_table_view.mm

Status: Fixed (was: Started)
NextAction: 2018-05-23
The NextAction date has arrived: 2018-05-23

Sign in to add a comment