Hubert Chao (Gerrit)

unread,

Sep 3, 2025, 12:03:20 PM (5 days ago) Sep 3

to Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Chris Thompson

Hubert Chao voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Chris Thompson

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Chris Thompson (Gerrit)

unread,

Sep 3, 2025, 1:25:30 PM (5 days ago) Sep 3

to Hubert Chao, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Hubert Chao

Chris Thompson added 3 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Chris Thompson . resolved

Thanks for adding this! A couple comments in-line.

File chrome/browser/chrome_web_platform_security_metrics_browsertest.cc

Line 465, Patchset 4 (Latest):

  CheckCounter(WebFeature::kLocalNetworkAccessWebSocketResourceNotKnownPrivate,
               0);

Chris Thompson . unresolved

Can we add a test that covers this use counter being emitted?

File content/browser/storage_partition_impl.cc

Line 2531, Patchset 4 (Latest):

    if (!network::IsUrlPotentiallyTrustworthy(request_url) &&
        !request_url.HostIsIPAddress() && !request_url.DomainIs("local")) {

Chris Thompson . unresolved

Would it be possible to include the IsLessPublicAddressSpace() check to restrict this to only cases that would count as LNA, to make this a slightly tighter upper bound? (i.e., not counting local->local)

Open in Gerrit

Related details

Attention is currently required from:

Hubert Chao

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hubert Chao (Gerrit)

unread,

Sep 3, 2025, 2:24:52 PM (5 days ago) Sep 3

to Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Chris Thompson

Hubert Chao added 2 comments

File chrome/browser/chrome_web_platform_security_metrics_browsertest.cc

Line 465, Patchset 4:

  CheckCounter(WebFeature::kLocalNetworkAccessWebSocketResourceNotKnownPrivate,
               0);

Chris Thompson . resolved

Can we add a test that covers this use counter being emitted?

Hubert Chao

oops, thought I had. done.

File content/browser/storage_partition_impl.cc

Line 2531, Patchset 4: if (!network::IsUrlPotentiallyTrustworthy(request_url) &&


        !request_url.HostIsIPAddress() && !request_url.DomainIs("local")) {

Chris Thompson . resolved

Would it be possible to include the IsLessPublicAddressSpace() check to restrict this to only cases that would count as LNA, to make this a slightly tighter upper bound? (i.e., not counting local->local)

Hubert Chao

The `IsLessPublicAddressSpace check` is already done above (line 2521), as part of the check for the `blink::mojom::WebFeature::kPrivateNetworkAccessWebSocketConnected` counter incrementing

Open in Gerrit

Related details

Attention is currently required from:

Chris Thompson

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Chris Thompson (Gerrit)

unread,

Sep 3, 2025, 2:32:06 PM (5 days ago) Sep 3

to Hubert Chao, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Hubert Chao

Chris Thompson voted and added 2 comments

Votes added by Chris Thompson

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Chris Thompson . resolved

LGTM!

File content/browser/storage_partition_impl.cc

Line 2531, Patchset 4:

    if (!network::IsUrlPotentiallyTrustworthy(request_url) &&
        !request_url.HostIsIPAddress() && !request_url.DomainIs("local")) {

Chris Thompson . resolved

Would it be possible to include the IsLessPublicAddressSpace() check to restrict this to only cases that would count as LNA, to make this a slightly tighter upper bound? (i.e., not counting local->local)

Hubert Chao

The `IsLessPublicAddressSpace check` is already done above (line 2521), as part of the check for the `blink::mojom::WebFeature::kPrivateNetworkAccessWebSocketConnected` counter incrementing

Chris Thompson

Ah totally missed that sorry!

Open in Gerrit

Related details

Attention is currently required from:

Hubert Chao

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hubert Chao (Gerrit)

unread,

Sep 3, 2025, 2:54:22 PM (5 days ago) Sep 3

to Joe Downing, Timothy Loh, Adam Rice, Camille Lamy, Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Adam Rice, Camille Lamy, Joe Downing and Timothy Loh

Hubert Chao added 1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Hubert Chao . resolved

ricea@: services/network/*
clamy@: content/browser/*, url_loader_network_service_observer.mojom, chrome/browser/chrome_web_platform_security_metrics_browsertest.cc
joedow@: remoting/base/*
timloh@ chrome/browser/ash/bruschetta/*

Open in Gerrit

Related details

Attention is currently required from:

Adam Rice
Camille Lamy
Joe Downing
Timothy Loh

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Joe Downing (Gerrit)

unread,

Sep 3, 2025, 2:55:21 PM (5 days ago) Sep 3

to Hubert Chao, Timothy Loh, Adam Rice, Camille Lamy, Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Adam Rice, Camille Lamy, Hubert Chao and Timothy Loh

Joe Downing voted and added 1 comment

Votes added by Joe Downing

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Joe Downing . resolved

remoting lgtm

Open in Gerrit

Related details

Attention is currently required from:

Adam Rice
Camille Lamy
Hubert Chao
Timothy Loh

satisfied_requirement

unsatisfied_requirement

open

diffy

Timothy Loh (Gerrit)

unread,

Sep 4, 2025, 1:48:00 AM (4 days ago) Sep 4

to Hubert Chao, Joe Downing, Adam Rice, Camille Lamy, Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Adam Rice, Camille Lamy and Hubert Chao

Timothy Loh voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Adam Rice
Camille Lamy
Hubert Chao

satisfied_requirement

unsatisfied_requirement

open

diffy

Camille Lamy (Gerrit)

unread,

Sep 4, 2025, 9:31:54 AM (4 days ago) Sep 4

to Hubert Chao, Timothy Loh, Joe Downing, Adam Rice, Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Adam Rice and Hubert Chao

Camille Lamy voted and added 2 comments

Votes added by Camille Lamy

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Camille Lamy . resolved

lgtm

File content/browser/storage_partition_impl.cc

Line 2529, Patchset 6 (Latest): // a mixed content bypass situation. This is similiar to the check below in

Camille Lamy . unresolved

Please fix this WARNING reported by Spellchecker: "similiar" is a possible misspelling of "similar".

To bypass Spellchecker, add ...

"similiar" is a possible misspelling of "similar".

To bypass Spellchecker, add a footer with DISABLE_SPELLCHECKER

Open in Gerrit

Related details

Attention is currently required from:

Adam Rice
Hubert Chao

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Adam Rice (Gerrit)

unread,

Sep 4, 2025, 10:17:07 AM (4 days ago) Sep 4

to Hubert Chao, Camille Lamy, Timothy Loh, Joe Downing, Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Attention needed from Hubert Chao

Adam Rice voted and added 1 comment

Votes added by Adam Rice

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Adam Rice . resolved

lgtm

Open in Gerrit

Related details

Attention is currently required from:

Hubert Chao

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
No-Unresolved-Comments

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hubert Chao (Gerrit)

unread,

Sep 4, 2025, 1:10:55 PM (4 days ago) Sep 4

to Adam Rice, Camille Lamy, Timothy Loh, Joe Downing, Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Hubert Chao added 1 comment

File content/browser/storage_partition_impl.cc

Line 2529, Patchset 6: // a mixed content bypass situation. This is similiar to the check below in

Camille Lamy . resolved

Please fix this WARNING reported by Spellchecker: "similiar" is a possible misspelling of "similar".
To bypass Spellchecker, add ...
"similiar" is a possible misspelling of "similar".
To bypass Spellchecker, add a footer with DISABLE_SPELLCHECKER

Hubert Chao

Done

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Hubert Chao (Gerrit)

unread,

Sep 4, 2025, 1:10:59 PM (4 days ago) Sep 4

to Adam Rice, Camille Lamy, Timothy Loh, Joe Downing, Chris Thompson, AyeAye, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Hubert Chao voted Commit-Queue+2

Commit-Queue

+2

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Sep 4, 2025, 2:14:01 PM (4 days ago) Sep 4

to Hubert Chao, Adam Rice, Camille Lamy, Timothy Loh, Joe Downing, Chris Thompson, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, chromotin...@chromium.org, James Maclean, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, network-ser...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

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

```
The name of the file: tools/metrics/histograms/metadata/blink/enums.xml
Insertions: 2, Deletions: 1.

The diff is too large to show. Please review the diff.
```
```
The name of the file: content/browser/storage_partition_impl.cc
Insertions: 1, Deletions: 1.

The diff is too large to show. Please review the diff.
```
```
The name of the file: third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
Insertions: 2, Deletions: 1.

The diff is too large to show. Please review the diff.
```

Change information

Commit message:

[LNA] add use counter for websocket mixed-content issues

Add a use counter that will measure possible breakages where a site
attempts to connect to a ws:// address but we cannot infer from the URL
that this is possibly a LNA request (which means that the request will
be blocked by mixed content checks).

This will allow us to have numbers to see if we need to introduce a
targetAddressSpace like option (which we have in fetch()).

This is similar to crrev.com/c/6259197 which added a similar for LNA
subresource/subframe loading.

Bug: 421156866

Change-Id: Ic876e9cccbbb9166fe4d3e5b088c8f936b87b35a

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

Reviewed-by: Camille Lamy <cl...@chromium.org>

Reviewed-by: Chris Thompson <cth...@chromium.org>

Reviewed-by: Timothy Loh <tim...@chromium.org>

Reviewed-by: Adam Rice <ri...@chromium.org>

Commit-Queue: Hubert Chao <hc...@chromium.org>

Reviewed-by: Joe Downing <joe...@chromium.org>

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

Files:

M chrome/browser/ash/bruschetta/bruschetta_network_context.cc
M chrome/browser/ash/bruschetta/bruschetta_network_context.h
M chrome/browser/chrome_web_platform_security_metrics_browsertest.cc
M content/browser/network_service_client.cc
M content/browser/network_service_client.h
M content/browser/storage_partition_impl.cc
M content/browser/storage_partition_impl.h
M remoting/base/url_loader_network_service_observer.cc
M remoting/base/url_loader_network_service_observer.h
M services/network/public/mojom/url_loader_network_service_observer.mojom
M services/network/test/test_url_loader_network_observer.cc
M services/network/test/test_url_loader_network_observer.h
M services/network/websocket.cc
M third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
M tools/metrics/histograms/metadata/blink/enums.xml

Change size: M

Delta: 15 files changed, 55 insertions(+), 2 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Timothy Loh, +1 by Joe Downing, +1 by Chris Thompson, +1 by Camille Lamy, +1 by Adam Rice

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

[LNA] add use counter for websocket mixed-content issues [chromium/src : main]

Hubert Chao (Gerrit)

Hubert Chao voted Commit-Queue+1

Related details

Chris Thompson (Gerrit)

Chris Thompson added 3 comments

Related details

Hubert Chao (Gerrit)

Hubert Chao added 2 comments

Related details

Chris Thompson (Gerrit)

Chris Thompson voted and added 2 comments

Votes added by Chris Thompson

2 comments

Related details

Hubert Chao (Gerrit)

Hubert Chao added 1 comment

Related details

Joe Downing (Gerrit)

Joe Downing voted and added 1 comment

Votes added by Joe Downing

1 comment

Related details

Timothy Loh (Gerrit)

Timothy Loh voted Code-Review+1

Related details

Camille Lamy (Gerrit)

Camille Lamy voted and added 2 comments

Votes added by Camille Lamy

2 comments

Related details

Adam Rice (Gerrit)

Adam Rice voted and added 1 comment

Votes added by Adam Rice

1 comment

Related details

Hubert Chao (Gerrit)

Hubert Chao added 1 comment

Related details

Hubert Chao (Gerrit)

Hubert Chao voted Commit-Queue+2

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information