[bedrock] Migrate session_service_base.cc away from BrowserListObserver. [chromium/src : main]

0 views
Skip to first unread message

Glenn Hartmann (Gerrit)

unread,
Nov 10, 2025, 3:38:21 PMNov 10
to Darryl James, Chromium LUCI CQ, chromium...@chromium.org, dewitt...@chromium.org, asvitki...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, chromiumme...@microsoft.com, dmurph+watchin...@chromium.org, dmurph+wat...@chromium.org, grt+...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mgiuca...@chromium.org, webap...@microsoft.com, jatapiaro+wat...@google.com, lwinston+watc...@google.com, rginda...@chromium.org, trewin...@google.com
Attention needed from Darryl James

Glenn Hartmann voted Commit-Queue+1

Commit-Queue+1
Open in Gerrit

Related details

Attention is currently required from:
  • Darryl James
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: Ifb47104029697a378ced847b14a884736a6a6964
Gerrit-Change-Number: 7091712
Gerrit-PatchSet: 22
Gerrit-Owner: Glenn Hartmann <hart...@chromium.org>
Gerrit-Reviewer: Darryl James <dlj...@chromium.org>
Gerrit-Reviewer: Glenn Hartmann <hart...@chromium.org>
Gerrit-Attention: Darryl James <dlj...@chromium.org>
Gerrit-Comment-Date: Mon, 10 Nov 2025 20:38:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Darryl James (Gerrit)

unread,
Nov 10, 2025, 4:50:15 PMNov 10
to Glenn Hartmann, Chromium LUCI CQ, chromium...@chromium.org, dewitt...@chromium.org, asvitki...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, chromiumme...@microsoft.com, dmurph+watchin...@chromium.org, dmurph+wat...@chromium.org, grt+...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mgiuca...@chromium.org, webap...@microsoft.com, jatapiaro+wat...@google.com, lwinston+watc...@google.com, rginda...@chromium.org, trewin...@google.com
Attention needed from Glenn Hartmann

Darryl James voted and added 2 comments

Votes added by Darryl James

Code-Review+1

2 comments

Patchset-level comments
File-level comment, Patchset 22 (Latest):
Darryl James . resolved

lgtm % 1 small nit; Thanks!

File chrome/browser/sessions/session_service_base.cc
Line 553, Patchset 22 (Latest): browser->GetBrowserForMigrationOnly()->session_id()));
Darryl James . unresolved

nit: `browser->GetSessionID();`

Open in Gerrit

Related details

Attention is currently required from:
  • Glenn Hartmann
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement 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: Ifb47104029697a378ced847b14a884736a6a6964
Gerrit-Change-Number: 7091712
Gerrit-PatchSet: 22
Gerrit-Owner: Glenn Hartmann <hart...@chromium.org>
Gerrit-Reviewer: Darryl James <dlj...@chromium.org>
Gerrit-Reviewer: Glenn Hartmann <hart...@chromium.org>
Gerrit-Attention: Glenn Hartmann <hart...@chromium.org>
Gerrit-Comment-Date: Mon, 10 Nov 2025 21:50:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Glenn Hartmann (Gerrit)

unread,
Nov 10, 2025, 7:46:32 PMNov 10
to Darryl James, Chromium LUCI CQ, chromium...@chromium.org, dewitt...@chromium.org, asvitki...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, chromiumme...@microsoft.com, dmurph+watchin...@chromium.org, dmurph+wat...@chromium.org, grt+...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mgiuca...@chromium.org, webap...@microsoft.com, jatapiaro+wat...@google.com, lwinston+watc...@google.com, rginda...@chromium.org, trewin...@google.com

Glenn Hartmann added 1 comment

File chrome/browser/sessions/session_service_base.cc
Line 553, Patchset 22: browser->GetBrowserForMigrationOnly()->session_id()));
Darryl James . resolved

nit: `browser->GetSessionID();`

Glenn Hartmann

Done

