Adam Rice (Gerrit)

unread,

Feb 26, 2026, 7:14:50 AM (5 days ago) Feb 26

to Ben Kelly, Tsuyoshi Horo, Chromium IPC Reviews, Dominic Farolino, Chromium LUCI CQ, chromium...@chromium.org, fenced-fra...@chromium.org, network-ser...@chromium.org

Attention needed from Ben Kelly and Dominic Farolino

Adam Rice voted and added 2 comments

Votes added by Adam Rice

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Adam Rice . resolved

lgtm with nit.

File services/network/network_service_unittest.cc

Line 196, Patchset 11 (Latest):

  base::RunLoop run_loop;
  net::CookieList cookies;
  cookie_manager->GetAllCookies(
      base::BindLambdaForTesting([&](const net::CookieList& result) {
        cookies = result;
        run_loop.Quit();
      }));
  run_loop.Run();

Adam Rice . unresolved

This can be simplified with `base::test::TestFuture`:

```suggestion
  base::test::TestFuture<net::CookieList> future;
  cookie_manager->GetAllCookies(future.GetCallback<const net::CookieList&>());
  net::CookeList cookies = future.Get();
```

Open in Gerrit

Related details

Attention is currently required from:

Ben Kelly
Dominic Farolino

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

Ben Kelly (Gerrit)

unread,

Feb 26, 2026, 3:24:05 PM (5 days ago) Feb 26

to Adam Rice, Tsuyoshi Horo, Chromium IPC Reviews, Dominic Farolino, Chromium LUCI CQ, chromium...@chromium.org, fenced-fra...@chromium.org, network-ser...@chromium.org

Attention needed from Dominic Farolino

Ben Kelly voted and added 2 comments

Votes added by Ben Kelly

Commit-Queue

+2

2 comments

Patchset-level comments

File-level comment, Patchset 12 (Latest):

Ben Kelly . resolved

Thanks!

File services/network/network_service_unittest.cc

Line 196, Patchset 11: base::RunLoop run_loop;


  net::CookieList cookies;
  cookie_manager->GetAllCookies(
      base::BindLambdaForTesting([&](const net::CookieList& result) {
        cookies = result;
        run_loop.Quit();
      }));
  run_loop.Run();

Adam Rice . resolved

This can be simplified with `base::test::TestFuture`:

```suggestion
  base::test::TestFuture<net::CookieList> future;
  cookie_manager->GetAllCookies(future.GetCallback<const net::CookieList&>());
  net::CookeList cookies = future.Get();
```

Ben Kelly

Done

Open in Gerrit

Related details

Attention is currently required from:

Dominic Farolino

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

Ben Kelly (Gerrit)

unread,

Feb 26, 2026, 3:46:25 PM (5 days ago) Feb 26

to Chromium IPC Reviews, Adam Rice, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, fenced-fra...@chromium.org, network-ser...@chromium.org

Attention needed from Chromium IPC Reviews

Ben Kelly added 1 comment

Patchset-level comments

File-level comment, Patchset 12 (Latest):

Ben Kelly . resolved

Requesting new mojom owner since Dominic is OOO.

Open in Gerrit

Related details

Attention is currently required from:

Chromium IPC Reviews

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

gwsq (Gerrit)

unread,

Feb 26, 2026, 3:48:09 PM (5 days ago) Feb 26

to Ben Kelly, Chromium IPC Reviews, Dominic Farolino, Adam Rice, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, fenced-fra...@chromium.org, network-ser...@chromium.org

Attention needed from Dominic Farolino

Message from gwsq

From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: d...@chromium.org

📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).

IPC reviewer(s): d...@chromium.org

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

Open in Gerrit

Related details

Attention is currently required from:

Dominic Farolino

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

Dominic Farolino (Gerrit)

unread,

Mar 2, 2026, 5:35:53 PM (18 hours ago) Mar 2

to Ben Kelly, Chromium IPC Reviews, Adam Rice, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, fenced-fra...@chromium.org, network-ser...@chromium.org

Attention needed from Ben Kelly

Dominic Farolino voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Ben Kelly

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

Ben Kelly (Gerrit)

unread,

8:24 AM (4 hours ago) 8:24 AM

to Dominic Farolino, Chromium IPC Reviews, Adam Rice, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org, fenced-fra...@chromium.org, network-ser...@chromium.org

Ben Kelly voted and added 1 comment

Votes added by Ben Kelly

Commit-Queue

+2

1 comment

Patchset-level comments

File-level comment, Patchset 12 (Latest):

Ben Kelly . resolved

Thank you.

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,

9:20 AM (3 hours ago) 9:20 AM

to Ben Kelly, Dominic Farolino, Chromium IPC Reviews, Adam Rice, Tsuyoshi Horo, chromium...@chromium.org, fenced-fra...@chromium.org, network-ser...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Delay binding NetworkContext when cookie manager is pending.

In WebViews there is a provisional CookieManager that is used before
the chromium WebView binary is loaded.  This allows apps to manipulate
the cookie store without paying the performance cost of loading the
entire binary.

When transitioning from the provisional cookie manager to the real
cookie manager, however, it's necessary to wait for any operations to
complete before flushing the database to disk.  Previously this
waiting was done by a synchronous block on the main thread.  We would
like to switch to a non-blocking approach where we defer creating
the NetworkContext until the cookie store is ready.

This CL adds a CookieStoreReadyCallback interface to
NetworkContextParams. When provided,
NetworkService::CreateNetworkContext() defers creating the
NetworkContext object until OnCookieStoreReady() is signaled.  Mojo
messages sent to the NetworkContext remote are naturally buffered in the
pipe until the context is created.  This avoids the need to audit
individual NetworkContext methods for cookie dependencies.

The next CL in the stack adds WebView code that uses this support.

Change-Id: If51c2efd0ff720502e820bd30364394871fc1344

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

Commit-Queue: Ben Kelly <wande...@meta.com>

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

Reviewed-by: Dominic Farolino <d...@chromium.org>

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

Files:

M services/network/network_service.cc
M services/network/network_service.h
M services/network/network_service_unittest.cc
M services/network/public/mojom/network_context.mojom

Change size: M

Delta: 4 files changed, 189 insertions(+), 0 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Dominic Farolino, +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

Delay binding NetworkContext when cookie manager is pending. [chromium/src : main]

Adam Rice (Gerrit)

Adam Rice voted and added 2 comments

Votes added by Adam Rice

2 comments

Related details

Ben Kelly (Gerrit)

Ben Kelly voted and added 2 comments

Votes added by Ben Kelly

2 comments

Related details

Ben Kelly (Gerrit)

Ben Kelly added 1 comment

Related details

gwsq (Gerrit)

Message from gwsq

Related details

Dominic Farolino (Gerrit)

Dominic Farolino voted Code-Review+1

Related details

Ben Kelly (Gerrit)

Ben Kelly voted and added 1 comment

Votes added by Ben Kelly

1 comment

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information