[ios] Move SettingsCommands handling to SceneCoordinator [chromium/src : main]

0 views

Skip to first unread message

Scott Yoder (Gerrit)

unread,

Jan 21, 2026, 12:00:57 PM (23 hours ago) Jan 21

to Gauthier Ambard, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Gauthier Ambard

Scott Yoder added 1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Scott Yoder . resolved

Please take a look.

Open in Gerrit

Related details

Attention is currently required from:

Gauthier Ambard

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Gauthier Ambard (Gerrit)

unread,

4:04 AM (6 hours ago) 4:04 AM

to Scott Yoder, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Scott Yoder

Gauthier Ambard added 7 comments

File ios/chrome/browser/scene/coordinator/scene_coordinator.mm

Line 486, Patchset 7 (Latest):- (void)showSavedPasswordsSettingsAfterModalDismissFromViewController:

Gauthier Ambard . unresolved

This is a private method right, should it go to private pragma with a comment?

Line 663, Patchset 7 (Latest):- (void)showSettingsFromViewController:(UIViewController*)baseViewController {

Gauthier Ambard . unresolved

Those are not Settings commands right?
Could you make it more clear in the pragmas what is part and not part of the protocol?
Also, maybe having the same order here and in the commands protocol.

Line 735, Patchset 7 (Latest): // This is handled in SceneController.

Gauthier Ambard . unresolved

Why adding it here?

File ios/chrome/browser/shared/coordinator/scene/scene_controller.mm

Line 2199, Patchset 7 (Latest): hasDefaultBrowserBlueDot:(BOOL)hasDefaultBrowserBlueDot {

Gauthier Ambard . unresolved

Why not forwarding the method call to the SceneCoordinator instead?

Line 2224, Patchset 7 (Latest): self.mainCoordinator.settingsNavigationController =

Gauthier Ambard . unresolved

Do you prefer to leave the settings nav controller as readwrite vs moving the 3 creation methods explicitly?

Line 2377, Patchset 7 (Latest): [self closePresentedViews];

Gauthier Ambard . unresolved

This is a scene command, you might want to use the dispatcher to call it from the coordinator instead of a delegate (for clarity).

Line 2430, Patchset 7 (Parent): if (self.currentInterface.incognito) {

Gauthier Ambard . unresolved

You removed that?

Open in Gerrit

Related details

Attention is currently required from:

Scott Yoder

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages