Scott Yoder (Gerrit)

unread,

Jan 12, 2026, 10:04:18 AM (20 hours ago) Jan 12

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

Attention needed from Gauthier Ambard and Ginny Huang

Scott Yoder added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Scott Yoder . resolved

Please take a look.

Open in Gerrit

Related details

Attention is currently required from:

Gauthier Ambard
Ginny Huang

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

Ginny Huang (Gerrit)

unread,

Jan 12, 2026, 10:08:01 AM (20 hours ago) Jan 12

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

Attention needed from Gauthier Ambard and Scott Yoder

Ginny Huang added 1 comment

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

Line 2414, Patchset 1 (Parent):

    [self closePresentedViews:YES
                   completion:^{
                     [safariDataImportCoordinator start];
                   }];

Ginny Huang . unresolved

How is this handled?

Open in Gerrit

Related details

Attention is currently required from:

Gauthier Ambard
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

Scott Yoder (Gerrit)

unread,

Jan 12, 2026, 10:29:34 AM (19 hours ago) Jan 12

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

Attention needed from Gauthier Ambard and Ginny Huang

Scott Yoder voted and added 1 comment

Votes added by Scott Yoder

Commit-Queue

+1

1 comment

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

Line 2414, Patchset 1 (Parent):

    [self closePresentedViews:YES
                   completion:^{
                     [safariDataImportCoordinator start];
                   }];

Ginny Huang . resolved

How is this handled?

Scott Yoder

Thanks for catching that - I had intended to circle back to this. I've added a call to closePresentedViews back in when !presentedOverSettings. It's a little awkward for now, because closePresentedViews can't be migrated to SceneCoordinator yet.

Open in Gerrit

Related details

Attention is currently required from:

Gauthier Ambard
Ginny Huang

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

Ginny Huang (Gerrit)

unread,

Jan 12, 2026, 10:40:12 AM (19 hours ago) Jan 12

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

Attention needed from Gauthier Ambard and Scott Yoder

Ginny Huang voted and added 1 comment

Votes added by Ginny Huang

Code-Review

+1

1 comment

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

Line 2393, Patchset 3 (Latest):

  __weak __typeof(self.mainCoordinator) weakMainCoordinator =
      self.mainCoordinator;

Ginny Huang . unresolved

Maybe this doesn't need to be weak :)

Open in Gerrit

Related details

Attention is currently required from:

Gauthier Ambard
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

Gauthier Ambard (Gerrit)

unread,

Jan 12, 2026, 10:46:16 AM (19 hours ago) Jan 12

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

Attention needed from Scott Yoder

Gauthier Ambard voted Code-Review+1

Code-Review

+1

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

Scott Yoder (Gerrit)

unread,

Jan 12, 2026, 11:00:44 AM (19 hours ago) Jan 12

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

Scott Yoder voted and added 1 comment

Votes added by Scott Yoder

Commit-Queue

+2

1 comment

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

Line 2393, Patchset 3 (Latest):

  __weak __typeof(self.mainCoordinator) weakMainCoordinator =
      self.mainCoordinator;

Ginny Huang . resolved

Maybe this doesn't need to be weak :)

Scott Yoder

Maybe not, but I don't think it hurts. If mainCoordinator goes away before it displays, it is because the scene is destroyed and we can stop trying to show the import screen.

Open in Gerrit

Related details

Attention set is empty

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

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jan 12, 2026, 11:52:19 AM (18 hours ago) Jan 12

to Scott Yoder, Gauthier Ambard, Ginny Huang, chromium...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

[ios] Move ownership of Safari Data Import to SceneCoordinator

Moves the responsibility of managing the Safari Data Import workflow
from SceneController to SceneCoordinator. This is part of the a
larger migration that will eventually have SceneCoordinator
implementing SceneCommands.

Fixed: 474643387

Change-Id: Ief3cf1e3ec41d8ed07a8ec13be9a7d06c0c2aeac

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7427241

Reviewed-by: Ginny Huang <ginny...@chromium.org>

Reviewed-by: Gauthier Ambard <gam...@chromium.org>

Commit-Queue: Scott Yoder <scott...@google.com>

Cr-Commit-Position: refs/heads/main@{#1567819}

Files:

M ios/chrome/browser/scene/coordinator/BUILD.gn
M ios/chrome/browser/scene/coordinator/DEPS
M ios/chrome/browser/scene/coordinator/scene_coordinator.h
M ios/chrome/browser/scene/coordinator/scene_coordinator.mm
M ios/chrome/browser/shared/coordinator/scene/BUILD.gn
M ios/chrome/browser/shared/coordinator/scene/DEPS
M ios/chrome/browser/shared/coordinator/scene/scene_controller.mm

Change size: M

Delta: 7 files changed, 75 insertions(+), 42 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Gauthier Ambard, +1 by Ginny Huang

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

[ios] Move ownership of Safari Data Import to SceneCoordinator [chromium/src : main]

Scott Yoder (Gerrit)

Scott Yoder added 1 comment

Related details

Ginny Huang (Gerrit)

Ginny Huang added 1 comment

Related details

Scott Yoder (Gerrit)

Scott Yoder voted and added 1 comment

Votes added by Scott Yoder

1 comment

Related details

Ginny Huang (Gerrit)

Ginny Huang voted and added 1 comment

Votes added by Ginny Huang

1 comment

Related details

Gauthier Ambard (Gerrit)

Gauthier Ambard voted Code-Review+1

Related details

Scott Yoder (Gerrit)

Scott Yoder voted and added 1 comment

Votes added by Scott Yoder

1 comment

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information