[Extension] Use base::UnsafeDangling for DispatchOnInstalledEvent post [chromium/src : main]

0 views
Skip to first unread message

Sohail Rajdev (Gerrit)

unread,
May 27, 2026, 7:07:36 AM (5 days ago) May 27
to Solomon Kinard, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Solomon Kinard

Sohail Rajdev voted and added 1 comment

Votes added by Sohail Rajdev

Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Sohail Rajdev . resolved

Hey Solomon! Could you please help with a review of this change? 😊

Open in Gerrit

Related details

Attention is currently required from:
  • Solomon Kinard
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: If7518477d06452319c649de3b0109230a86ad3dc
Gerrit-Change-Number: 7875806
Gerrit-PatchSet: 1
Gerrit-Owner: Sohail Rajdev <sora...@microsoft.com>
Gerrit-Reviewer: Sohail Rajdev <sora...@microsoft.com>
Gerrit-Reviewer: Solomon Kinard <solomo...@chromium.org>
Gerrit-Attention: Solomon Kinard <solomo...@chromium.org>
Gerrit-Comment-Date: Wed, 27 May 2026 11:07:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Sohail Rajdev (Gerrit)

unread,
May 29, 2026, 12:38:10 AM (4 days ago) May 29
to Devlin Cronin, Utkarsh Patankar, Solomon Kinard, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Devlin Cronin and Solomon Kinard

Sohail Rajdev added 1 comment

Patchset-level comments
Sohail Rajdev . resolved

Hi Devlin, Solomon seems to be unavailable. Could you please help with this one? 😊

Open in Gerrit

Related details

Attention is currently required from:
  • Devlin Cronin
  • Solomon Kinard
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: If7518477d06452319c649de3b0109230a86ad3dc
Gerrit-Change-Number: 7875806
Gerrit-PatchSet: 1
Gerrit-Owner: Sohail Rajdev <sora...@microsoft.com>
Gerrit-Reviewer: Devlin Cronin <rdevlin...@chromium.org>
Gerrit-Reviewer: Sohail Rajdev <sora...@microsoft.com>
Gerrit-Reviewer: Solomon Kinard <solomo...@chromium.org>
Gerrit-CC: Utkarsh Patankar <utk...@microsoft.com>
Gerrit-Attention: Devlin Cronin <rdevlin...@chromium.org>
Gerrit-Attention: Solomon Kinard <solomo...@chromium.org>
Gerrit-Comment-Date: Fri, 29 May 2026 04:37:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Solomon Kinard (Gerrit)

unread,
May 29, 2026, 1:19:57 PM (3 days ago) May 29
to Sohail Rajdev, Devlin Cronin, Utkarsh Patankar, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Devlin Cronin and Sohail Rajdev

Solomon Kinard added 1 comment

Commit Message
Line 39, Patchset 1 (Latest):Fix by:
Solomon Kinard . unresolved

Is it possible to write a test, or is there already test coverage?

Open in Gerrit

Related details

Attention is currently required from:
  • Devlin Cronin
  • Sohail Rajdev
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: If7518477d06452319c649de3b0109230a86ad3dc
    Gerrit-Change-Number: 7875806
    Gerrit-PatchSet: 1
    Gerrit-Owner: Sohail Rajdev <sora...@microsoft.com>
    Gerrit-Reviewer: Devlin Cronin <rdevlin...@chromium.org>
    Gerrit-Reviewer: Sohail Rajdev <sora...@microsoft.com>
    Gerrit-Reviewer: Solomon Kinard <solomo...@chromium.org>
    Gerrit-CC: Utkarsh Patankar <utk...@microsoft.com>
    Gerrit-Attention: Devlin Cronin <rdevlin...@chromium.org>
    Gerrit-Attention: Sohail Rajdev <sora...@microsoft.com>
    Gerrit-Comment-Date: Fri, 29 May 2026 17:19:46 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Devlin Cronin (Gerrit)

    unread,
    May 29, 2026, 2:22:01 PM (3 days ago) May 29
    to Sohail Rajdev, Devlin Cronin, Utkarsh Patankar, Solomon Kinard, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
    Attention needed from Sohail Rajdev

    Devlin Cronin added 1 comment

    Patchset-level comments
    Sohail Rajdev . resolved

    Hi Devlin, Solomon seems to be unavailable. Could you please help with this one? 😊

    Devlin Cronin

    Solomon seems to be on it; I'll take a pass after him (it can sometimes take us a day or so to get to a CL). Please add me back to the attention set then.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Sohail Rajdev
    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: If7518477d06452319c649de3b0109230a86ad3dc
    Gerrit-Change-Number: 7875806
    Gerrit-PatchSet: 1
    Gerrit-Owner: Sohail Rajdev <sora...@microsoft.com>
    Gerrit-Reviewer: Devlin Cronin <rdevlin...@chromium.org>
    Gerrit-Reviewer: Sohail Rajdev <sora...@microsoft.com>
    Gerrit-Reviewer: Solomon Kinard <solomo...@chromium.org>
    Gerrit-CC: Utkarsh Patankar <utk...@microsoft.com>
    Gerrit-Attention: Sohail Rajdev <sora...@microsoft.com>
    Gerrit-Comment-Date: Fri, 29 May 2026 18:21:43 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Sohail Rajdev <sora...@microsoft.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Sohail Rajdev (Gerrit)

    unread,
    1:07 PM (4 hours ago) 1:07 PM
    to Devlin Cronin, Utkarsh Patankar, Solomon Kinard, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
    Attention needed from Solomon Kinard

    Sohail Rajdev added 1 comment

    Commit Message
    Solomon Kinard . unresolved

    Is it possible to write a test, or is there already test coverage?

    Sohail Rajdev

    Thanks for the suggestion, Solomon! I looked into adding a regression unit test, but I don't think a meaningful one is possible.

    A runtime test would have to call `DispatchOnInstalledEvent` with a `MayBeDangling<void>` argument and then destroy the underlying `BrowserContext` before the task runs. But `MayBeDangling<T>` opts the pointer out of the dangling-ptr check by design, so such a test would never crash. It would always pass without exercising the bug.

    We can't substitute `base::Unretained` to force the check either: the bind machinery rejects it at compile time for `MayBeDangling<void>` parameters, and even if it didn't, it would misrepresent the call site because production code does not pass an unretained pointer here.

    Correctness is guaranteed at compile time by the `MayBeDangling<void>` parameter type: every caller is forced to use `base::UnsafeDangling()`, and a future regression in the signature would fail to compile rather than silently break at runtime.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Solomon Kinard
    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: If7518477d06452319c649de3b0109230a86ad3dc
    Gerrit-Change-Number: 7875806
    Gerrit-PatchSet: 1
    Gerrit-Owner: Sohail Rajdev <sora...@microsoft.com>
    Gerrit-Reviewer: Devlin Cronin <rdevlin...@chromium.org>
    Gerrit-Reviewer: Sohail Rajdev <sora...@microsoft.com>
    Gerrit-Reviewer: Solomon Kinard <solomo...@chromium.org>
    Gerrit-CC: Utkarsh Patankar <utk...@microsoft.com>
    Gerrit-Attention: Solomon Kinard <solomo...@chromium.org>
    Gerrit-Comment-Date: Mon, 01 Jun 2026 17:07:04 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Solomon Kinard <solomo...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages