[projects] Refactor SavedTabGroupBar to use WeakPtr (1/6) [chromium/src : main]

0 views
Skip to first unread message

Gabriella Queen (Gerrit)

unread,
5:47 AM (18 hours ago) 5:47 AM
to Chromium Metrics Reviews, (Julie)Jeongeun Kim, Enterprise Policy Reviews, AyeAye, Matthew Jones, David Maunder, David Pennington, Kunal Daftari, Chromium LUCI CQ, chromium...@chromium.org, extension...@chromium.org, kinuko...@chromium.org, tluk+...@chromium.org, josiah...@chromium.org, jshin...@chromium.org, rrsilva+wat...@google.com, cros-setti...@google.com, print-rev...@chromium.org, dmurph+wat...@chromium.org, jophba...@chromium.org, chromium-a...@chromium.org, net-r...@chromium.org, kyungjunle...@google.com, roblia...@chromium.org, devtools...@chromium.org, network-ser...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, nektar...@chromium.org, oshima...@chromium.org, blink-re...@chromium.org, asvitki...@chromium.org, mfoltz+wa...@chromium.org, omnibox-...@chromium.org, webap...@microsoft.com, abigailbk...@google.com, jmedle...@chromium.org, francisjp...@google.com, tbarzi...@chromium.org, dtseng...@chromium.org, hanxi...@chromium.org, peilinwa...@google.com, ffred...@chromium.org, cros-print...@google.com, dewitt...@chromium.org, blink-rev...@chromium.org, gavin...@chromium.org, yuzo+...@chromium.org, jdonnel...@chromium.org, croissant-...@chromium.org, sky+...@chromium.org, tote-eng...@google.com, mac-r...@chromium.org, estali...@chromium.org, dfried...@chromium.org
Attention needed from David Maunder, David Pennington, Kunal Daftari and Matthew Jones

Gabriella Queen voted and added 1 comment

Votes added by Gabriella Queen

Auto-Submit+1

1 comment

File chrome/browser/ui/views/bookmarks/saved_tab_groups/saved_tab_group_bar.cc
Line 82, Patchset 12 (Parent): DCHECK(tab_group_service);
David Pennington . resolved

lets add this back in, this class doesnt work without tab_group_service_. please also remove all of the if expressions that include tab_group_service_ checks.

Gabriella Queen

Done

Open in Gerrit

Related details

Attention is currently required from:
  • David Maunder
  • David Pennington
  • Kunal Daftari
  • Matthew Jones
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: I1b66c985af7794c3c5892a657b62cd0fc8bb4631
Gerrit-Change-Number: 7633588
Gerrit-PatchSet: 16
Gerrit-Owner: Gabriella Queen <gqu...@chromium.org>
Gerrit-Reviewer: David Maunder <dav...@chromium.org>
Gerrit-Reviewer: David Pennington <dpen...@chromium.org>
Gerrit-Reviewer: Gabriella Queen <gqu...@chromium.org>
Gerrit-Reviewer: Kunal Daftari <kunald...@google.com>
Gerrit-Reviewer: Matthew Jones <mdj...@chromium.org>
Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>
Gerrit-Attention: David Maunder <dav...@chromium.org>
Gerrit-Attention: David Pennington <dpen...@chromium.org>
Gerrit-Attention: Kunal Daftari <kunald...@google.com>
Gerrit-Attention: Matthew Jones <mdj...@chromium.org>
Gerrit-Comment-Date: Fri, 06 Mar 2026 10:47:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: David Pennington <dpen...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Kunal Daftari (Gerrit)

unread,
11:55 AM (12 hours ago) 11:55 AM
to Gabriella Queen, Chromium Metrics Reviews, (Julie)Jeongeun Kim, Enterprise Policy Reviews, AyeAye, Matthew Jones, David Maunder, David Pennington, Chromium LUCI CQ, chromium...@chromium.org, extension...@chromium.org, kinuko...@chromium.org, tluk+...@chromium.org, josiah...@chromium.org, jshin...@chromium.org, rrsilva+wat...@google.com, cros-setti...@google.com, print-rev...@chromium.org, dmurph+wat...@chromium.org, jophba...@chromium.org, chromium-a...@chromium.org, net-r...@chromium.org, kyungjunle...@google.com, roblia...@chromium.org, devtools...@chromium.org, network-ser...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, nektar...@chromium.org, oshima...@chromium.org, blink-re...@chromium.org, asvitki...@chromium.org, mfoltz+wa...@chromium.org, omnibox-...@chromium.org, webap...@microsoft.com, abigailbk...@google.com, jmedle...@chromium.org, francisjp...@google.com, tbarzi...@chromium.org, dtseng...@chromium.org, hanxi...@chromium.org, peilinwa...@google.com, ffred...@chromium.org, cros-print...@google.com, dewitt...@chromium.org, blink-rev...@chromium.org, gavin...@chromium.org, yuzo+...@chromium.org, jdonnel...@chromium.org, croissant-...@chromium.org, sky+...@chromium.org, tote-eng...@google.com, mac-r...@chromium.org, estali...@chromium.org, dfried...@chromium.org
Attention needed from David Maunder, David Pennington, Gabriella Queen and Matthew Jones

Kunal Daftari voted Code-Review+1

Code-Review+1
Open in Gerrit

Related details

Attention is currently required from:
  • David Maunder
  • David Pennington
  • Gabriella Queen
  • Matthew Jones
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not 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: I1b66c985af7794c3c5892a657b62cd0fc8bb4631
    Gerrit-Change-Number: 7633588
    Gerrit-PatchSet: 18
    Gerrit-Owner: Gabriella Queen <gqu...@chromium.org>
    Gerrit-Reviewer: David Maunder <dav...@chromium.org>
    Gerrit-Reviewer: David Pennington <dpen...@chromium.org>
    Gerrit-Reviewer: Gabriella Queen <gqu...@chromium.org>
    Gerrit-Reviewer: Kunal Daftari <kunald...@google.com>
    Gerrit-Reviewer: Matthew Jones <mdj...@chromium.org>
    Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>
    Gerrit-Attention: David Maunder <dav...@chromium.org>
    Gerrit-Attention: David Pennington <dpen...@chromium.org>
    Gerrit-Attention: Gabriella Queen <gqu...@chromium.org>
    Gerrit-Attention: Matthew Jones <mdj...@chromium.org>
    Gerrit-Comment-Date: Fri, 06 Mar 2026 16:55:28 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    David Pennington (Gerrit)

    unread,
    12:09 PM (11 hours ago) 12:09 PM
    to Gabriella Queen, Kunal Daftari, Chromium Metrics Reviews, (Julie)Jeongeun Kim, Enterprise Policy Reviews, AyeAye, Matthew Jones, David Maunder, Chromium LUCI CQ, chromium...@chromium.org, extension...@chromium.org, kinuko...@chromium.org, tluk+...@chromium.org, josiah...@chromium.org, jshin...@chromium.org, rrsilva+wat...@google.com, cros-setti...@google.com, print-rev...@chromium.org, dmurph+wat...@chromium.org, jophba...@chromium.org, chromium-a...@chromium.org, net-r...@chromium.org, kyungjunle...@google.com, roblia...@chromium.org, devtools...@chromium.org, network-ser...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, nektar...@chromium.org, oshima...@chromium.org, blink-re...@chromium.org, asvitki...@chromium.org, mfoltz+wa...@chromium.org, omnibox-...@chromium.org, webap...@microsoft.com, abigailbk...@google.com, jmedle...@chromium.org, francisjp...@google.com, tbarzi...@chromium.org, dtseng...@chromium.org, hanxi...@chromium.org, peilinwa...@google.com, ffred...@chromium.org, cros-print...@google.com, dewitt...@chromium.org, blink-rev...@chromium.org, gavin...@chromium.org, yuzo+...@chromium.org, jdonnel...@chromium.org, croissant-...@chromium.org, sky+...@chromium.org, tote-eng...@google.com, mac-r...@chromium.org, estali...@chromium.org, dfried...@chromium.org
    Attention needed from David Maunder, Gabriella Queen and Matthew Jones

    David Pennington voted and added 3 comments

    Votes added by David Pennington

    Code-Review+1

    3 comments

    Patchset-level comments
    File-level comment, Patchset 19 (Latest):
    David Pennington . resolved

    overall +1 a lot of this is good cleanup!

    File chrome/browser/ui/views/bookmarks/saved_tab_groups/saved_tab_group_bar.cc
    Line 442, Patchset 19 (Latest): }
    David Pennington . unresolved

    nit lets move this back to a CHECK.

    Line 697, Patchset 19 (Latest): if (!browser_ || !browser_->capabilities() ||
    David Pennington . unresolved

    when does this occur where the browser doesnt have the capabilities object. it seems like this extra check was added unrelated to the cl description.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • David Maunder
    • Gabriella Queen
    • Matthew Jones
    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: I1b66c985af7794c3c5892a657b62cd0fc8bb4631
    Gerrit-Change-Number: 7633588
    Gerrit-PatchSet: 19
    Gerrit-Owner: Gabriella Queen <gqu...@chromium.org>
    Gerrit-Reviewer: David Maunder <dav...@chromium.org>
    Gerrit-Reviewer: David Pennington <dpen...@chromium.org>
    Gerrit-Reviewer: Gabriella Queen <gqu...@chromium.org>
    Gerrit-Reviewer: Kunal Daftari <kunald...@google.com>
    Gerrit-Reviewer: Matthew Jones <mdj...@chromium.org>
    Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>
    Gerrit-Attention: David Maunder <dav...@chromium.org>
    Gerrit-Attention: Gabriella Queen <gqu...@chromium.org>
    Gerrit-Attention: Matthew Jones <mdj...@chromium.org>
    Gerrit-Comment-Date: Fri, 06 Mar 2026 17:09:03 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Matthew Jones (Gerrit)

    unread,
    2:30 PM (9 hours ago) 2:30 PM
    to Gabriella Queen, David Pennington, Kunal Daftari, Chromium Metrics Reviews, (Julie)Jeongeun Kim, Enterprise Policy Reviews, AyeAye, David Maunder, Chromium LUCI CQ, chromium...@chromium.org, extension...@chromium.org, kinuko...@chromium.org, tluk+...@chromium.org, josiah...@chromium.org, jshin...@chromium.org, rrsilva+wat...@google.com, cros-setti...@google.com, print-rev...@chromium.org, dmurph+wat...@chromium.org, jophba...@chromium.org, chromium-a...@chromium.org, net-r...@chromium.org, kyungjunle...@google.com, roblia...@chromium.org, devtools...@chromium.org, network-ser...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, nektar...@chromium.org, oshima...@chromium.org, blink-re...@chromium.org, asvitki...@chromium.org, mfoltz+wa...@chromium.org, omnibox-...@chromium.org, webap...@microsoft.com, abigailbk...@google.com, jmedle...@chromium.org, francisjp...@google.com, tbarzi...@chromium.org, dtseng...@chromium.org, hanxi...@chromium.org, peilinwa...@google.com, ffred...@chromium.org, cros-print...@google.com, dewitt...@chromium.org, blink-rev...@chromium.org, gavin...@chromium.org, yuzo+...@chromium.org, jdonnel...@chromium.org, croissant-...@chromium.org, sky+...@chromium.org, tote-eng...@google.com, mac-r...@chromium.org, estali...@chromium.org, dfried...@chromium.org
    Attention needed from David Maunder and Gabriella Queen

    Matthew Jones voted Code-Review+1

    Code-Review+1
    Open in Gerrit

    Related details

    Attention is currently required from:
    • David Maunder
    • Gabriella Queen
    Gerrit-Comment-Date: Fri, 06 Mar 2026 19:30:08 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Gabriella Queen (Gerrit)

    unread,
    3:53 PM (8 hours ago) 3:53 PM
    to Matthew Jones, David Pennington, Kunal Daftari, Chromium Metrics Reviews, (Julie)Jeongeun Kim, Enterprise Policy Reviews, AyeAye, David Maunder, Chromium LUCI CQ, chromium...@chromium.org, extension...@chromium.org, kinuko...@chromium.org, tluk+...@chromium.org, josiah...@chromium.org, jshin...@chromium.org, rrsilva+wat...@google.com, cros-setti...@google.com, print-rev...@chromium.org, dmurph+wat...@chromium.org, jophba...@chromium.org, chromium-a...@chromium.org, net-r...@chromium.org, kyungjunle...@google.com, roblia...@chromium.org, devtools...@chromium.org, network-ser...@chromium.org, blink-...@chromium.org, asvitkine...@chromium.org, nektar...@chromium.org, oshima...@chromium.org, blink-re...@chromium.org, asvitki...@chromium.org, mfoltz+wa...@chromium.org, omnibox-...@chromium.org, webap...@microsoft.com, abigailbk...@google.com, jmedle...@chromium.org, francisjp...@google.com, tbarzi...@chromium.org, dtseng...@chromium.org, hanxi...@chromium.org, peilinwa...@google.com, ffred...@chromium.org, cros-print...@google.com, dewitt...@chromium.org, blink-rev...@chromium.org, gavin...@chromium.org, yuzo+...@chromium.org, jdonnel...@chromium.org, croissant-...@chromium.org, sky+...@chromium.org, tote-eng...@google.com, mac-r...@chromium.org, estali...@chromium.org, dfried...@chromium.org
    Attention needed from David Maunder, David Pennington, Kunal Daftari and Matthew Jones

    Gabriella Queen voted and added 3 comments

    Votes added by Gabriella Queen

    Auto-Submit+1

    3 comments

    Patchset-level comments
    File-level comment, Patchset 20 (Latest):
    Gabriella Queen . resolved

    All done!

    File chrome/browser/ui/views/bookmarks/saved_tab_groups/saved_tab_group_bar.cc
    Line 442, Patchset 19: }
    David Pennington . resolved

    nit lets move this back to a CHECK.

    Gabriella Queen

    Done

    Line 697, Patchset 19: if (!browser_ || !browser_->capabilities() ||
    David Pennington . resolved

    when does this occur where the browser doesnt have the capabilities object. it seems like this extra check was added unrelated to the cl description.

    Gabriella Queen

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • David Maunder
    • David Pennington
    • Kunal Daftari
    • Matthew Jones
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement 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: I1b66c985af7794c3c5892a657b62cd0fc8bb4631
        Gerrit-Change-Number: 7633588
        Gerrit-PatchSet: 20
        Gerrit-Owner: Gabriella Queen <gqu...@chromium.org>
        Gerrit-Reviewer: David Maunder <dav...@chromium.org>
        Gerrit-Reviewer: David Pennington <dpen...@chromium.org>
        Gerrit-Reviewer: Gabriella Queen <gqu...@chromium.org>
        Gerrit-Reviewer: Kunal Daftari <kunald...@google.com>
        Gerrit-Reviewer: Matthew Jones <mdj...@chromium.org>
        Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
        Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
        Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
        Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>
        Gerrit-Attention: David Maunder <dav...@chromium.org>
        Gerrit-Attention: David Pennington <dpen...@chromium.org>
        Gerrit-Attention: Kunal Daftari <kunald...@google.com>
        Gerrit-Attention: Matthew Jones <mdj...@chromium.org>
        Gerrit-Comment-Date: Fri, 06 Mar 2026 20:53:30 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        Comment-In-Reply-To: David Pennington <dpen...@chromium.org>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages