Skip to content

Commit

Permalink
[IOS][Bkmrks] Move BookmarkBridgeObserver to mediator
Browse files Browse the repository at this point in the history
This cl also moves ownership of `editedNodes` from the coordinator to
the mediator through std::move()

Bug: 1405746
Change-Id: I8ef212bdeeb7e9b51c20276eaf4d0dd159ba3f16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4313863
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Commit-Queue: Tanmoy Mollik <triploblastic@google.com>
Cr-Commit-Position: refs/heads/main@{#1115857}
  • Loading branch information
Tanmoy Mollik authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent d65d7bf commit 2bbd6db
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 150 deletions.
1 change: 1 addition & 0 deletions ios/chrome/browser/ui/bookmarks/folder_chooser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ source_set("ui") {
"bookmarks_folder_chooser_consumer.h",
"bookmarks_folder_chooser_mediator.h",
"bookmarks_folder_chooser_mediator.mm",
"bookmarks_folder_chooser_mediator_delegate.h",
"bookmarks_folder_chooser_mutator.h",
"bookmarks_folder_chooser_view_controller.h",
"bookmarks_folder_chooser_view_controller.mm",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#define IOS_CHROME_BROWSER_UI_BOOKMARKS_FOLDER_CHOOSER_BOOKMARKS_FOLDER_CHOOSER_CONSUMER_H_

#import <Foundation/Foundation.h>
#import <set>
#import <vector>

namespace bookmarks {
Expand All @@ -31,9 +30,6 @@ class BookmarkNode;
- (const bookmarks::BookmarkNode*)rootFolder;
// The folder that should have a blue check mark beside it in the UI.
- (const bookmarks::BookmarkNode*)selectedFolder;
// TODO(crbug.com/1405746): Delete this method and return visible folders
// instead after BookmarkModelBridge is configured inside the mediator.
- (const std::set<const bookmarks::BookmarkNode*>&)editedNodes;
// The list of visible folders to show in the folder chooser UI.
- (std::vector<const bookmarks::BookmarkNode*>)visibleFolders;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#import "ios/chrome/browser/ui/bookmarks/bookmark_navigation_controller.h"
#import "ios/chrome/browser/ui/bookmarks/folder_chooser/bookmarks_folder_chooser_coordinator_delegate.h"
#import "ios/chrome/browser/ui/bookmarks/folder_chooser/bookmarks_folder_chooser_mediator.h"
#import "ios/chrome/browser/ui/bookmarks/folder_chooser/bookmarks_folder_chooser_mediator_delegate.h"
#import "ios/chrome/browser/ui/bookmarks/folder_chooser/bookmarks_folder_chooser_view_controller.h"
#import "ios/chrome/browser/ui/bookmarks/folder_chooser/bookmarks_folder_chooser_view_controller_presentation_delegate.h"
#import "ios/chrome/browser/ui/bookmarks/folder_editor/bookmarks_folder_editor_coordinator.h"
Expand All @@ -27,9 +28,13 @@
#endif

@interface BookmarksFolderChooserCoordinator () <
BookmarksFolderChooserMediatorDelegate,
BookmarksFolderChooserViewControllerPresentationDelegate,
BookmarksFolderEditorCoordinatorDelegate,
UIAdaptivePresentationControllerDelegate> {
UIAdaptivePresentationControllerDelegate>
@end

@implementation BookmarksFolderChooserCoordinator {
BookmarksFolderChooserMediator* _mediator;
// If folder chooser is created with a base view controller then folder
// chooser will create and own `_navigationController` that should be deleted
Expand All @@ -50,10 +55,6 @@ @interface BookmarksFolderChooserCoordinator () <
const bookmarks::BookmarkNode* _selectedFolder;
}

@end

@implementation BookmarksFolderChooserCoordinator

@synthesize baseNavigationController = _baseNavigationController;

- (instancetype)
Expand Down Expand Up @@ -93,7 +94,7 @@ - (BOOL)canDismiss {
}

- (const std::set<const bookmarks::BookmarkNode*>&)editedNodes {
return _hiddenNodes;
return [_mediator editedNodes];
}

- (void)setSelectedFolder:(const bookmarks::BookmarkNode*)folder {
Expand All @@ -112,13 +113,13 @@ - (void)start {
self.browser->GetBrowserState());
_mediator = [[BookmarksFolderChooserMediator alloc]
initWithBookmarkModel:model
editedNodes:&_hiddenNodes];
editedNodes:std::move(_hiddenNodes)];
_hiddenNodes.clear();
_mediator.delegate = self;
_mediator.selectedFolder = _selectedFolder;
_viewController = [[BookmarksFolderChooserViewController alloc]
initWithBookmarkModel:model
allowsNewFolders:_allowsNewFolders
allowsCancel:!_baseNavigationController
browser:self.browser];
initWithAllowsCancel:!_baseNavigationController
allowsNewFolders:_allowsNewFolders];
_viewController.delegate = self;
_viewController.dataSource = _mediator;
_viewController.mutator = _mediator;
Expand Down Expand Up @@ -146,7 +147,9 @@ - (void)stop {

DCHECK(_mediator);
DCHECK(_viewController);
[_mediator disconnect];
_mediator.consumer = nil;
_mediator.delegate = nil;
_mediator = nil;
if (_baseNavigationController) {
DCHECK_EQ(_baseNavigationController.topViewController, _viewController);
Expand All @@ -169,6 +172,13 @@ - (void)stop {
_viewController = nil;
}

#pragma mark - BookmarksFolderChooserMediatorDelegate

- (void)bookmarksFolderChooserMediatorWantsDismissal:
(BookmarksFolderChooserMediator*)mediator {
[_delegate bookmarksFolderChooserCoordinatorDidCancel:self];
}

#pragma mark - BookmarksFolderChooserViewControllerPresentationDelegate

- (void)showBookmarksFolderEditorWithParentFolder:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#import <Foundation/Foundation.h>
#import <set>

@protocol BookmarksFolderChooserMediatorDelegate;

namespace bookmarks {
class BookmarkModel;
class BookmarkNode;
Expand All @@ -22,6 +24,7 @@ class BookmarkNode;

// Consumer to reflect model changes in the UI.
@property(nonatomic, weak) id<BookmarksFolderChooserConsumer> consumer;
@property(nonatomic, weak) id<BookmarksFolderChooserMediatorDelegate> delegate;
// The currently selected folder.
@property(nonatomic, assign) const bookmarks::BookmarkNode* selectedFolder;

Expand All @@ -32,10 +35,13 @@ class BookmarkNode;
// nodes that are being edited (moved to a folder).
- (instancetype)initWithBookmarkModel:(bookmarks::BookmarkModel*)model
editedNodes:
(std::set<const bookmarks::BookmarkNode*>*)nodes
(std::set<const bookmarks::BookmarkNode*>)nodes
NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;

- (void)disconnect;
- (const std::set<const bookmarks::BookmarkNode*>&)editedNodes;

@end

#endif // IOS_CHROME_BROWSER_UI_BOOKMARKS_FOLDER_CHOOSER_BOOKMARKS_FOLDER_CHOOSER_MEDIATOR_H_
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

#import "base/containers/contains.h"
#import "components/bookmarks/browser/bookmark_model.h"
#import "ios/chrome/browser/bookmarks/bookmark_model_bridge_observer.h"
#import "ios/chrome/browser/ui/bookmarks/bookmark_ui_constants.h"
#import "ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.h"
#import "ios/chrome/browser/ui/bookmarks/folder_chooser/bookmarks_folder_chooser_consumer.h"
#import "ios/chrome/browser/ui/bookmarks/folder_chooser/bookmarks_folder_chooser_mediator_delegate.h"
#import "ios/chrome/browser/ui/bookmarks/folder_chooser/bookmarks_folder_chooser_mutator.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
Expand All @@ -18,40 +20,53 @@
using bookmarks::BookmarkModel;
using bookmarks::BookmarkNode;

@interface BookmarksFolderChooserMediator () <BookmarksFolderChooserMutator> {
BookmarkModel* _bookmarkModel;
std::set<const BookmarkNode*>* _editedNodes;
}

@interface BookmarksFolderChooserMediator () <BookmarksFolderChooserMutator,
BookmarkModelBridgeObserver>
@end

@implementation BookmarksFolderChooserMediator
@implementation BookmarksFolderChooserMediator {
// Model object that holds all bookmarks.
BookmarkModel* _bookmarkModel;
// Observer for `_bookmarkModel` changes.
std::unique_ptr<BookmarkModelBridge> _modelBridge;
// List of nodes to hide when displaying folders. This is to avoid to move a
// folder inside a child folder. These are also the list of nodes that are
// being edited (moved to a folder).
std::set<const BookmarkNode*> _editedNodes;
}

- (instancetype)initWithBookmarkModel:(BookmarkModel*)model
editedNodes:(std::set<const BookmarkNode*>*)nodes {
editedNodes:(std::set<const BookmarkNode*>)nodes {
DCHECK(model);
DCHECK(model->loaded());

self = [super init];
if (self) {
_bookmarkModel = model;
_editedNodes = nodes;
_modelBridge.reset(new BookmarkModelBridge(self, _bookmarkModel));
_editedNodes = std::move(nodes);
}
return self;
}

- (void)disconnect {
_bookmarkModel = nullptr;
_modelBridge = nil;
_editedNodes.clear();
}

- (const std::set<const BookmarkNode*>&)editedNodes {
return _editedNodes;
}

#pragma mark - BookmarksFolderChooserDataSource

- (const BookmarkNode*)rootFolder {
return _bookmarkModel->root_node();
}

- (const std::set<const BookmarkNode*>&)editedNodes {
return *_editedNodes;
}

- (std::vector<const BookmarkNode*>)visibleFolders {
return bookmark_utils_ios::VisibleNonDescendantNodes(*_editedNodes,
return bookmark_utils_ios::VisibleNonDescendantNodes(_editedNodes,
_bookmarkModel);
}

Expand All @@ -62,8 +77,64 @@ - (void)setSelectedFolder:(const BookmarkNode*)folder {
[_consumer notifyModelUpdated];
}

- (void)removeFromEditedNodes:(const BookmarkNode*)node {
_editedNodes->erase(node);
#pragma mark - BookmarkModelBridgeObserver

- (void)bookmarkModelLoaded {
// The bookmark model is assumed to be loaded when this controller is created.
NOTREACHED();
}

- (void)bookmarkNodeChanged:(const BookmarkNode*)bookmarkNode {
if (bookmarkNode->is_folder()) {
[_consumer notifyModelUpdated];
}
}

- (void)bookmarkNodeChildrenChanged:(const BookmarkNode*)bookmarkNode {
[_consumer notifyModelUpdated];
}

- (void)bookmarkNode:(const BookmarkNode*)bookmarkNode
movedFromParent:(const BookmarkNode*)oldParent
toParent:(const BookmarkNode*)newParent {
if (bookmarkNode->is_folder()) {
[_consumer notifyModelUpdated];
}
}

- (void)bookmarkNodeDeleted:(const BookmarkNode*)bookmarkNode
fromFolder:(const BookmarkNode*)folder {
// Remove node from `_editedNodes` if it is already deleted (possibly remotely
// by another sync device).
if (base::Contains(_editedNodes, bookmarkNode)) {
_editedNodes.erase(bookmarkNode);
// if `_editedNodes` becomes empty, nothing to move. Exit the folder
// chooser.
if (_editedNodes.empty()) {
[_delegate bookmarksFolderChooserMediatorWantsDismissal:self];
}
// Exit here because no visible node was deleted. Nodes in `_editedNodes`
// cannot be any visible folder in folder chooser.
return;
}

if (!bookmarkNode->is_folder()) {
return;
}

if (bookmarkNode == _selectedFolder) {
// The selected folder has been deleted. Fallback on the Mobile Bookmarks
// node.
_selectedFolder = _bookmarkModel->mobile_node();
}
[_consumer notifyModelUpdated];
}

- (void)bookmarkModelRemovedAllNodes {
// The selected folder is no longer valid. Fallback on the Mobile Bookmarks
// node.
_selectedFolder = _bookmarkModel->mobile_node();
[_consumer notifyModelUpdated];
}

@end
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef IOS_CHROME_BROWSER_UI_BOOKMARKS_FOLDER_CHOOSER_BOOKMARKS_FOLDER_CHOOSER_MEDIATOR_DELEGATE_H_
#define IOS_CHROME_BROWSER_UI_BOOKMARKS_FOLDER_CHOOSER_BOOKMARKS_FOLDER_CHOOSER_MEDIATOR_DELEGATE_H_

@class BookmarksFolderChooserMediator;

// Delegate protocol for the `BookmarksFolderChooserMediator` class.
@protocol BookmarksFolderChooserMediatorDelegate <NSObject>

// Called when folder chooser mediator wants to dismiss the UI.
- (void)bookmarksFolderChooserMediatorWantsDismissal:
(BookmarksFolderChooserMediator*)mediator;

@end

#endif // IOS_CHROME_BROWSER_UI_BOOKMARKS_FOLDER_CHOOSER_BOOKMARKS_FOLDER_CHOOSER_MEDIATOR_DELEGATE_H_
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ class BookmarkNode;
// TODO(crbug.com/1405746): Change parameter signature. View controller should
// not know about BookmarkNode.
- (void)setSelectedFolder:(const bookmarks::BookmarkNode*)folder;
// TODO(crbug.com/1405746): Delete this method after the mediator is configured
// to observe BookmarkModelBridge.
- (void)removeFromEditedNodes:(const bookmarks::BookmarkNode*)node;

@end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
@protocol BookmarksFolderChooserDataSource;
@protocol BookmarksFolderChooserMutator;
@protocol BookmarksFolderChooserViewControllerPresentationDelegate;
class Browser;

namespace bookmarks {
class BookmarkModel;
Expand All @@ -34,19 +33,15 @@ class BookmarkModel;
// Mutator to apply changes to model layer.
@property(nonatomic, weak) id<BookmarksFolderChooserMutator> mutator;

// TODO(crbug.com/1405746): Move `bookmarkModel` and `browser` to the model
// layer.
// Initializes the view controller with a bookmark model.
// `bookmarkModel` must not be `nullptr` and must be loaded.
// `allowsNewFolders` will instruct the controller to provide the necessary UI
// to create a folder.
// Initializes the view controller.
// `allowsCancel` puts a cancel and done button in the navigation bar instead
// of a back button, which is needed if this view controller is presented
// modally.
- (instancetype)initWithBookmarkModel:(bookmarks::BookmarkModel*)bookmarkModel
allowsNewFolders:(BOOL)allowsNewFolders
allowsCancel:(BOOL)allowsCancel
browser:(Browser*)browser;
// `allowsNewFolders` will instruct the controller to provide the necessary UI
// to create a folder.
- (instancetype)initWithAllowsCancel:(BOOL)allowsCancel
allowsNewFolders:(BOOL)allowsNewFolders
NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithStyle:(UITableViewStyle)style NS_UNAVAILABLE;

@end
Expand Down

0 comments on commit 2bbd6db

Please sign in to comment.