[bedrock] Migrate chrome::FindTabbedBrowser() to ProfileBrowserCollection::FindTabbedBrowser() [chromium/src : main]

0 views
Skip to first unread message

Yu He (Gerrit)

unread,
Mar 27, 2026, 5:58:59 AM (6 days ago) Mar 27
to Qikai Zhong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, lens-chrome...@google.com, stanfie...@google.com, mercer...@google.com, rginda...@chromium.org, aixba+wat...@chromium.org, ayman...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, dibyapal+wa...@chromium.org, dmurph+wat...@chromium.org, dmurph+watc...@chromium.org, extension...@chromium.org, feature-v...@chromium.org, japhet+...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mdjone...@chromium.org, mek+w...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, yuezhang...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Qikai Zhong

Yu He voted Commit-Queue+1

Commit-Queue+1
Open in Gerrit

Related details

Attention is currently required from:
  • Qikai Zhong
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not 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: Iee13b0317b694f976071ae0c0a462ab53a9144f9
Gerrit-Change-Number: 7684422
Gerrit-PatchSet: 29
Gerrit-Owner: Yu He <y...@microsoft.com>
Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
Gerrit-Reviewer: Yu He <y...@microsoft.com>
Gerrit-Attention: Qikai Zhong <qikai...@microsoft.com>
Gerrit-Comment-Date: Fri, 27 Mar 2026 09:58:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Qikai Zhong (Gerrit)

unread,
Mar 30, 2026, 4:08:23 AM (3 days ago) Mar 30
to Yu He, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, lens-chrome...@google.com, stanfie...@google.com, mercer...@google.com, rginda...@chromium.org, aixba+wat...@chromium.org, ayman...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, dibyapal+wa...@chromium.org, dmurph+wat...@chromium.org, dmurph+watc...@chromium.org, extension...@chromium.org, feature-v...@chromium.org, japhet+...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mdjone...@chromium.org, mek+w...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, yuezhang...@chromium.org, zelin+watch-we...@chromium.org
Attention needed from Yu He

Qikai Zhong added 2 comments

Patchset-level comments
File-level comment, Patchset 29 (Latest):
Qikai Zhong . unresolved

We should first determine the destination of the new API. Add new column on task page.

Commit Message
Line 15, Patchset 29 (Latest):- Move FindTabbedBrowser() from BrowserCollection (base class) to
ProfileBrowserCollection (subclass), since it requires profile-aware
logic for match_original_profiles support.
Qikai Zhong . unresolved

From browser_collection.cc? And match_original_profiles look like have a larger scope than `ProfileBrowserCollection`. We may need to find a better place to put `FindTabbedBrowser()`.

Open in Gerrit

Related details

Attention is currently required from:
  • Yu He
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not 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: Iee13b0317b694f976071ae0c0a462ab53a9144f9
    Gerrit-Change-Number: 7684422
    Gerrit-PatchSet: 29
    Gerrit-Owner: Yu He <y...@microsoft.com>
    Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
    Gerrit-Reviewer: Yu He <y...@microsoft.com>
    Gerrit-Attention: Yu He <y...@microsoft.com>
    Gerrit-Comment-Date: Mon, 30 Mar 2026 08:07:45 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Yu He (Gerrit)

    unread,
    Mar 31, 2026, 9:34:51 PM (2 days ago) Mar 31
    to Qikai Zhong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, lens-chrome...@google.com, stanfie...@google.com, mercer...@google.com, rginda...@chromium.org, aixba+wat...@chromium.org, ayman...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, dibyapal+wa...@chromium.org, dmurph+wat...@chromium.org, dmurph+watc...@chromium.org, extension...@chromium.org, feature-v...@chromium.org, japhet+...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mdjone...@chromium.org, mek+w...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, yuezhang...@chromium.org, zelin+watch-we...@chromium.org
    Attention needed from Qikai Zhong

    Yu He added 2 comments

    Patchset-level comments
    File-level comment, Patchset 29:
    Qikai Zhong . resolved

    We should first determine the destination of the new API. Add new column on task page.

    Yu He

    Sure.

    Commit Message
    Line 15, Patchset 29:- Move FindTabbedBrowser() from BrowserCollection (base class) to

    ProfileBrowserCollection (subclass), since it requires profile-aware
    logic for match_original_profiles support.
    Qikai Zhong . resolved

    From browser_collection.cc? And match_original_profiles look like have a larger scope than `ProfileBrowserCollection`. We may need to find a better place to put `FindTabbedBrowser()`.

    Yu He

    ok

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Qikai Zhong
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not 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: Iee13b0317b694f976071ae0c0a462ab53a9144f9
      Gerrit-Change-Number: 7684422
      Gerrit-PatchSet: 35
      Gerrit-Owner: Yu He <y...@microsoft.com>
      Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
      Gerrit-Reviewer: Yu He <y...@microsoft.com>
      Gerrit-Attention: Qikai Zhong <qikai...@microsoft.com>
      Gerrit-Comment-Date: Wed, 01 Apr 2026 01:34:22 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Qikai Zhong <qikai...@microsoft.com>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Qikai Zhong (Gerrit)

      unread,
      Apr 1, 2026, 1:00:44 AM (yesterday) Apr 1
      to Yu He, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, lens-chrome...@google.com, stanfie...@google.com, mercer...@google.com, rginda...@chromium.org, aixba+wat...@chromium.org, ayman...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, dibyapal+wa...@chromium.org, dmurph+wat...@chromium.org, dmurph+watc...@chromium.org, extension...@chromium.org, feature-v...@chromium.org, japhet+...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mdjone...@chromium.org, mek+w...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, yuezhang...@chromium.org, zelin+watch-we...@chromium.org
      Attention needed from Yu He

      Qikai Zhong added 9 comments

      Patchset-level comments
      File-level comment, Patchset 35 (Latest):
      Qikai Zhong . resolved

      Leave some comments

      File chrome/browser/extensions/api/search/search_api.cc
      Line 117, Patchset 35 (Latest): if (!profile) {
      return RespondNow(Error("No active browser."));
      }
      Qikai Zhong . unresolved

      We may not need this nullptr check since profile is from `Profile* profile = Profile::FromBrowserContext(browser_context());`

      Line 126, Patchset 35 (Latest): [&browser, original](BrowserWindowInterface* bwi) {
      Qikai Zhong . unresolved

      Please try to avoid bwi

      Line 121, Patchset 35 (Latest): // When the extension has incognito access, search across both
      // regular and incognito profiles in global activation order to
      // replicate the old match_original_profiles semantics.
      Profile* original = profile->GetOriginalProfile();
      ForEachCurrentBrowserWindowInterfaceOrderedByActivation(
      [&browser, original](BrowserWindowInterface* bwi) {
      if (bwi->GetType() != BrowserWindowInterface::TYPE_NORMAL) {
      return true;
      }
      if (bwi->IsDeleteScheduled()) {
      return true;
      }
      if (bwi->GetProfile()->GetOriginalProfile() != original) {
      return true;
      }
      browser = bwi;
      return false; // stop iterating
      });
      Qikai Zhong . unresolved

      Maybe `ProfileBrowserCollection::GetForProfile(profile->GetOriginalProfile())` works here?

      File chrome/browser/ui/browser_window/internal/profile_browser_collection.cc
      Line 14, Patchset 35 (Latest):#include "chrome/browser/ui/browser.h"
      #include "chrome/browser/ui/browser_window.h"
      #include "ui/display/screen.h"
      Qikai Zhong . unresolved

      Better move these into
      #if BUILDFLAG(IS_ANDROID)
      #else
      here
      #endif

      File chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm
      Line 71, Patchset 35 (Latest): BrowserWindowInterface* browser_window =
      Qikai Zhong . unresolved

      Maybe browser_window_interface/current_browser, browser_window is another class.

      File chrome/browser/ui/views/web_apps/web_app_integration_test_driver.cc
      Line 4899, Patchset 35 (Latest): BrowserWindowInterface* browser_window =
      Qikai Zhong . unresolved

      Same here.

      File chrome/browser/ui/views/web_apps/web_ui_web_app_browsertest.cc
      Line 94, Patchset 35 (Latest): BrowserWindowInterface* new_browser_window =
      Qikai Zhong . unresolved

      Same here.

      File chrome/test/base/in_process_browser_test.cc
      Line 759, Patchset 35 (Latest): BrowserWindowInterface* browser_window =
      Qikai Zhong . unresolved

      Same here

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Yu He
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Owners
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not 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: Iee13b0317b694f976071ae0c0a462ab53a9144f9
        Gerrit-Change-Number: 7684422
        Gerrit-PatchSet: 35
        Gerrit-Owner: Yu He <y...@microsoft.com>
        Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
        Gerrit-Reviewer: Yu He <y...@microsoft.com>
        Gerrit-Attention: Yu He <y...@microsoft.com>
        Gerrit-Comment-Date: Wed, 01 Apr 2026 05:00:09 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Yu He (Gerrit)

        unread,
        4:02 AM (11 hours ago) 4:02 AM
        to Qikai Zhong, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, lens-chrome...@google.com, stanfie...@google.com, mercer...@google.com, rginda...@chromium.org, aixba+wat...@chromium.org, ayman...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, dibyapal+wa...@chromium.org, dmurph+wat...@chromium.org, dmurph+watc...@chromium.org, extension...@chromium.org, feature-v...@chromium.org, japhet+...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mdjone...@chromium.org, mek+w...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, yuezhang...@chromium.org, zelin+watch-we...@chromium.org
        Attention needed from Qikai Zhong

        Yu He added 8 comments

        File chrome/browser/extensions/api/search/search_api.cc
        Line 117, Patchset 35: if (!profile) {

        return RespondNow(Error("No active browser."));
        }
        Qikai Zhong . unresolved

        We may not need this nullptr check since profile is from `Profile* profile = Profile::FromBrowserContext(browser_context());`

        Yu He

        SearchApiBrowserTest.NoActiveBrowser passed nullptr as BrowserContext results in an empty profile. Without this check, a null pointer exception will occur in subsequent calls to profile->GetOriginalProfile() or GetForProfile(profile).

        Line 126, Patchset 35: [&browser, original](BrowserWindowInterface* bwi) {
        Qikai Zhong . resolved

        Please try to avoid bwi

        Yu He

        got it

        Line 121, Patchset 35: // When the extension has incognito access, search across both

        // regular and incognito profiles in global activation order to
        // replicate the old match_original_profiles semantics.
        Profile* original = profile->GetOriginalProfile();
        ForEachCurrentBrowserWindowInterfaceOrderedByActivation(
        [&browser, original](BrowserWindowInterface* bwi) {
        if (bwi->GetType() != BrowserWindowInterface::TYPE_NORMAL) {
        return true;
        }
        if (bwi->IsDeleteScheduled()) {
        return true;
        }
        if (bwi->GetProfile()->GetOriginalProfile() != original) {
        return true;
        }
        browser = bwi;
        return false; // stop iterating
        });
        Qikai Zhong . resolved

        Maybe `ProfileBrowserCollection::GetForProfile(profile->GetOriginalProfile())` works here?

        Yu He

        It cannot match both normal and incognito browsers under the incognito path.

        File chrome/browser/ui/browser_window/internal/profile_browser_collection.cc
        Line 14, Patchset 35:#include "chrome/browser/ui/browser.h"
        #include "chrome/browser/ui/browser_window.h"
        #include "ui/display/screen.h"
        Qikai Zhong . resolved

        Better move these into
        #if BUILDFLAG(IS_ANDROID)
        #else
        here
        #endif

        Yu He

        right

        File chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm
        Line 71, Patchset 35: BrowserWindowInterface* browser_window =
        Qikai Zhong . resolved

        Maybe browser_window_interface/current_browser, browser_window is another class.

        Yu He

        got it

        File chrome/browser/ui/views/web_apps/web_app_integration_test_driver.cc
        Line 4899, Patchset 35: BrowserWindowInterface* browser_window =
        Qikai Zhong . resolved

        Same here.

        Yu He

        ok

        File chrome/browser/ui/views/web_apps/web_ui_web_app_browsertest.cc
        Line 94, Patchset 35: BrowserWindowInterface* new_browser_window =
        Qikai Zhong . resolved

        Same here.

        Yu He

        ok

        File chrome/test/base/in_process_browser_test.cc
        Line 759, Patchset 35: BrowserWindowInterface* browser_window =
        Qikai Zhong . resolved

        Same here

        Yu He

        ok

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Qikai Zhong
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Owners
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not 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: Iee13b0317b694f976071ae0c0a462ab53a9144f9
        Gerrit-Change-Number: 7684422
        Gerrit-PatchSet: 38
        Gerrit-Owner: Yu He <y...@microsoft.com>
        Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
        Gerrit-Reviewer: Yu He <y...@microsoft.com>
        Gerrit-Attention: Qikai Zhong <qikai...@microsoft.com>
        Gerrit-Comment-Date: Thu, 02 Apr 2026 08:02:06 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Qikai Zhong <qikai...@microsoft.com>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Qikai Zhong (Gerrit)

        unread,
        5:51 AM (9 hours ago) 5:51 AM
        to Yu He, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, lens-chrome...@google.com, stanfie...@google.com, mercer...@google.com, rginda...@chromium.org, aixba+wat...@chromium.org, ayman...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, dibyapal+wa...@chromium.org, dmurph+wat...@chromium.org, dmurph+watc...@chromium.org, extension...@chromium.org, feature-v...@chromium.org, japhet+...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mdjone...@chromium.org, mek+w...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, yuezhang...@chromium.org, zelin+watch-we...@chromium.org
        Attention needed from Yu He

        Qikai Zhong added 1 comment

        Commit Message
        Line 7, Patchset 38 (Latest):[bedrock] Migrate chrome::FindTabbedBrowser() to ProfileBrowserCollection::FindTabbedBrowser()
        Qikai Zhong . unresolved

        The title may have exceeded the length limit.

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Yu He
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Owners
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not 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: Iee13b0317b694f976071ae0c0a462ab53a9144f9
        Gerrit-Change-Number: 7684422
        Gerrit-PatchSet: 38
        Gerrit-Owner: Yu He <y...@microsoft.com>
        Gerrit-Reviewer: Qikai Zhong <qikai...@microsoft.com>
        Gerrit-Reviewer: Yu He <y...@microsoft.com>
        Gerrit-Attention: Yu He <y...@microsoft.com>
        Gerrit-Comment-Date: Thu, 02 Apr 2026 09:50:39 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages