New issue
Advanced search Search tips

Issue 840381 link

Starred by 1 user

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: Get rid of BookmarkTableView

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

Issue description

We'd like to convert BookmarkHomeViewController into a subclass of ChromeTableViewController with a stock UITableView.  In order to do this, we will need to get rid of BookmarkTableView.

The code in this class will move to either BookmarkHomeViewController or into a new BookmarkHomeMediator class.

 
Project Member

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

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

commit bf8e840d97fc0c4d149106cc0ff9d6537144f73e
Author: Rohit Rao <rohitrao@chromium.org>
Date: Mon May 07 16:56:24 2018

[ios] Introduces BookmarkHomeSharedState.

An upcoming refactoring will move methods out of BookmarkTableView and into
BookmarkHomeViewController or BookmarkHomeMediator. To ease this transition, we
introduce a data structure that holds various ivars that used to be in
BookmarkTableView.

BUG= 839427 , 840381 

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

Rough plan of action (first CL at the bottom):
8ac7927b44ab (hoist) git cl format and gn check.
71ca075acd38 Almost final cleanups.
4804d5ed40d5 Moves traitCollectionDidChange and editingFolderCell to the VC.
4a1d5016b0f5 Moves BookmarkTableCellTitleEditDelegate to the VC. Also moves properties that are no longer used by the table view.
9c58953099cc Merge model observers into the existing VC observer methods.
ec4ab11dd97c Moves the sync observer to the VC.  Also moves the showBackground method, because it needs the current sync state.
052e20e1f82c Moves keyboard notification code to the VC
161a58a55e61 Moves addNewFolder to the VC.
f9463b70610a Moves favicon code to the VC.  Adds a delegate method so that bookmark model favicon notifications can still trigger favicon fetches.
0ad8cbcc07f0 Move a set of methods that is not called by the tableview.
0ee892be12ce Moves |editing| to the VC and renames it currentlyInEditMode. Can move because no remaining uses in the tableview.
bc2d9afc1f10 Moves editNodes into the VC.  resetEditNodes is added to the delegate because it's called from setEditing.
b302829ee9f5 Moves populating the tableviewmodel into the VC.
408e9f417583 Moves promo configuration to the VC.  Exposes currentRootNode as a property on the table view.
8d3a0803d2bc Moves tableView configuration and gesture recognizers to the VC.  Also fixes a long press bug when editing.
094ffe87fb06 Moves the delegate to BookmarkHomeViewController.
53677182f230 Move data source methods to BookmarkHomeViewController.
c93ed0bfd686 Model is owned by the BookmarkHomeVC

Project Member

Comment 3 by bugdroid1@chromium.org, May 7 2018

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

commit e122eb1541ea014e04a93354916c67929587c86f
Author: Rohit Rao <rohitrao@chromium.org>
Date: Mon May 07 23:42:56 2018

[ios] Moves BookmarkHomeNodeItem and BookmarkHomePromoItem to new files.

BUG= 840381 

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Id48c1ea24bbbc642a3f70501ea19767ac83aec0a
Reviewed-on: https://chromium-review.googlesource.com/1047686
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556609}
[modify] https://crrev.com/e122eb1541ea014e04a93354916c67929587c86f/ios/chrome/browser/ui/bookmarks/bookmark_table_view.h
[modify] https://crrev.com/e122eb1541ea014e04a93354916c67929587c86f/ios/chrome/browser/ui/bookmarks/bookmark_table_view.mm
[modify] https://crrev.com/e122eb1541ea014e04a93354916c67929587c86f/ios/chrome/browser/ui/bookmarks/cells/BUILD.gn
[add] https://crrev.com/e122eb1541ea014e04a93354916c67929587c86f/ios/chrome/browser/ui/bookmarks/cells/bookmark_home_node_item.h
[add] https://crrev.com/e122eb1541ea014e04a93354916c67929587c86f/ios/chrome/browser/ui/bookmarks/cells/bookmark_home_node_item.mm
[add] https://crrev.com/e122eb1541ea014e04a93354916c67929587c86f/ios/chrome/browser/ui/bookmarks/cells/bookmark_home_promo_item.h
[add] https://crrev.com/e122eb1541ea014e04a93354916c67929587c86f/ios/chrome/browser/ui/bookmarks/cells/bookmark_home_promo_item.mm

Project Member

Comment 4 by bugdroid1@chromium.org, May 8 2018

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

commit b32d39b663bc6f4e4b6a5d565bfd7babf51bf011
Author: Rohit Rao <rohitrao@chromium.org>
Date: Tue May 08 11:07:50 2018

[ios] Moves table view data source and delegate methods to BookmarkHomeViewController.

Also moves the implementation of BookmarkTableCellTitleEditDelegate to
BookmarkHomeViewController, as that was only referenced by one of the data
source methods.

Makes several BookmarkTableView methods public in order to ease migration.

BUG= 840381 

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

Project Member

Comment 5 by bugdroid1@chromium.org, May 8 2018

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

commit 5a3fc2ce2182c6cee84cefb26cc1119875ff191a
Author: Rohit Rao <rohitrao@chromium.org>
Date: Tue May 08 17:04:11 2018

[ios] Moves table view setup, gesture handling, and keyboard code to BookmarksHomeViewController.

BUG= 840381 

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

Project Member

Comment 6 by bugdroid1@chromium.org, May 9 2018

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

