Implement SupportsUserDataHolder for composition. [chromium/src : main]

0 views
Skip to first unread message

Daniel Cheng (Gerrit)

unread,
Jul 3, 2024, 1:02:21 AM (2 days ago) Jul 3
to Daniel Cheng, David Benjamin, danakj, Enterprise Policy Reviews, Kevin McNee, James Maclean, Chromium LUCI CQ, chromium...@chromium.org, dullweb...@chromium.org, ffred...@chromium.org, msrame...@chromium.org, tluk+...@chromium.org, alexmo...@chromium.org, chromium-a...@chromium.org, creis...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, navigation...@chromium.org, rhalava...@chromium.org, steimel+...@chromium.org, andysjl...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, croissant-...@chromium.org, druber...@chromium.org, nwoked...@chromium.org, vakh+safe_br...@chromium.org, xinghui...@chromium.org, zackha...@chromium.org
Attention needed from David Benjamin and danakj

Daniel Cheng added 1 comment

Patchset-level comments
File-level comment, Patchset 11 (Latest):
Daniel Cheng . resolved

WDYT? Terrible idea? Really terrible idea?

I originally implemented this because I wanted to use this for WebContents. However, it turned out to be Hard™, though I kind of feel like this is really the right thing to do.

The tricky part, of course, is there actually *are* things that depend on being generic SupportsUserData::Data and allowing themselves to be attached to, say, BrowserContext or WebContents.

Open in Gerrit

Related details

Attention is currently required from:
  • David Benjamin
  • danakj
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
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: Ida82df4d6813671237e4f280a94bcd391605aaa6
Gerrit-Change-Number: 5659461
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Cheng <dch...@chromium.org>
Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
Gerrit-Reviewer: David Benjamin <davi...@chromium.org>
Gerrit-Reviewer: danakj <dan...@chromium.org>
Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>
Gerrit-CC: James Maclean <wjma...@chromium.org>
Gerrit-CC: Kevin McNee <mc...@chromium.org>
Gerrit-Attention: danakj <dan...@chromium.org>
Gerrit-Attention: David Benjamin <davi...@chromium.org>
Gerrit-Comment-Date: Wed, 03 Jul 2024 05:02:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Daniel Cheng (Gerrit)

unread,
Jul 3, 2024, 11:09:40 AM (2 days ago) Jul 3
to Daniel Cheng, Elias Klim, David Benjamin, danakj, Enterprise Policy Reviews, Kevin McNee, James Maclean, Chromium LUCI CQ, chromium...@chromium.org, dullweb...@chromium.org, ffred...@chromium.org, msrame...@chromium.org, tluk+...@chromium.org, alexmo...@chromium.org, chromium-a...@chromium.org, creis...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, navigation...@chromium.org, rhalava...@chromium.org, steimel+...@chromium.org, andysjl...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, croissant-...@chromium.org, druber...@chromium.org, nwoked...@chromium.org, vakh+safe_br...@chromium.org, xinghui...@chromium.org, zackha...@chromium.org
Attention needed from Daniel Cheng, David Benjamin and danakj

Daniel Cheng removed Elias Klim from this change

Deleted Reviewers:
  • Elias Klim
Open in Gerrit

Related details

Attention is currently required from:
  • Daniel Cheng
  • David Benjamin
  • danakj
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: deleteReviewer
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ida82df4d6813671237e4f280a94bcd391605aaa6
Gerrit-Change-Number: 5659461
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Cheng <dch...@chromium.org>
Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
Gerrit-Reviewer: David Benjamin <davi...@chromium.org>
Gerrit-Reviewer: danakj <dan...@chromium.org>
Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>
Gerrit-CC: James Maclean <wjma...@chromium.org>
Gerrit-CC: Kevin McNee <mc...@chromium.org>
Gerrit-Attention: Daniel Cheng <dch...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Daniel Cheng (Gerrit)

unread,
Jul 3, 2024, 11:44:18 PM (2 days ago) Jul 3
to Daniel Cheng, Kai Ninomiya, David Benjamin, danakj, Enterprise Policy Reviews, Kevin McNee, James Maclean, Chromium LUCI CQ, chromium...@chromium.org, dullweb...@chromium.org, ffred...@chromium.org, msrame...@chromium.org, tluk+...@chromium.org, alexmo...@chromium.org, chromium-a...@chromium.org, creis...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, navigation...@chromium.org, rhalava...@chromium.org, steimel+...@chromium.org, andysjl...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, croissant-...@chromium.org, druber...@chromium.org, nwoked...@chromium.org, vakh+safe_br...@chromium.org, xinghui...@chromium.org, zackha...@chromium.org
Attention needed from Daniel Cheng, David Benjamin and danakj

Daniel Cheng removed Kai Ninomiya from this change

Deleted Reviewers:
  • Kai Ninomiya
Open in Gerrit

Related details

Attention is currently required from:
  • Daniel Cheng
  • David Benjamin
  • danakj
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: deleteReviewer
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ida82df4d6813671237e4f280a94bcd391605aaa6
Gerrit-Change-Number: 5659461
Gerrit-PatchSet: 12
satisfied_requirement
unsatisfied_requirement
open
diffy

danakj (Gerrit)

unread,
Jul 4, 2024, 4:13:50 PM (12 hours ago) Jul 4
to Daniel Cheng, David Benjamin, Enterprise Policy Reviews, Kevin McNee, James Maclean, Chromium LUCI CQ, chromium...@chromium.org, network-ser...@chromium.org, dullweb...@chromium.org, ffred...@chromium.org, msrame...@chromium.org, tluk+...@chromium.org, alexmo...@chromium.org, chromium-a...@chromium.org, creis...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, navigation...@chromium.org, rhalava...@chromium.org, steimel+...@chromium.org, andysjl...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, croissant-...@chromium.org, druber...@chromium.org, nwoked...@chromium.org, vakh+safe_br...@chromium.org, xinghui...@chromium.org, zackha...@chromium.org
Attention needed from Daniel Cheng and David Benjamin

danakj added 4 comments

File base/supports_user_data.h
Line 96, Patchset 13 (Latest):// This is a helper for classes that want to allow users to stash random data by
danakj . unresolved

arbitrary

Line 34, Patchset 13 (Latest):// setting user dataa, et cetera.
danakj . unresolved

data

Line 24, Patchset 13 (Latest):// // Internally, delegates to `user_data_holder_`.
danakj . unresolved

I am having trouble understanding the point of this holder TBH. The comment here isn't explaining what's going on and is different from before. Is this replacing inheritance or something else?

Line 18, Patchset 13 (Latest):// random data by key. The random data is owned by the holder and will be
danakj . unresolved

s/random/arbitrary/g

Open in Gerrit

Related details

Attention is currently required from:
  • Daniel Cheng
  • David Benjamin
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    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: Ida82df4d6813671237e4f280a94bcd391605aaa6
    Gerrit-Change-Number: 5659461
    Gerrit-PatchSet: 13
    Gerrit-Owner: Daniel Cheng <dch...@chromium.org>
    Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
    Gerrit-Reviewer: David Benjamin <davi...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>
    Gerrit-CC: James Maclean <wjma...@chromium.org>
    Gerrit-CC: Kevin McNee <mc...@chromium.org>
    Gerrit-Attention: Daniel Cheng <dch...@chromium.org>
    Gerrit-Attention: David Benjamin <davi...@chromium.org>
    Gerrit-Comment-Date: Thu, 04 Jul 2024 20:13:41 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages