Takashi Nakayama (Gerrit)

unread,

Mar 1, 2026, 11:07:59 PM (3 hours ago) Mar 1

to Kouhei Ueno, Hiroshige Hayashizaki, Leszek Swirski, Greg Thompson, Chromium IPC Reviews, Rakina Zata Amni, Christian Dullweber, chromium...@chromium.org, Nate Chapin, AyeAye, Chromium LUCI CQ, ipc-securi...@chromium.org, blink-re...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, alexmo...@chromium.org, msrame...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, blink-...@chromium.org, dullweb...@chromium.org, navigation...@chromium.org, creis...@chromium.org

Attention needed from Christian Dullweber, Chromium IPC Reviews, Greg Thompson, Hiroshige Hayashizaki, Kouhei Ueno, Leszek Swirski and Rakina Zata Amni

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:

Christian Dullweber
Chromium IPC Reviews
Greg Thompson
Hiroshige Hayashizaki
Kouhei Ueno
Leszek Swirski
Rakina Zata Amni

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,

Mar 1, 2026, 11:10:53 PM (3 hours ago) Mar 1

to Takashi Nakayama, Chromium IPC Reviews, Takashi Toyoshima, Kouhei Ueno, Hiroshige Hayashizaki, Leszek Swirski, Greg Thompson, Rakina Zata Amni, Christian Dullweber, chromium...@chromium.org, Nate Chapin, AyeAye, Chromium LUCI CQ, ipc-securi...@chromium.org, blink-re...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, alexmo...@chromium.org, msrame...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, blink-...@chromium.org, dullweb...@chromium.org, navigation...@chromium.org, creis...@chromium.org

Attention needed from Christian Dullweber, Greg Thompson, Hiroshige Hayashizaki, Kouhei Ueno, Leszek Swirski, Rakina Zata Amni and Takashi Toyoshima

Message from gwsq

From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: toyo...@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): toyo...@chromium.org

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

Open in Gerrit

Related details

Attention is currently required from:

Christian Dullweber

Greg Thompson
Hiroshige Hayashizaki
Kouhei Ueno
Leszek Swirski
Rakina Zata Amni

Takashi Toyoshima

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

Takashi Toyoshima (Gerrit)

unread,

12:28 AM (2 hours ago) 12:28 AM

to Takashi Nakayama, Chromium IPC Reviews, Kouhei Ueno, Hiroshige Hayashizaki, Leszek Swirski, Greg Thompson, Rakina Zata Amni, Christian Dullweber, chromium...@chromium.org, Nate Chapin, AyeAye, Chromium LUCI CQ, ipc-securi...@chromium.org, blink-re...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, alexmo...@chromium.org, msrame...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, blink-...@chromium.org, dullweb...@chromium.org, navigation...@chromium.org, creis...@chromium.org

Attention needed from Christian Dullweber, Greg Thompson, Hiroshige Hayashizaki, Kouhei Ueno, Leszek Swirski, Rakina Zata Amni and Takashi Nakayama

Takashi Toyoshima added 4 comments

File content/browser/renderer_host/code_cache_host_impl.cc

Line 314, Patchset 7: mojo_base::BigBuffer data) override {}

Takashi Toyoshima . unresolved

Can you comment why this does nothing?

Line 378, Patchset 7: mojo_base::BigBuffer data) override {

Takashi Toyoshima . unresolved

Can you comment why this is not permitted (and why disallowed calls could happen)?

Line 575, Patchset 7: CHECK_EQ(source_hash.size(), 32u);

Takashi Toyoshima . unresolved

Can you have a comment that says this CHECK is safe as this vector size is ensured in the mojo binding, as this is declared as array<uint8, 32>.

Line 680, Patchset 7:

Takashi Toyoshima . unresolved

Let's explain what's this magic is, e.g. why this is needed, and why this magic name is fine.
Also, I think it's robust to modify the original GeneratedCodeCache::GetContextKey() so that we can have a single key generator for the ContextKey. Otherwise, this kind of wrapper causes confusion on creating a consistent key for all users in the chromium.
For instance, if we follow the key name rules in the original method, you may use kSeparator (" \n") to append your specific new prefix, or suffix might be recommended?

Open in Gerrit

Related details

Attention is currently required from:

Christian Dullweber
Greg Thompson
Hiroshige Hayashizaki
Kouhei Ueno
Leszek Swirski
Rakina Zata Amni

Takashi Nakayama

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

Reply all

Reply to author

Forward

[persistent_cache] Support source-keyed code cache on browser process [chromium/src : main]

Takashi Nakayama (Gerrit)

New activity on the change

Related details

gwsq (Gerrit)

Message from gwsq

Related details

Takashi Toyoshima (Gerrit)

Takashi Toyoshima added 4 comments

Related details