commit ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25
Author: Rohit Rao <rohitrao@chromium.org>
Date: Wed May 09 01:18:35 2018

[ios] Adds BookmarkHomeMediator and BookmarkHomeConsumer.

Adds skeleton implementation of a mediator and consumer interface for
the BookmarksHomeViewController. Business logic will be added in a
future CL.

The ChromeTableViewConsumer protocol is added to serve as a base
protocol for all tableview-based mediators. Individual subclasses can
extend this protocol and add feature-specific methods.

BUG= 840381 

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I42ff91edfaa6c85a3e534ce515bb9fc82ae1e1fc
Reviewed-on: https://chromium-review.googlesource.com/1050196
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557046}
[modify] https://crrev.com/ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25/ios/chrome/browser/ui/bookmarks/BUILD.gn
[add] https://crrev.com/ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25/ios/chrome/browser/ui/bookmarks/bookmark_home_consumer.h
[add] https://crrev.com/ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25/ios/chrome/browser/ui/bookmarks/bookmark_home_mediator.h
[add] https://crrev.com/ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25/ios/chrome/browser/ui/bookmarks/bookmark_home_mediator.mm
[modify] https://crrev.com/ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm
[modify] https://crrev.com/ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25/ios/chrome/browser/ui/table_view/BUILD.gn
[add] https://crrev.com/ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25/ios/chrome/browser/ui/table_view/chrome_table_view_consumer.h
[modify] https://crrev.com/ec75dd2ad7e33b1f1539b1dab7e79fd13d687e25/ios/chrome/browser/ui/table_view/chrome_table_view_controller.h

The NextAction date has arrived: 2018-05-09
Project Member

Comment 8 by bugdroid1@chromium.org, May 9 2018

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

commit bb7a6a5a68cead25adc95c0167a07757be97012b
Author: Rohit Rao <rohitrao@chromium.org>
Date: Wed May 09 23:17:05 2018

[ios] Moves BookmarkModel interaction into BookmarkHomeMediator.

The mediator is now responsible for populating the "Bookmarks" section
of the TableViewModel and for registering as a BookmarkModelObserver.

Methods are added to BookmarkHomeConsumer in order to allow the mediator
to drive the UI.  Methods are also added to BookmarkTableViewDelegate
and to the public API of BookmarkTableView in order to ease this code
migration.

BUG= 840381 

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

Project Member

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

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

commit f44af86672756bbc58cd2dec519bae824f35da17
Author: Rohit Rao <rohitrao@chromium.org>
Date: Thu May 10 00:49:50 2018

[ios] Moves sync code into BookmarkHomeMediator.

The mediator now acts as the SyncSessionObserver for the bookmarks home UI.
Methods are added to the consumer protocol to support modifying the table view
background to show the loading and empty states.  The mediator is modified to
automatially update the table view background whenever the set of bookmarks data
changes.

BUG= 840381 

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

Project Member

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

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

commit 7b703c591566a882b1e98c1628508ef509af1892
Author: Rohit Rao <rohitrao@chromium.org>
Date: Thu May 10 16:03:37 2018

[ios] Moves promo cell code out of BookmarksTableView.

Model-related code moves into BookmarkHomeMediator. UI and
cell-related code moves into BookmarkHomeViewController.

BUG= 840381 

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

Project Member

Comment 11 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

Project Member

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

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

commit 0fe1faa1ce879d83359a2343cbef69e7bc35b096
Author: Rohit Rao <rohitrao@chromium.org>
Date: Thu May 10 17:55:11 2018

[ios] Moves remaining methods out of BookmarksTableView.

The only code remaining in BookmarksTableView is directly related to the
UITableView or its loading and empty state backgrounds.

BUG= 840381 

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

Project Member

Comment 13 by bugdroid1@chromium.org, May 14 2018

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

commit cb526bb53f0399662a2c9d5bd3889257b52907f9
Author: Rohit Rao <rohitrao@chromium.org>
Date: Mon May 14 22:02:56 2018

[ios] Deletes BookmarkTableView.

Makes BookmarkHomeViewController subclass UITableViewController instead of
UIViewController. Moves the loading spinner and empty background view into
BookmarkHomeViewController.


BUG= 840381 , 839439 

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I935a99313eb3aa7f8701d46d803bea575e5bc89a
Reviewed-on: https://chromium-review.googlesource.com/1056328
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558484}
[modify] https://crrev.com/cb526bb53f0399662a2c9d5bd3889257b52907f9/ios/chrome/browser/ui/bookmarks/BUILD.gn
[modify] https://crrev.com/cb526bb53f0399662a2c9d5bd3889257b52907f9/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.h
[modify] https://crrev.com/cb526bb53f0399662a2c9d5bd3889257b52907f9/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm
[modify] https://crrev.com/cb526bb53f0399662a2c9d5bd3889257b52907f9/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller_unittest.mm
[delete] https://crrev.com/ee749a4e70158c219fd83fd25e3a39b707de7bde/ios/chrome/browser/ui/bookmarks/bookmark_table_view.h
[delete] https://crrev.com/ee749a4e70158c219fd83fd25e3a39b707de7bde/ios/chrome/browser/ui/bookmarks/bookmark_table_view.mm
[modify] https://crrev.com/cb526bb53f0399662a2c9d5bd3889257b52907f9/ios/chrome/browser/ui/bookmarks/cells/bookmark_home_promo_item.mm

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

Sign in to add a comment