James Cook (Gerrit)

unread,

Jan 9, 2026, 2:58:36 PM (yesterday) Jan 9

to Zoraiz Naeem, Mike Wasserman, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Mike Wasserman and Zoraiz Naeem

James Cook added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

James Cook . resolved

Zoraiz, please take a look at //chrome/browser/extensions
Mike, please take a look at //chrome/browser/ui. There are a few Android C++ files in there, but they are all just stub methods. Hope that's OK.
Thanks!

Open in Gerrit

Related details

Attention is currently required from:

Mike Wasserman
Zoraiz Naeem

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

Mike Wasserman (Gerrit)

unread,

Jan 9, 2026, 3:09:14 PM (yesterday) Jan 9

to James Cook, Zoraiz Naeem, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from James Cook and Zoraiz Naeem

Mike Wasserman voted and added 2 comments

Votes added by Mike Wasserman

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Mike Wasserman . resolved

lgtm with an edge case question

File chrome/browser/ui/tabs/tab_list_bridge.cc

Line 269, Patchset 1 (Latest): return tab_group->ListTabs();

Mike Wasserman . unresolved

Should the non-contiguous intermediate state concerns from this function's comment be expressed in a GetTabGroupTabIndices function comment? Or is there some better behavior we could consider?

Open in Gerrit

Related details

Attention is currently required from:

James Cook
Zoraiz Naeem

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

James Cook (Gerrit)

unread,

Jan 9, 2026, 7:44:17 PM (21 hours ago) Jan 9

to Mike Wasserman, Zoraiz Naeem, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Zoraiz Naeem

James Cook voted and added 2 comments

Votes added by James Cook

Auto-Submit	+1
Commit-Queue	+1

2 comments

Patchset-level comments

File-level comment, Patchset 2 (Latest):

James Cook . resolved

Zoraiz, over to you.

Mike, thanks for the review!

File chrome/browser/ui/tabs/tab_list_bridge.cc

Line 269, Patchset 1: return tab_group->ListTabs();

Mike Wasserman . resolved

Should the non-contiguous intermediate state concerns from this function's comment be expressed in a GetTabGroupTabIndices function comment? Or is there some better behavior we could consider?

James Cook

I updated the comment. I think changing the behavior would be difficult, as observers could fire pretty much any time and I suspect it would be hard to enforce that the return value is always contiguous.

Open in Gerrit

Related details

Attention is currently required from:

Zoraiz Naeem

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

Zoraiz Naeem (Gerrit)

unread,

Jan 9, 2026, 8:31:28 PM (20 hours ago) Jan 9

to James Cook, Mike Wasserman, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from James Cook

Zoraiz Naeem voted and added 3 comments

Votes added by Zoraiz Naeem

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Zoraiz Naeem . resolved

lgtm & nit

File chrome/browser/extensions/api/tab_groups/tab_groups_api.cc

Line 84, Patchset 1: std::optional<tab_groups::TabGroupId> target_group = target_tab->GetGroup();

Zoraiz Naeem . unresolved

optional nit: move after line 87?

File chrome/browser/ui/android/tab_model/tab_model_test_helper.h

Line 230, Patchset 1: gfx::Range GetTabGroupTabIndices(tab_groups::TabGroupId group_id) override;

Zoraiz Naeem . unresolved

Wasn't aware of `gfx::Range`. Good to know data structure!

Open in Gerrit

Related details

Attention is currently required from:

James Cook

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

James Cook (Gerrit)

unread,

Jan 9, 2026, 9:12:15 PM (19 hours ago) Jan 9

to Zoraiz Naeem, Mike Wasserman, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

James Cook voted and added 3 comments

Votes added by James Cook

Auto-Submit	+0
Commit-Queue	+2

3 comments

Patchset-level comments

File-level comment, Patchset 2 (Latest):

James Cook . resolved

Thanks for the reviews!

File chrome/browser/extensions/api/tab_groups/tab_groups_api.cc

Line 84, Patchset 1: std::optional<tab_groups::TabGroupId> target_group = target_tab->GetGroup();

Zoraiz Naeem . resolved

optional nit: move after line 87?

James Cook

I think it makes more sense to compute the target_group before the adjacent_group. Since you said optional, I'm going to leave as-is.

File chrome/browser/ui/android/tab_model/tab_model_test_helper.h

Line 230, Patchset 1: gfx::Range GetTabGroupTabIndices(tab_groups::TabGroupId group_id) override;

Zoraiz Naeem . resolved

Wasn't aware of `gfx::Range`. Good to know data structure!

James Cook

Acknowledged

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 9, 2026, 9:16:00 PM (19 hours ago) Jan 9

to James Cook, Zoraiz Naeem, Mike Wasserman, Achuith Bhandarkar, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

extensions: Port chrome.tabGroups.move() to desktop Android, part 1

This converts most of the tab groups API implementation to use cross
platform BrowserWindowInterface/TabListInterface concepts.

It adds a method to TabListInterface to get a list of the tabs in a
group, which is needed for this API. It's left not-implemented on
Android. I'll do the JNI/Java in a followup CL.

This CL only affects Win/Mac/Linux/ChromeOS and is well covered by
existing browser tests. I started porting one test to Android, but it
depends on the JNI I haven't added yet, so I'll add it when I add
the JNI.

Bug: 405219902

Change-Id: I26c8b35640dc24112dc5f09b1f1ce12f84c1600f

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

Reviewed-by: Mike Wasserman <m...@chromium.org>

Commit-Queue: James Cook <jame...@chromium.org>

Reviewed-by: Zoraiz Naeem <zorai...@chromium.org>

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

Files:

M chrome/browser/extensions/api/tab_groups/tab_groups_api.cc
M chrome/browser/extensions/api/tab_groups/tab_groups_api.h
M chrome/browser/extensions/api/tab_groups/tab_groups_api_browsertest.cc
M chrome/browser/extensions/extension_tab_util.cc
M chrome/browser/extensions/extension_tab_util.h
M chrome/browser/ui/android/tab_model/tab_model_jni_bridge.cc
M chrome/browser/ui/android/tab_model/tab_model_jni_bridge.h
M chrome/browser/ui/android/tab_model/tab_model_test_helper.cc
M chrome/browser/ui/android/tab_model/tab_model_test_helper.h
M chrome/browser/ui/tabs/tab_list_bridge.cc
M chrome/browser/ui/tabs/tab_list_bridge.h
M chrome/browser/ui/tabs/tab_list_interface.h

Change size: M

Delta: 12 files changed, 137 insertions(+), 66 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Zoraiz Naeem, +1 by Mike Wasserman

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

extensions: Port chrome.tabGroups.move() to desktop Android, part 1 [chromium/src : main]

James Cook (Gerrit)

James Cook added 1 comment

Related details

Mike Wasserman (Gerrit)

Mike Wasserman voted and added 2 comments

Votes added by Mike Wasserman

2 comments

Related details

James Cook (Gerrit)

James Cook voted and added 2 comments

Votes added by James Cook

2 comments

Related details

Zoraiz Naeem (Gerrit)

Zoraiz Naeem voted and added 3 comments

Votes added by Zoraiz Naeem

3 comments

Related details

James Cook (Gerrit)

James Cook voted and added 3 comments

Votes added by James Cook

3 comments

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information