Open in Gerrit

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: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ifb47104029697a378ced847b14a884736a6a6964
    Gerrit-Change-Number: 7091712
    Gerrit-PatchSet: 23
    Gerrit-Owner: Glenn Hartmann <hart...@chromium.org>
    Gerrit-Reviewer: Darryl James <dlj...@chromium.org>
    Gerrit-Reviewer: Glenn Hartmann <hart...@chromium.org>
    Gerrit-Comment-Date: Tue, 11 Nov 2025 00:46:26 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Darryl James <dlj...@chromium.org>
    satisfied_requirement
    open
    diffy

    Glenn Hartmann (Gerrit)

    unread,
    Nov 10, 2025, 7:46:35 PMNov 10
    to Darryl James, Chromium LUCI CQ, chromium...@chromium.org, dewitt...@chromium.org, asvitki...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, chromiumme...@microsoft.com, dmurph+watchin...@chromium.org, dmurph+wat...@chromium.org, grt+...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mgiuca...@chromium.org, webap...@microsoft.com, jatapiaro+wat...@google.com, lwinston+watc...@google.com, rginda...@chromium.org, trewin...@google.com

    Glenn Hartmann voted Commit-Queue+2

    Commit-Queue+2
    Open in Gerrit

    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: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ifb47104029697a378ced847b14a884736a6a6964
    Gerrit-Change-Number: 7091712
    Gerrit-PatchSet: 23
    Gerrit-Owner: Glenn Hartmann <hart...@chromium.org>
    Gerrit-Reviewer: Darryl James <dlj...@chromium.org>
    Gerrit-Reviewer: Glenn Hartmann <hart...@chromium.org>
    Gerrit-Comment-Date: Tue, 11 Nov 2025 00:46:30 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    open
    diffy

    Chromium LUCI CQ (Gerrit)

    unread,
    Nov 10, 2025, 8:22:49 PMNov 10
    to Glenn Hartmann, Darryl James, chromium...@chromium.org, dewitt...@chromium.org, asvitki...@chromium.org, chrome-gr...@chromium.org, chromium-a...@chromium.org, chromiumme...@microsoft.com, dmurph+watchin...@chromium.org, dmurph+wat...@chromium.org, grt+...@chromium.org, loyso...@chromium.org, mac-r...@chromium.org, mgiuca...@chromium.org, webap...@microsoft.com, jatapiaro+wat...@google.com, lwinston+watc...@google.com, rginda...@chromium.org, trewin...@google.com

    Chromium LUCI CQ submitted the change with unreviewed changes

    Unreviewed changes

    22 is the latest approved patch-set.
    The change was submitted with unreviewed changes in the following files:

    ```
    The name of the file: chrome/browser/sessions/session_service_base.cc
    Insertions: 2, Deletions: 2.

    @@ -549,8 +549,8 @@
     
     void SessionServiceBase::OnBrowserActivated(BrowserWindowInterface* browser) {
       if (ShouldTrackBrowser(browser)) {
    -    ScheduleCommand(sessions::CreateSetActiveWindowCommand(
    -        browser->GetBrowserForMigrationOnly()->session_id()));
    +    ScheduleCommand(
    +        sessions::CreateSetActiveWindowCommand(browser->GetSessionID()));
       }
     }
     
    ```

    Change information

    Commit message:
    [bedrock] Migrate session_service_base.cc away from BrowserListObserver.

    Note that this changes the existing observer from listening to all
    browsers globally to just listening to browsers within the relevant
    Profile.

    This migration is part of project bedrock to reduce the dependencies on
    Browser and BrowserList. See https://crbug.com/431671320 for more info.
    Bug: 459505408
    Change-Id: Ifb47104029697a378ced847b14a884736a6a6964
    Commit-Queue: Glenn Hartmann <hart...@chromium.org>
    Reviewed-by: Darryl James <dlj...@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#1542884}
    Files:
    • M chrome/browser/sessions/app_session_service.cc
    • M chrome/browser/sessions/session_service_base.cc
    • M chrome/browser/sessions/session_service_base.h
    Change size: M
    Delta: 3 files changed, 29 insertions(+), 26 deletions(-)
    Branch: refs/heads/main
    Submit Requirements:
    • requirement satisfiedCode-Review: +1 by Darryl James
    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: Ifb47104029697a378ced847b14a884736a6a6964
    Gerrit-Change-Number: 7091712
    Gerrit-PatchSet: 24
    Gerrit-Owner: Glenn Hartmann <hart...@chromium.org>
    Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
    Gerrit-Reviewer: Darryl James <dlj...@chromium.org>
    Gerrit-Reviewer: Glenn Hartmann <hart...@chromium.org>
    open
    diffy
    satisfied_requirement
    Reply all
    Reply to author
    Forward
    0 new messages