WIP: Remove cookie labels from URLRequestContext [chromium/src : main]

[Avi Drissman (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi, Joshua Hood, Kent Tamura, Kevin Graney, Nan Lin, Russ Hamilton and mmenke

Avi Drissman voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Joshua Hood
• Kent Tamura
• Kevin Graney
• Nan Lin
• Russ Hamilton
• mmenke

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I04ab2c2973befcfcf669550c87269f49023ea80b

Gerrit-Change-Number: 7155846

Gerrit-PatchSet: 9

Gerrit-Owner: Kevin Graney <k...@google.com>

Gerrit-Reviewer: Avi Drissman <a...@chromium.org>

Gerrit-Reviewer: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Reviewer: Joshua Hood <j...@chromium.org>

Gerrit-Reviewer: Kent Tamura <tk...@chromium.org>

Gerrit-Reviewer: Kevin Graney <k...@google.com>

Gerrit-Reviewer: Nan Lin <lin...@chromium.org>

Gerrit-Reviewer: Russ Hamilton <beham...@google.com>

Gerrit-Reviewer: mmenke <mme...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Brian Lefler <b...@google.com>

Gerrit-CC: Chromium IPC Reviews <chrome-ip...@google.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Fiona Macintosh <fmaci...@google.com>

Gerrit-CC: James Maclean <wjma...@chromium.org>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Nate Chapin <jap...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: gwsq

Gerrit-Attention: Russ Hamilton <beham...@google.com>

Gerrit-Attention: mmenke <mme...@chromium.org>

Gerrit-Attention: Kevin Graney <k...@google.com>

Gerrit-Attention: Joshua Hood <j...@chromium.org>

Gerrit-Attention: Nan Lin <lin...@chromium.org>

Gerrit-Attention: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Attention: Kent Tamura <tk...@chromium.org>

Gerrit-Comment-Date: Wed, 19 Nov 2025 16:43:05 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[mmenke (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi, Joshua Hood, Kent Tamura, Kevin Graney, Nan Lin and Russ Hamilton

mmenke added 4 comments

Patchset-level comments

File-level comment, Patchset 9 (Latest):

mmenke . resolved

LGTM

Commit Message

Line 11, Patchset 8:- Disabled in binary by default in M143
- Code removal in M144 (next branch)

mmenke . unresolved

Was there an intent for this / is there a feature in chrome status we can link to?

Kevin Graney

https://chromestatus.com/feature/5189079788683264

More context in these threads:
https://groups.google.com/a/chromium.org/g/blink-dev/c/WnIy9TTDXpk/m/66Xio4YGDAAJ
https://groups.google.com/a/chromium.org/g/blink-dev/c/63u-0KmBG4w/m/nhIsSbLfAwAJ

mmenke

Thanks! Guess this was always considered an experiment and nor a shipped feature. Could you mention that in the CL description?

Line 14, Patchset 8:Bug: 447619585

mmenke . resolved

This is a public, web-exposed feature. Seems like there should be a public bug for this work.

Kevin Graney

The bug has a combination of Finch and public CLs on it. Is it typical to include Finch CLs on public bugs?

mmenke

I'm not really sure what the best practice is, now-a-days. Others may have better suggestions.

Historically, we haven't linked internal CLs monorail (which they didn't support linking to, anyways). We did often have a private launch bug for a feature that dealt with permissions and then a release public bug we linked CLs to. I guess launches don't have bugs anymore, but may still be a useful model. Alternatively, monorail bugs used to not even support linking internal CLs, so could refer to public bugs in private CLs without linking them.

All that having been said, I'm certainly not losing sleep over Finch CLs linking public bugs, but does seem like probably not a great thing to do, though I still think it's better than public CLs linking private bugs.

File net/url_request/url_request_context_builder.cc

Line 627, Patchset 8 (Parent): context->set_cookie_deprecation_label(*cookie_deprecation_label_);

mmenke . unresolved

This CL does not remove URLRequestContext::cookie_deprecation_label(). Is that deliberate? The CL description doesn't mention it at all.

Kevin Graney

No, that was an oversight, thanks. Do you mind if I remove that code as a follow-up? (This CL is becoming quite large and unwieldy.)

mmenke

I'd be fine with that, but looks like you already expanded the CL to remove that code?

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Joshua Hood
• Kent Tamura
• Kevin Graney
• Nan Lin
• Russ Hamilton

Gerrit-Attention: Kevin Graney <k...@google.com>

Gerrit-Attention: Joshua Hood <j...@chromium.org>

Gerrit-Attention: Nan Lin <lin...@chromium.org>

Gerrit-Attention: Giovanni Ortuno Urquidi <ort...@chromium.org>

Gerrit-Attention: Kent Tamura <tk...@chromium.org>

Gerrit-Comment-Date: Wed, 19 Nov 2025 16:54:30 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: mmenke <mme...@chromium.org>

Comment-In-Reply-To: Kevin Graney <k...@google.com>

[mmenke (Gerrit)]

Attention needed from Giovanni Ortuno Urquidi, Joshua Hood, Kent Tamura, Kevin Graney, Nan Lin and Russ Hamilton

mmenke voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

• Giovanni Ortuno Urquidi
• Joshua Hood
• Kent Tamura
• Kevin Graney
• Nan Lin
• Russ Hamilton

Submit Requirements:

• Code-Coverage
• Code-Owners

Gerrit-Comment-Date: Wed, 19 Nov 2025 16:55:04 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes