[bedrock] Migrate BrowserListObserver to BrowserCollectionObserver - part 8/n [chromium/src : main]

0 views
Skip to first unread message

Qikai Zhong (Gerrit)

unread,
Feb 9, 2026, 12:37:26 AM (yesterday) Feb 9
to Zhentao Lu, Tom Lukaszewicz, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, webap...@microsoft.com, loyso...@chromium.org, mgiuca...@chromium.org, dmurph+watc...@chromium.org, philli...@chromium.org, kuragin+web-ap...@chromium.org, dibyapal+wa...@chromium.org, mek+w...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, aixba+wat...@chromium.org, dfried...@chromium.org, mickeybu...@chromium.org, estali...@chromium.org
Attention needed from Tom Lukaszewicz and Zhentao Lu

Qikai Zhong voted and added 3 comments

Votes added by Qikai Zhong

Code-Review+1

3 comments

Patchset-level comments
File chrome/browser/ui/webui/whats_new/whats_new_fetcher.cc
Line 205, Patchset 11 (Parent): if (browser != browser_) {
return;
}
Qikai Zhong . unresolved

nit: maybe add a CHECK here.

File chrome/test/base/interactive_test_utils.cc
Line 102, Patchset 11 (Parent): if (browser != browser_.get()) {
return;
}
Qikai Zhong . unresolved

nit: Maybe add a CHECK here.

Open in Gerrit

Related details

Attention is currently required from:
  • Tom Lukaszewicz
  • Zhentao Lu
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I80309f4c279838a7feef76f2f60c2407022fcf89
Gerrit-Change-Number: 7550350
Gerrit-PatchSet: 11
Gerrit-Owner: Zhentao Lu <zhent...@microsoft.com>
Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
Gerrit-Reviewer: Tom Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Zhentao Lu <zhent...@microsoft.com>
Gerrit-Attention: Zhentao Lu <zhent...@microsoft.com>
Gerrit-Attention: Tom Lukaszewicz <tl...@chromium.org>
Gerrit-Comment-Date: Mon, 09 Feb 2026 05:36:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Zhentao Lu (Gerrit)

unread,
Feb 9, 2026, 1:03:43 AM (yesterday) Feb 9
to Qikai Zhong, Tom Lukaszewicz, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, webap...@microsoft.com, loyso...@chromium.org, mgiuca...@chromium.org, dmurph+watc...@chromium.org, philli...@chromium.org, kuragin+web-ap...@chromium.org, dibyapal+wa...@chromium.org, mek+w...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, aixba+wat...@chromium.org, dfried...@chromium.org, mickeybu...@chromium.org, estali...@chromium.org
Attention needed from Tom Lukaszewicz

Zhentao Lu added 2 comments

File chrome/browser/ui/webui/whats_new/whats_new_fetcher.cc
Line 205, Patchset 11 (Parent): if (browser != browser_) {
return;
}
Qikai Zhong . resolved

nit: maybe add a CHECK here.

Zhentao Lu

Done

File chrome/test/base/interactive_test_utils.cc
Line 102, Patchset 11 (Parent): if (browser != browser_.get()) {
return;
}
Qikai Zhong . resolved

nit: Maybe add a CHECK here.

Zhentao Lu

I tend to leave it like this because the `browser_` is no longer stored as a class memeber.

Open in Gerrit

Related details

Attention is currently required from:
  • Tom Lukaszewicz
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I80309f4c279838a7feef76f2f60c2407022fcf89
    Gerrit-Change-Number: 7550350
    Gerrit-PatchSet: 11
    Gerrit-Owner: Zhentao Lu <zhent...@microsoft.com>
    Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
    Gerrit-Reviewer: Tom Lukaszewicz <tl...@chromium.org>
    Gerrit-Reviewer: Zhentao Lu <zhent...@microsoft.com>
    Gerrit-Attention: Tom Lukaszewicz <tl...@chromium.org>
    Gerrit-Comment-Date: Mon, 09 Feb 2026 06:03:10 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Qikai Zhong <qikai...@microsoft.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Tom Lukaszewicz (Gerrit)

    unread,
    Feb 9, 2026, 12:58:21 PM (24 hours ago) Feb 9
    to Zhentao Lu, Qikai Zhong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, webap...@microsoft.com, loyso...@chromium.org, mgiuca...@chromium.org, dmurph+watc...@chromium.org, philli...@chromium.org, kuragin+web-ap...@chromium.org, dibyapal+wa...@chromium.org, mek+w...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, aixba+wat...@chromium.org, dfried...@chromium.org, mickeybu...@chromium.org, estali...@chromium.org
    Attention needed from Zhentao Lu

    Tom Lukaszewicz added 4 comments

    Patchset-level comments
    File-level comment, Patchset 12 (Latest):
    Tom Lukaszewicz . resolved

    Overall looks good just one comment

    File chrome/browser/devtools/devtools_browser_context_manager.cc
    Line 121, Patchset 12 (Latest): if (pending_context_disposals_.empty())
    browser_collection_observation_.Observe(
    GlobalBrowserCollection::GetInstance());
    Tom Lukaszewicz . unresolved

    nit: Can we use curly braces for this condition? (new guidance)

    Line 172, Patchset 12 (Latest): if (pending_context_disposals_.empty())
    browser_collection_observation_.Reset();
    Tom Lukaszewicz . unresolved

    nit: Curly braces here also

    File chrome/test/base/interactive_test_utils.h
    Line 64, Patchset 12 (Latest):class BrowserDeactivationWaiter {
    Tom Lukaszewicz . unresolved

    Can we remove this class? It looks like we don't have any clients for this. If we need to keep it could we update the constructor param to be a non-const BWI pointer? (to remove the const_cast)

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Zhentao Lu
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I80309f4c279838a7feef76f2f60c2407022fcf89
      Gerrit-Change-Number: 7550350
      Gerrit-PatchSet: 12
      Gerrit-Owner: Zhentao Lu <zhent...@microsoft.com>
      Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
      Gerrit-Reviewer: Tom Lukaszewicz <tl...@chromium.org>
      Gerrit-Reviewer: Zhentao Lu <zhent...@microsoft.com>
      Gerrit-Attention: Zhentao Lu <zhent...@microsoft.com>
      Gerrit-Comment-Date: Mon, 09 Feb 2026 17:57:47 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Zhentao Lu (Gerrit)

      unread,
      Feb 9, 2026, 8:28:35 PM (16 hours ago) Feb 9
      to Qikai Zhong, Tom Lukaszewicz, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, webap...@microsoft.com, loyso...@chromium.org, mgiuca...@chromium.org, dmurph+watc...@chromium.org, philli...@chromium.org, kuragin+web-ap...@chromium.org, dibyapal+wa...@chromium.org, mek+w...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, aixba+wat...@chromium.org, dfried...@chromium.org, mickeybu...@chromium.org, estali...@chromium.org
      Attention needed from Tom Lukaszewicz

      Zhentao Lu added 3 comments

      File chrome/browser/devtools/devtools_browser_context_manager.cc
      Line 121, Patchset 12: if (pending_context_disposals_.empty())
      browser_collection_observation_.Observe(
      GlobalBrowserCollection::GetInstance());
      Tom Lukaszewicz . resolved

      nit: Can we use curly braces for this condition? (new guidance)

      Zhentao Lu

      Done

      Line 172, Patchset 12: if (pending_context_disposals_.empty())
      browser_collection_observation_.Reset();
      Tom Lukaszewicz . resolved

      nit: Curly braces here also

      Zhentao Lu

      Done

      File chrome/test/base/interactive_test_utils.h
      Line 64, Patchset 12:class BrowserDeactivationWaiter {
      Tom Lukaszewicz . resolved

      Can we remove this class? It looks like we don't have any clients for this. If we need to keep it could we update the constructor param to be a non-const BWI pointer? (to remove the const_cast)

      Zhentao Lu

      Done. Removed the class.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Tom Lukaszewicz
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I80309f4c279838a7feef76f2f60c2407022fcf89
        Gerrit-Change-Number: 7550350
        Gerrit-PatchSet: 13
        Gerrit-Owner: Zhentao Lu <zhent...@microsoft.com>
        Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
        Gerrit-Reviewer: Tom Lukaszewicz <tl...@chromium.org>
        Gerrit-Reviewer: Zhentao Lu <zhent...@microsoft.com>
        Gerrit-Attention: Tom Lukaszewicz <tl...@chromium.org>
        Gerrit-Comment-Date: Tue, 10 Feb 2026 01:28:03 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Tom Lukaszewicz <tl...@chromium.org>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Tom Lukaszewicz (Gerrit)

        unread,
        Feb 9, 2026, 9:23:44 PM (15 hours ago) Feb 9
        to Zhentao Lu, Qikai Zhong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, webap...@microsoft.com, loyso...@chromium.org, mgiuca...@chromium.org, dmurph+watc...@chromium.org, philli...@chromium.org, kuragin+web-ap...@chromium.org, dibyapal+wa...@chromium.org, mek+w...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, aixba+wat...@chromium.org, dfried...@chromium.org, mickeybu...@chromium.org, estali...@chromium.org
        Attention needed from Zhentao Lu

        Tom Lukaszewicz voted and added 1 comment

        Votes added by Tom Lukaszewicz

        Code-Review+1
        Commit-Queue+2

        1 comment

        Patchset-level comments
        File-level comment, Patchset 13 (Latest):
        Tom Lukaszewicz . resolved

        Thanks, lgtm!

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Zhentao Lu
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I80309f4c279838a7feef76f2f60c2407022fcf89
        Gerrit-Change-Number: 7550350
        Gerrit-PatchSet: 13
        Gerrit-Owner: Zhentao Lu <zhent...@microsoft.com>
        Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
        Gerrit-Reviewer: Tom Lukaszewicz <tl...@chromium.org>
        Gerrit-Reviewer: Zhentao Lu <zhent...@microsoft.com>
        Gerrit-Attention: Zhentao Lu <zhent...@microsoft.com>
        Gerrit-Comment-Date: Tue, 10 Feb 2026 02:23:11 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        open
        diffy

        Chromium LUCI CQ (Gerrit)

        unread,
        Feb 9, 2026, 10:44:53 PM (14 hours ago) Feb 9
        to Zhentao Lu, Tom Lukaszewicz, Qikai Zhong, AyeAye, chromium...@chromium.org, devtools...@chromium.org, webap...@microsoft.com, loyso...@chromium.org, mgiuca...@chromium.org, dmurph+watc...@chromium.org, philli...@chromium.org, kuragin+web-ap...@chromium.org, dibyapal+wa...@chromium.org, mek+w...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, aixba+wat...@chromium.org, dfried...@chromium.org, mickeybu...@chromium.org, estali...@chromium.org

        Chromium LUCI CQ submitted the change

        Change information

        Commit message:
        [bedrock] Migrate BrowserListObserver to BrowserCollectionObserver - part 8/n

        This migration is part of project bedrock to reduce the dependencies on
        Browser and BrowserList. See https://crbug.com/431671320 for more info.
        Bug: 431671320
        Change-Id: I80309f4c279838a7feef76f2f60c2407022fcf89
        Commit-Queue: Tom Lukaszewicz <tl...@chromium.org>
        Commit-Queue: Zhentao Lu <zhent...@microsoft.com>
        Reviewed-by: Qikai Zhong <qikai...@microsoft.com>
        Reviewed-by: Tom Lukaszewicz <tl...@chromium.org>
        Cr-Commit-Position: refs/heads/main@{#1582244}
        Files:
        • M chrome/browser/devtools/devtools_browser_context_manager.cc
        • M chrome/browser/devtools/devtools_browser_context_manager.h
        • M chrome/browser/sync/sessions/browser_list_router_helper.cc
        • M chrome/browser/sync/sessions/browser_list_router_helper.h
        • M chrome/browser/ui/browser_tab_strip_tracker.cc
        • M chrome/browser/ui/browser_tab_strip_tracker.h
        • M chrome/browser/ui/views/profiles/profile_picker_view_browsertest.cc
        • M chrome/browser/ui/views/web_apps/web_app_integration_test_driver.cc
        • M chrome/browser/ui/web_applications/test/web_app_browsertest_util.cc
        • M chrome/browser/ui/web_applications/test/web_app_browsertest_util.h
        • M chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc
        • M chrome/browser/ui/web_applications/web_app_ui_manager_impl.h
        • M chrome/browser/ui/webui/whats_new/whats_new_fetcher.cc
        • M chrome/test/base/interactive_test_utils.cc
        • M chrome/test/base/interactive_test_utils.h
        • M chrome/test/interaction/webcontents_interaction_test_util.cc
        Change size: L
        Delta: 16 files changed, 157 insertions(+), 194 deletions(-)
        Branch: refs/heads/main
        Submit Requirements:
        • requirement satisfiedCode-Review: +1 by Tom Lukaszewicz, +1 by Qikai Zhong
        Open in Gerrit
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: merged
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I80309f4c279838a7feef76f2f60c2407022fcf89
        Gerrit-Change-Number: 7550350
        Gerrit-PatchSet: 14
        Gerrit-Owner: Zhentao Lu <zhent...@microsoft.com>
        Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
        Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
        Gerrit-Reviewer: Tom Lukaszewicz <tl...@chromium.org>
        Gerrit-Reviewer: Zhentao Lu <zhent...@microsoft.com>
        open
        diffy
        satisfied_requirement

        luci-bisection@appspot.gserviceaccount.com (Gerrit)

        unread,
        10:32 AM (2 hours ago) 10:32 AM
        to Zhentao Lu, Chromium LUCI CQ, Tom Lukaszewicz, Qikai Zhong, AyeAye, chromium...@chromium.org, devtools...@chromium.org, webap...@microsoft.com, loyso...@chromium.org, mgiuca...@chromium.org, dmurph+watc...@chromium.org, philli...@chromium.org, kuragin+web-ap...@chromium.org, dibyapal+wa...@chromium.org, mek+w...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, aixba+wat...@chromium.org, dfried...@chromium.org, mickeybu...@chromium.org, estali...@chromium.org

        luci-bi...@appspot.gserviceaccount.com has created a revert of this change

        Related details

        Attention set is empty
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: revert
        satisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages