Greg Thompson (Gerrit)

unread,

Jul 25, 2024, 11:04:39 AM7/25/24

to Francois Pierre Doray, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Francois Pierre Doray

Greg Thompson added 2 comments

Patchset-level comments

File-level comment, Patchset 15 (Latest):

Greg Thompson . resolved

FYI: i'm starting with the IOCP CL. once we have that one in pretty good shape, i'll look more closely at this one (in case changes in the other ripple up into this one).

File net/socket/tcp_socket_win.h

Line 292, Patchset 15 (Latest): CoreImpl* core_impl();

Greg Thompson . unresolved

all callers of this seem to assume that it's only used after the core is created. can we make it return a reference to the impl rather than a pointer so that there's no mystery about it being null vs non-null?

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

Submit Requirements:

Code-Coverage
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

Greg Thompson (Gerrit)

unread,

Aug 14, 2024, 9:24:42 AM8/14/24

to Francois Pierre Doray, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Francois Pierre Doray

Greg Thompson added 5 comments

Commit Message

Line 11, Patchset 18 (Latest):

signaled event [1], which comes with OS thread scheduling and task queuing overhead. We want to test the hypothesis that using an

Greg Thompson . unresolved

nit: wrap at 72 cols

File net/socket/tcp_socket_posix.h

Line 50, Patchset 18 (Latest): // These constructors must be invoked via Create() (enforced via PassKey).

Greg Thompson . unresolved

can we instead make them `private` and use `base::WrapUnique(new ...)` instead of `std::make_unique` in `Create()`?

File net/socket/tcp_socket_win.h

Line 166, Patchset 18 (Latest): Core();

Greg Thompson . unresolved

nit: move this into the `protected` block since this is a pure-virtual interface -- only subclasses can run the ctor

File net/socket/tcp_socket_win.cc

Line 1126, Patchset 18 (Latest): : TCPSocketWin(std::move(socket_performance_watcher), net_log_source) {}

Greg Thompson . unresolved

`std::move` this? maybe it doesn't matter, but what the heck

Line 1132, Patchset 18 (Latest): return *static_cast<CoreImpl*>(core_.get());

Greg Thompson . unresolved

same comment as https://chromium-review.googlesource.com/c/chromium/src/+/5627052/comment/27bef2b1_64db160b/

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

Submit Requirements:

Code-Coverage
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

Francois Pierre Doray (Gerrit)

unread,

Aug 20, 2024, 4:49:21 PM8/20/24

to Greg Thompson, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Greg Thompson

Francois Pierre Doray added 7 comments

Patchset-level comments

File-level comment, Patchset 24 (Latest):

Francois Pierre Doray . resolved

Please take another look. Thanks.

Commit Message

Line 11, Patchset 18:

signaled event [1], which comes with OS thread scheduling and task queuing overhead. We want to test the hypothesis that using an

Greg Thompson . resolved

nit: wrap at 72 cols

Francois Pierre Doray

Done

File net/socket/tcp_socket_posix.h

Line 50, Patchset 18: // These constructors must be invoked via Create() (enforced via PassKey).

Greg Thompson . resolved

can we instead make them `private` and use `base::WrapUnique(new ...)` instead of `std::make_unique` in `Create()`?

Francois Pierre Doray

Yes, done.

File net/socket/tcp_socket_win.h

Line 166, Patchset 18: Core();

Greg Thompson . resolved

nit: move this into the `protected` block since this is a pure-virtual interface -- only subclasses can run the ctor

Francois Pierre Doray

Done

Line 292, Patchset 15: CoreImpl* core_impl();

Greg Thompson . resolved

all callers of this seem to assume that it's only used after the core is created. can we make it return a reference to the impl rather than a pointer so that there's no mystery about it being null vs non-null?

Francois Pierre Doray

Done

File net/socket/tcp_socket_win.cc

Line 1126, Patchset 18: : TCPSocketWin(std::move(socket_performance_watcher), net_log_source) {}

Greg Thompson . resolved

`std::move` this? maybe it doesn't matter, but what the heck

Francois Pierre Doray

Done

Line 1132, Patchset 18: return *static_cast<CoreImpl*>(core_.get());

Greg Thompson . resolved

same comment as https://chromium-review.googlesource.com/c/chromium/src/+/5627052/comment/27bef2b1_64db160b/

Francois Pierre Doray

Switched to CHECK_DEREF but kept the implementation in the .cc file, because the .h file doesn't know that CoreImpl is related to Core and therefore doesn't allow the cast (error: static_cast from 'net::TCPSocketWin::Core *' to 'CoreImpl *', which are not related by inheritance, is not allowed).

Open in Gerrit

Related details

Attention is currently required from:

Greg Thompson

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Greg Thompson (Gerrit)

unread,

Aug 21, 2024, 3:44:38 AM8/21/24

to Francois Pierre Doray, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Francois Pierre Doray

Greg Thompson voted and added 7 comments

Votes added by Greg Thompson

Code-Review

+1

7 comments

File net/socket/tcp_socket_posix.h

Line 182, Patchset 24 (Latest): // These constructors must be invoked via Create() (enforced via PassKey).

Greg Thompson . unresolved

maybe this comment isn't needed anymore now that the ctors are private? if you think it's still useful, please remove mention of PassKey.

File net/socket/tcp_socket_win.h

Line 52, Patchset 24 (Latest): ~TCPSocketWin() override;

Greg Thompson . unresolved

please put a scary comment here stating that subclasses must call `Close()` in their dtors. (see the comment on `base::Thread::~Thread` for something similar.)

File net/socket/tcp_socket_win.cc

Line 255, Patchset 24 (Latest): if (!!core_->socket_->connect_callback_) {

Greg Thompson . unresolved

`connect_callback_` has a well-defined `operator bool`, so there is no need/reason for `!!`. please remove -- "if the socket holds a connect callback" is more readable than "if the socket doesn't not hold a connect callback."

Line 486, Patchset 24 (Latest): if (socket_ == INVALID_SOCKET || !!connect_callback_) {

Greg Thompson . unresolved

here, too

Line 509, Patchset 24 (Latest): if (socket_ == INVALID_SOCKET || !!connect_callback_) {

Greg Thompson . unresolved

and here

Line 989, Patchset 24 (Latest): CHECK(!!connect_callback_);

Greg Thompson . unresolved

and here

Line 1133, Patchset 24 (Latest): CHECK(!read_callback_ || !!read_if_ready_callback_);

Greg Thompson . unresolved

and these two

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

Submit Requirements:

Code-Coverage
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

Francois Pierre Doray (Gerrit)

unread,

Aug 22, 2024, 1:00:23 PM8/22/24

to Greg Thompson, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Francois Pierre Doray added 7 comments

File net/socket/tcp_socket_posix.h

Line 182, Patchset 24: // These constructors must be invoked via Create() (enforced via PassKey).

Greg Thompson . resolved

maybe this comment isn't needed anymore now that the ctors are private? if you think it's still useful, please remove mention of PassKey.

Francois Pierre Doray

Done

File net/socket/tcp_socket_win.h

Line 52, Patchset 24: ~TCPSocketWin() override;

Greg Thompson . resolved

please put a scary comment here stating that subclasses must call `Close()` in their dtors. (see the comment on `base::Thread::~Thread` for something similar.)

Francois Pierre Doray

Done

File net/socket/tcp_socket_win.cc

Line 255, Patchset 24: if (!!core_->socket_->connect_callback_) {

Greg Thompson . resolved

`connect_callback_` has a well-defined `operator bool`, so there is no need/reason for `!!`. please remove -- "if the socket holds a connect callback" is more readable than "if the socket doesn't not hold a connect callback."

Francois Pierre Doray

Done

Line 486, Patchset 24: if (socket_ == INVALID_SOCKET || !!connect_callback_) {

Greg Thompson . resolved

here, too

Francois Pierre Doray

Done

Line 509, Patchset 24: if (socket_ == INVALID_SOCKET || !!connect_callback_) {

Greg Thompson . resolved

and here

Francois Pierre Doray

Done

Line 989, Patchset 24: CHECK(!!connect_callback_);

Greg Thompson . resolved

and here

Francois Pierre Doray

Done

Line 1133, Patchset 24: CHECK(!read_callback_ || !!read_if_ready_callback_);

Greg Thompson . resolved

and these two

Francois Pierre Doray

Done

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Greg Thompson (Gerrit)

unread,

Aug 23, 2024, 3:41:49 AM8/23/24

to Francois Pierre Doray, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Francois Pierre Doray

Greg Thompson voted and added 2 comments

Votes added by Greg Thompson

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 27 (Latest):

Greg Thompson . resolved

still lgtm.

File net/socket/tcp_socket_win.cc

Line 1145, Patchset 27 (Latest): return !!read_if_ready_callback_;

Greg Thompson . unresolved

could you get rid of this one, too? :-)

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

Submit Requirements:

Code-Coverage
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

Francois Pierre Doray (Gerrit)

unread,

Aug 28, 2024, 3:26:09 PM8/28/24

to Adam Rice, Greg Thompson, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Adam Rice

Francois Pierre Doray added 1 comment

Patchset-level comments

File-level comment, Patchset 27 (Latest):

Francois Pierre Doray . resolved

ricea@: Please take a look.
See context at: https://groups.google.com/a/chromium.org/g/net-dev/c/AT79ASeYEpI
Thanks!

Open in Gerrit

Related details

Attention is currently required from:

Adam Rice

Submit Requirements:

Code-Coverage
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

Francois Pierre Doray (Gerrit)

unread,

Sep 4, 2024, 1:30:02 PM9/4/24

to Adam Rice, Greg Thompson, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Adam Rice

Francois Pierre Doray added 1 comment

File net/socket/tcp_socket_win.cc

Line 1145, Patchset 27: return !!read_if_ready_callback_;

Greg Thompson . resolved

could you get rid of this one, too? :-)

Francois Pierre Doray

This does not compile:

```
bool HasCallback() {
  base::OnceClosure closure;
  return closure;
}
```

```
error: no viable conversion from returned value of type 'base::OnceClosure' (aka 'OnceCallback<void ()>') to function return type 'bool'
   52 |   return closure;
      |          ^~~~~~~
../../base/functional/callback.h:111:12: note: explicit conversion function is not a candidate
  111 |   explicit operator bool() const { return !!holder_; }
```

But this does:

```
bool HasCallback() {
  base::OnceClosure closure;
  return !closure.is_null();
}
```

So I switched to the latter.

Open in Gerrit

Related details

Attention is currently required from:

Adam Rice

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Greg Thompson (Gerrit)

unread,

Sep 5, 2024, 3:16:43 AM9/5/24

to Francois Pierre Doray, Adam Rice, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Adam Rice and Francois Pierre Doray

Greg Thompson voted and added 1 comment

Votes added by Greg Thompson

Code-Review

+1

1 comment

File net/socket/tcp_socket_win.cc

Line 1145, Patchset 27: return !!read_if_ready_callback_;

Greg Thompson . resolved

could you get rid of this one, too? :-)

Francois Pierre Doray

This does not compile:

```
bool HasCallback() {
  base::OnceClosure closure;
  return closure;
}
```

```
error: no viable conversion from returned value of type 'base::OnceClosure' (aka 'OnceCallback<void ()>') to function return type 'bool'
   52 |   return closure;
      |          ^~~~~~~
../../base/functional/callback.h:111:12: note: explicit conversion function is not a candidate
  111 |   explicit operator bool() const { return !!holder_; }
```

But this does:

```
bool HasCallback() {
  base::OnceClosure closure;
  return !closure.is_null();
}
```

So I switched to the latter.

Greg Thompson

oh, yeah, it's an `explicit` bool operator. another option is `bool(read_if_ready_callback_)`, but i find what you've written here eminently readable. :-)

Open in Gerrit

Related details

Attention is currently required from:

Adam Rice
Francois Pierre Doray

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Adam Rice (Gerrit)

unread,

Sep 6, 2024, 10:41:00 AM9/6/24

to Francois Pierre Doray, Greg Thompson, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Francois Pierre Doray

Adam Rice voted and added 2 comments

Votes added by Adam Rice

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 28 (Latest):

Adam Rice . resolved

lgtm. Very nice. Sorry for the delay.

File net/socket/tcp_socket_win.h

Line 255, Patchset 28 (Latest):class NET_EXPORT TCPSocketDefaultWin : public TCPSocketWin {

Adam Rice . unresolved

Can't this go in the .cc file?

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

Submit Requirements:

Code-Coverage
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

Francois Pierre Doray (Gerrit)

unread,

Sep 6, 2024, 12:19:34 PM9/6/24

to Matt Reynolds, Adam Rice, Greg Thompson, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Matt Reynolds

Francois Pierre Doray voted and added 3 comments

Votes added by Francois Pierre Doray

Auto-Submit

+1

3 comments

Patchset-level comments

File-level comment, Patchset 28:

Adam Rice . resolved

lgtm. Very nice. Sorry for the delay.

Francois Pierre Doray

Thanks!

File-level comment, Patchset 29 (Latest):

Francois Pierre Doray . resolved

mattreynolds@: Please take a look at //device/bluetooth/.
Thanks!

File net/socket/tcp_socket_win.h

Line 255, Patchset 28:class NET_EXPORT TCPSocketDefaultWin : public TCPSocketWin {

Adam Rice . resolved

Can't this go in the .cc file?

Francois Pierre Doray

Yes it can. Done.

Open in Gerrit

Related details

Attention is currently required from:

Matt Reynolds

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Matt Reynolds (Gerrit)

unread,

Sep 6, 2024, 2:18:00 PM9/6/24

to Francois Pierre Doray, Adam Rice, Greg Thompson, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Attention needed from Francois Pierre Doray

Matt Reynolds voted and added 1 comment

Votes added by Matt Reynolds

Code-Review	+1
Commit-Queue	+2

1 comment

Patchset-level comments

File-level comment, Patchset 29 (Latest):

Matt Reynolds . resolved

lgtm

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Sep 6, 2024, 2:21:37 PM9/6/24

to Francois Pierre Doray, Matt Reynolds, Adam Rice, Greg Thompson, AyeAye, Tricium, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

[net] Refactor TCPSocketWin to allow alternative read/write impls.

TCPSocketWin uses base::win::ObjectWatcher to watch for completed
reads and writes. Under the hood, this incurs one PostTask per


signaled event [1], which comes with OS thread scheduling and task
queuing overhead. We want to test the hypothesis that using an

IO completion port to be notified of completed reads and writes
without going through PostTask improves page load time.

Steps:

1. [this CL] Make the methods of TCPSocketWin which implement
   the read/write operations pure virtual and implement them in
   TCPSocketDefaultWin. Introduce TCPSocketWin::Create which
   instantiates a TCPSocketDefaultWin. No behavior change in this CL.

2. [crrev.com/c/5627052] Introduce TcpSocketIoCompletionPortWin which
   uses an IO completion port to be notified of completed reads and
   writes, instead of an event watched by base::win::ObjectWatcher.
   Make TCPSocketWin::Create instantiate that class instead of
   TCPSocketDefaultWin when the new "cpSocketIoCompletionPortWin"
   feature is enabled.

[1] https://source.chromium.org/chromium/chromium/src/+/main:base/win/object_watcher.cc;l=87-92;drc=82d96b9965565487874b66b939ca2527a9b2783d

Bug: 40287434

Change-Id: I9d3a60cb3877138b89da03a844da413c6ec4d6e3

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

Reviewed-by: Greg Thompson <g...@chromium.org>

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

Auto-Submit: Francois Pierre Doray <fdo...@chromium.org>

Reviewed-by: Matt Reynolds <mattre...@chromium.org>

Commit-Queue: Matt Reynolds <mattre...@chromium.org>

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

Files:

M chrome/services/sharing/nearby/platform/wifi_direct_medium.cc
M content/browser/network/transferable_socket_browsertest.cc
M content/public/test/network_service_test_helper.cc
M device/bluetooth/bluetooth_socket_net.cc
M device/bluetooth/bluetooth_socket_win.cc
M net/android/network_library_unittest.cc
M net/socket/tcp_client_socket.cc
M net/socket/tcp_server_socket.cc
M net/socket/tcp_socket_posix.cc
M net/socket/tcp_socket_posix.h
M net/socket/tcp_socket_unittest.cc
M net/socket/tcp_socket_win.cc
M net/socket/tcp_socket_win.h
M services/network/brokered_tcp_client_socket.cc
M services/network/public/cpp/transferable_socket_unittest.cc
M services/network/tcp_bound_socket.cc

Change size: L

Delta: 16 files changed, 438 insertions(+), 242 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Greg Thompson, +1 by Adam Rice, +1 by Matt Reynolds

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Zijie He (Gerrit)

unread,

Nov 8, 2024, 2:23:21 PM11/8/24

to Francois Pierre Doray, Chromium LUCI CQ, Matt Reynolds, Adam Rice, Greg Thompson, AyeAye, Tricium, chromium...@chromium.org, hansenmichael...@google.com, pushi+wat...@google.com, hansberry+w...@chromium.org, jackshira+w...@google.com, dclasson+w...@google.com, suetfei+wa...@google.com, crisrael+w...@google.com, ajayramamurth...@google.com, xlythe+wa...@google.com, hais+wat...@google.com, blundell+...@chromium.org, mattreyno...@chromium.org, net-r...@chromium.org, network-ser...@chromium.org

Zijie He has created a revert of this change

Open original change

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[net] Refactor TCPSocketWin to allow alternative read/write impls. [chromium/src : main]

Greg Thompson (Gerrit)

Greg Thompson added 2 comments

Related details

Greg Thompson (Gerrit)

Greg Thompson added 5 comments

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 7 comments

Related details

Greg Thompson (Gerrit)

Greg Thompson voted and added 7 comments

Votes added by Greg Thompson

7 comments

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 7 comments

Related details

Greg Thompson (Gerrit)

Greg Thompson voted and added 2 comments

Votes added by Greg Thompson

2 comments

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 1 comment

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 1 comment

Related details

Greg Thompson (Gerrit)

Greg Thompson voted and added 1 comment

Votes added by Greg Thompson

1 comment

Related details

Adam Rice (Gerrit)

Adam Rice voted and added 2 comments

Votes added by Adam Rice

2 comments

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray voted and added 3 comments

Votes added by Francois Pierre Doray

3 comments

Related details

Matt Reynolds (Gerrit)

Matt Reynolds voted and added 1 comment

Votes added by Matt Reynolds

1 comment

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information

Zijie He (Gerrit)

Zijie He has created a revert of this change

Related details