gwsq (Gerrit)

unread,

Mar 20, 2026, 10:00:36 AM (3 days ago) Mar 20

to Orko Garai, chromium-bedrock-reviews, Tom Lukaszewicz, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, net-r...@chromium.org

Attention needed from Tom Lukaszewicz

Message from gwsq

Reviewer source(s):
tl...@chromium.org is from context(googleclient/chrome/chromium_gwsq/chrome/browser/config.gwsq)

Open in Gerrit

Related details

Attention is currently required from:

Tom Lukaszewicz

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Tom Lukaszewicz (Gerrit)

unread,

Mar 22, 2026, 5:46:34 PM (19 hours ago) Mar 22

to Orko Garai, chromium-bedrock-reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, net-r...@chromium.org

Attention needed from Orko Garai

Tom Lukaszewicz voted and added 5 comments

Votes added by Tom Lukaszewicz

Code-Review

+1

5 comments

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Tom Lukaszewicz . resolved

lgtm % minor nits

Commit Message

Line 15, Patchset 2 (Latest):

Also remove //c/b/tpcd/http_error_observer folder which had two empty
files.

Tom Lukaszewicz . resolved

Appreciate the find!

File chrome/browser/net/BUILD.gn

Line 30, Patchset 2 (Latest):

    sources = [
      "storage_test_utils.cc",
      "storage_test_utils.h",
    ]

Tom Lukaszewicz . unresolved

nit: As below could we also use the `public` / `sources` split?

File chrome/browser/tpcd/metadata/BUILD.gn

Tom Lukaszewicz . unresolved

nit: 2026

Line 6, Patchset 2 (Latest):

  sources = [
    "manager_factory.cc",
    "manager_factory.h",
  ]

Tom Lukaszewicz . unresolved

nit: Could we split this into a `public` section (for the headers) and `sources` for the impl file?

Open in Gerrit

Related details

Attention is currently required from:

Orko Garai

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Orko Garai (Gerrit)

unread,

10:14 AM (3 hours ago) 10:14 AM

to Tom Lukaszewicz, chromium-bedrock-reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, net-r...@chromium.org

Orko Garai added 3 comments

File chrome/browser/net/BUILD.gn

Line 30, Patchset 2:

    sources = [
      "storage_test_utils.cc",
      "storage_test_utils.h",
    ]

Tom Lukaszewicz . resolved

nit: As below could we also use the `public` / `sources` split?

Orko Garai

Done

File chrome/browser/tpcd/metadata/BUILD.gn

Tom Lukaszewicz . resolved

nit: 2026

Orko Garai

Done

Line 6, Patchset 2:

  sources = [
    "manager_factory.cc",
    "manager_factory.h",
  ]

Tom Lukaszewicz . resolved

nit: Could we split this into a `public` section (for the headers) and `sources` for the impl file?

Orko Garai

Done

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

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

Gerrit

satisfied_requirement

open

diffy

Orko Garai (Gerrit)

unread,

10:14 AM (3 hours ago) 10:14 AM

to Tom Lukaszewicz, chromium-bedrock-reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, net-r...@chromium.org

Orko Garai voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

11:17 AM (2 hours ago) 11:17 AM

to Orko Garai, Tom Lukaszewicz, chromium-bedrock-reviews, chromium...@chromium.org, devtools...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, net-r...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

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

```
The name of the file: chrome/browser/net/BUILD.gn
Insertions: 2, Deletions: 4.

@@ -27,10 +27,8 @@
 } else {
   source_set("storage_test_utils") {
     testonly = true
-    sources = [
-      "storage_test_utils.cc",
-      "storage_test_utils.h",
-    ]
+    public = [ "storage_test_utils.h" ]
+    sources = [ "storage_test_utils.cc" ]
 
     public_deps = [ "//base" ]
     deps = [ "//content/test:test_support" ]
```
```
The name of the file: chrome/browser/tpcd/metadata/BUILD.gn
Insertions: 3, Deletions: 5.

@@ -1,12 +1,10 @@
-# Copyright 2024 The Chromium Authors
+# Copyright 2026 The Chromium Authors
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 source_set("metadata") {
-  sources = [
-    "manager_factory.cc",
-    "manager_factory.h",
-  ]
+  public = [ "manager_factory.h" ]
+  sources = [ "manager_factory.cc" ]
 
   public_deps = [
     "//chrome/browser/profiles:profile",
```

Change information

Commit message:

Modularize //c/b/tpcd, //c/b/net/storage_test_utils

Create separate targets for c/b/tpcd/metadata.

Create a separate target for c/b/net/storage_test_utils.{h,cc} which is
needed by //chrome/test:browser_tests and
//chrome/browser/tpcd/metadata:browser_tests.



Also remove //c/b/tpcd/http_error_observer folder which had two empty
files.

Bug: 353332589

Change-Id: Iad930a86bfdedf6844fe9135a0c2335cdf3a08b6

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7685315

Reviewed-by: Tom Lukaszewicz <tl...@chromium.org>

Commit-Queue: Orko Garai <or...@igalia.com>

Cr-Commit-Position: refs/heads/main@{#1603484}

Files:

M chrome/browser/BUILD.gn
M chrome/browser/content_settings/BUILD.gn
M chrome/browser/devtools/BUILD.gn
M chrome/browser/net/BUILD.gn
D chrome/browser/tpcd/http_error_observer/http_error_tab_helper_browsertest.cc
D chrome/browser/tpcd/http_error_observer/js_error_ukm_browsertest.cc
A chrome/browser/tpcd/metadata/BUILD.gn
M chrome/test/BUILD.gn

Change size: M

Delta: 8 files changed, 62 insertions(+), 5 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Tom Lukaszewicz

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Modularize //c/b/tpcd, //c/b/net/storage_test_utils [chromium/src : main]

gwsq (Gerrit)

Message from gwsq

Related details

Tom Lukaszewicz (Gerrit)

Tom Lukaszewicz voted and added 5 comments

Votes added by Tom Lukaszewicz

5 comments

Related details

Orko Garai (Gerrit)

Orko Garai added 3 comments

Related details

Orko Garai (Gerrit)

Orko Garai voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information