Devlin Cronin (Gerrit)

unread,

Jul 14, 2026, 7:01:18 PMJul 14

to Devlin Cronin, Patrick Monette, Darryl James, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Darryl James and Patrick Monette

Devlin Cronin added 1 comment

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Devlin Cronin . resolved

Heya folks, mind taking a look?
Patrick: resource_coordinator
Darryl: c/b/ui

Thanks!

Open in Gerrit

Related details

Attention is currently required from:

Darryl James
Patrick Monette

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

Darryl James (Gerrit)

unread,

Jul 15, 2026, 1:40:29 PMJul 15

to Devlin Cronin, Patrick Monette, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Devlin Cronin and Patrick Monette

Darryl James added 1 comment

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Darryl James . resolved

c/b/ui lgtm % waiting on area owner to approve first 😄

Open in Gerrit

Related details

Attention is currently required from:

Devlin Cronin
Patrick Monette

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

Patrick Monette (Gerrit)

unread,

Jul 16, 2026, 10:33:14 AMJul 16

to Devlin Cronin, Darryl James, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Devlin Cronin

Patrick Monette added 1 comment

File chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc

Line 161, Patchset 7 (Latest):

  auto auto_reset = base::AutoReset<raw_ptr<TabStripModel>>(
      &focused_tab_strip_model_for_testing_, tab_strip);

Patrick Monette . unresolved

Before this change, to reset the tab strip model, you had to call SetFocusedTabStripModelForTesting() again, which would invoke UpdateFocusedTab().

Now this UpdateFocusedTab() call is gone.

Is that intentional?

Open in Gerrit

Related details

Attention is currently required from:

Devlin Cronin

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

Devlin Cronin (Gerrit)

unread,

Jul 16, 2026, 5:17:09 PMJul 16

to Devlin Cronin, Patrick Monette, Darryl James, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Patrick Monette

Devlin Cronin added 1 comment

File chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc

Line 161, Patchset 7:

  auto auto_reset = base::AutoReset<raw_ptr<TabStripModel>>(
      &focused_tab_strip_model_for_testing_, tab_strip);

Patrick Monette . resolved

Before this change, to reset the tab strip model, you had to call SetFocusedTabStripModelForTesting() again, which would invoke UpdateFocusedTab().
Now this UpdateFocusedTab() call is gone.
Is that intentional?

Devlin Cronin

Great catch!

ScopedClosureRunner it is : )

Open in Gerrit

Related details

Attention is currently required from:

Patrick Monette

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

Patrick Monette (Gerrit)

unread,

Jul 17, 2026, 11:40:02 AMJul 17

to Devlin Cronin, Darryl James, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Devlin Cronin

Patrick Monette voted and added 4 comments

Votes added by Patrick Monette

Code-Review

+1

4 comments

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Patrick Monette . resolved

lgtm with nits

Commit Message

Line 7, Patchset 8 (Latest):[Resource Coordinator] Use an AutoReset for testing focused tab strip

Patrick Monette . unresolved

fix this commit message, here and below, as you're no longer using AutoReset

File chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h

Line 70, Patchset 8 (Latest): base::ScopedClosureRunner SetFocusedTabStripModelForTesting(

Patrick Monette . unresolved

add [[nodiscard]]

Line 8, Patchset 8 (Latest):#include "base/auto_reset.h"

Patrick Monette . unresolved

remove unused header.

Open in Gerrit

Related details

Attention is currently required from:

Devlin Cronin

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

Devlin Cronin (Gerrit)

unread,

Jul 17, 2026, 2:12:26 PMJul 17

to Devlin Cronin, Patrick Monette, Darryl James, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Darryl James

Devlin Cronin added 4 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Devlin Cronin . resolved

Thanks, Patrick!

Back to you for the stamp, Darryl : )

Commit Message

Line 7, Patchset 8:[Resource Coordinator] Use an AutoReset for testing focused tab strip

Patrick Monette . resolved

fix this commit message, here and below, as you're no longer using AutoReset

Devlin Cronin

Done

File chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h

Line 70, Patchset 8: base::ScopedClosureRunner SetFocusedTabStripModelForTesting(

Patrick Monette . resolved

add [[nodiscard]]

Devlin Cronin

Done

Line 8, Patchset 8:#include "base/auto_reset.h"

Patrick Monette . resolved

remove unused header.

Devlin Cronin

Done

Open in Gerrit

Related details

Attention is currently required from:

Darryl James

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

Darryl James (Gerrit)

unread,

Jul 20, 2026, 1:31:49 PMJul 20

to Devlin Cronin, Patrick Monette, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Devlin Cronin

Darryl James voted and added 1 comment

Votes added by Darryl James

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Darryl James . resolved

lgtm for test files; thanks!

Open in Gerrit

Related details

Attention is currently required from:

Devlin Cronin

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

Devlin Cronin (Gerrit)

unread,

Jul 20, 2026, 2:33:16 PMJul 20

to Devlin Cronin, Darryl James, Patrick Monette, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Devlin Cronin voted Commit-Queue+2

Commit-Queue

+2

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,

Jul 20, 2026, 5:55:04 PMJul 20

to Devlin Cronin, Darryl James, Patrick Monette, chromium...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

[Resource Coordinator] Use an AutoReset for testing focused tab strip

Use a ScopedClosureRunner in the SetFocusedTabStripModelForTesting()
method in TabLifecycleUnitSource. This prevents tests from having to
remember to reset the variable (which can be important, since it's
global state attached to the BrowserProcess).

Update all callers as appropriate.

As a bonus, this also fixes the dangling pointer associated with the
overridden focused tab strip.

Bug: None

Change-Id: I744a6a0d1c1a598c6933206f522afd235e2fd2a5

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

Reviewed-by: Darryl James <dlj...@chromium.org>

Reviewed-by: Patrick Monette <pmon...@chromium.org>

Commit-Queue: Devlin Cronin <rdevlin...@chromium.org>

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

Files:

M chrome/browser/extensions/api/tabs/tabs_test.cc
M chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc
M chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
M chrome/browser/resource_coordinator/tab_manager_browsertest.cc
M chrome/browser/ui/performance_controls/test_support/memory_saver_browser_test_mixin.h
M chrome/browser/ui/thumbnails/thumbnail_tab_helper_interactive_uitest.cc
M chrome/browser/ui/views/tabs/hovercard/tab_hover_card_controller_interactive_uitest.cc

Change size: M

Delta: 8 files changed, 94 insertions(+), 64 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Patrick Monette, +1 by Darryl James

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

[Resource Coordinator] Use an AutoReset for testing focused tab strip [chromium/src : main]

Devlin Cronin (Gerrit)

Devlin Cronin added 1 comment

Related details

Darryl James (Gerrit)

Darryl James added 1 comment

Related details

Patrick Monette (Gerrit)

Patrick Monette added 1 comment

Related details

Devlin Cronin (Gerrit)

Devlin Cronin added 1 comment

Related details

Patrick Monette (Gerrit)

Patrick Monette voted and added 4 comments

Votes added by Patrick Monette

4 comments

Related details

Devlin Cronin (Gerrit)

Devlin Cronin added 4 comments

Related details

Darryl James (Gerrit)

Darryl James voted and added 1 comment

Votes added by Darryl James

1 comment

Related details

Devlin Cronin (Gerrit)

Devlin Cronin voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information