gwsq (Gerrit)

unread,

Feb 12, 2025, 11:40:53 AMFeb 12

to Mingyu Lei, Chromium IPC Reviews, Takashi Toyoshima, Mike Wasserman, Daseul Lee, Chromium Metrics Reviews, Chromium LUCI CQ, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Attention needed from Mike Wasserman 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:

Mike Wasserman
Takashi Toyoshima

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

Mike Wasserman (Gerrit)

unread,

Feb 12, 2025, 1:30:36 PMFeb 12

to Mingyu Lei, Chromium IPC Reviews, Takashi Toyoshima, Daseul Lee, Chromium Metrics Reviews, Chromium LUCI CQ, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Attention needed from Mingyu Lei and Takashi Toyoshima

Mike Wasserman added 7 comments

File third_party/blink/renderer/modules/ai/ai_availability.h

Line 15, Patchset 9 (Latest):inline constexpr char kAILanguageTagEn[] = "en";

Mike Wasserman . unresolved

optional nit: duplicate and inline this in the `languageAvailable` functions of `ai_language_model_capabilities.cc` and `ai_summarizer_capabilities.cc`, to be more clearly removed with those deprecated functions.

File third_party/blink/renderer/modules/ai/ai_availability.cc

Line 20, Patchset 9 (Latest): AIAvailability availability;

Mike Wasserman . unresolved

nit: assign the default `kUnavailable` value on construction

Line 21, Patchset 9 (Latest): if (result == mojom::blink::ModelAvailabilityCheckResult::kAvailable) {

Mike Wasserman . unresolved

nit: can this be a switch?

File third_party/blink/renderer/modules/ai/ai_availability.idl

Line 5, Patchset 9 (Latest):// https://github.com/webmachinelearning/prompt-api

Mike Wasserman . unresolved

nit: also cite `https://github.com/webmachinelearning/writing-assistance-apis` and `https://github.com/webmachinelearning/translation-api`

File third_party/blink/renderer/modules/ai/ai_metrics.h

Line 60, Patchset 9 (Latest): static std::string GetAICapabilityAvailabilityMetricName(

Mike Wasserman . unresolved

This can be removed now if it's not called anymore.

File third_party/blink/renderer/modules/ai/ai_rewriter_factory.cc

Line 177, Patchset 9 (Latest): AIAvailabilityToAICapabilityAvailability(

Mike Wasserman . unresolved

I think all the *Factory::availability() functions (writer, rewriter, summarizer, languageModel) can use the new enum straight away, right?

File third_party/blink/web_tests/wpt_internal/ai/resources/utils.js

Line 222, Patchset 9 (Latest): assert_true(availability !== 'unavailable');

Mike Wasserman . unresolved

optional nit: assert_not_equals

Open in Gerrit

Related details

Attention is currently required from:

Mingyu Lei
Takashi Toyoshima

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

Mingyu Lei (Gerrit)

unread,

Feb 13, 2025, 4:43:56 AMFeb 13

to Chromium IPC Reviews, Takashi Toyoshima, Mike Wasserman, Daseul Lee, Chromium Metrics Reviews, Chromium LUCI CQ, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, ipc-securi...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Attention needed from Mike Wasserman and Takashi Toyoshima

Mingyu Lei added 7 comments

File third_party/blink/renderer/modules/ai/ai_availability.h

Line 15, Patchset 9:inline constexpr char kAILanguageTagEn[] = "en";

Mike Wasserman . resolved

optional nit: duplicate and inline this in the `languageAvailable` functions of `ai_language_model_capabilities.cc` and `ai_summarizer_capabilities.cc`, to be more clearly removed with those deprecated functions.

Mingyu Lei

I just remove the const and use "en" literal for those 2 methods to be removed.

File third_party/blink/renderer/modules/ai/ai_availability.cc

Line 20, Patchset 9: AIAvailability availability;

Mike Wasserman . resolved

nit: assign the default `kUnavailable` value on construction

Mingyu Lei

Done

Line 21, Patchset 9: if (result == mojom::blink::ModelAvailabilityCheckResult::kAvailable) {

Mike Wasserman . resolved

nit: can this be a switch?

Mingyu Lei

Done

File third_party/blink/renderer/modules/ai/ai_availability.idl

Line 5, Patchset 9:// https://github.com/webmachinelearning/prompt-api

Mike Wasserman . resolved

nit: also cite `https://github.com/webmachinelearning/writing-assistance-apis` and `https://github.com/webmachinelearning/translation-api`

Mingyu Lei

Done

File third_party/blink/renderer/modules/ai/ai_metrics.h

Line 60, Patchset 9: static std::string GetAICapabilityAvailabilityMetricName(

Mike Wasserman . resolved

This can be removed now if it's not called anymore.

Mingyu Lei

Done

File third_party/blink/renderer/modules/ai/ai_rewriter_factory.cc

Line 177, Patchset 9: AIAvailabilityToAICapabilityAvailability(

Mike Wasserman . resolved

I think all the *Factory::availability() functions (writer, rewriter, summarizer, languageModel) can use the new enum straight away, right?

Mingyu Lei

I was not very sure about the current plan for the other `availability()` method so just conservatively kept the old behavior, let me change them together in the follow up CL.

File third_party/blink/web_tests/wpt_internal/ai/resources/utils.js

Line 222, Patchset 9: assert_true(availability !== 'unavailable');

Mike Wasserman . resolved

optional nit: assert_not_equals

Mingyu Lei

Resolved in another CL which cleans up many other `assert_true`s.

Open in Gerrit

Related details

Attention is currently required from:

Mike Wasserman
Takashi Toyoshima

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

Mingyu Lei (Gerrit)

unread,

Feb 13, 2025, 5:24:17 AMFeb 13

to Daseul Lee, Chromium IPC Reviews, Takashi Toyoshima, Mike Wasserman, Daseul Lee, Chromium Metrics Reviews, Chromium LUCI CQ, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, ipc-securi...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Attention needed from Daseul Lee, Mike Wasserman and Takashi Toyoshima

Mingyu Lei voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Daseul Lee
Mike Wasserman
Takashi Toyoshima

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

Mike Wasserman (Gerrit)

unread,

Feb 13, 2025, 7:00:22 AMFeb 13

to Mingyu Lei, Daseul Lee, Chromium IPC Reviews, Takashi Toyoshima, Daseul Lee, Chromium Metrics Reviews, Chromium LUCI CQ, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, ipc-securi...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Attention needed from Daseul Lee, Mingyu Lei and Takashi Toyoshima

Mike Wasserman voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Daseul Lee
Mingyu Lei
Takashi Toyoshima

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

Daseul Lee (Gerrit)

unread,

Feb 13, 2025, 11:33:49 AMFeb 13

to Mingyu Lei, Christine Hollingsworth, Nathan Memmott, Mike Wasserman, Chromium IPC Reviews, Takashi Toyoshima, Daseul Lee, Chromium Metrics Reviews, Chromium LUCI CQ, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, ipc-securi...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Attention needed from Mingyu Lei and Takashi Toyoshima

Daseul Lee voted and added 1 comment

Votes added by Daseul Lee

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 15 (Latest):

Daseul Lee . resolved

LGTM!

Open in Gerrit

Related details

Attention is currently required from:

Mingyu Lei
Takashi Toyoshima

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

Takashi Toyoshima (Gerrit)

unread,

Feb 13, 2025, 11:24:51 PMFeb 13

to Mingyu Lei, Daseul Lee, Christine Hollingsworth, Nathan Memmott, Mike Wasserman, Chromium IPC Reviews, Daseul Lee, Chromium Metrics Reviews, Chromium LUCI CQ, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, ipc-securi...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Attention needed from Mingyu Lei

Takashi Toyoshima voted and added 2 comments

Votes added by Takashi Toyoshima

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 15 (Latest):

Takashi Toyoshima . resolved

ipc, blink, histograms lgtm

File tools/metrics/histograms/metadata/ai/histograms.xml

Line 131, Patchset 15 (Latest): <owner>fer...@chromium.org</owner>

Takashi Toyoshima . unresolved

optional: You may specify a component's owner file, src/chrome/browser/ai/OWNERS, as the secondary or tertiary owner so that the information can be up-to-date semi-automatically.

Open in Gerrit

Related details

Attention is currently required from:

Mingyu Lei

Submit Requirements:

Code-Coverage
Code-Review

No-Unresolved-Comments

satisfied_requirement

unsatisfied_requirement

open

diffy

Mingyu Lei (Gerrit)

unread,

Feb 14, 2025, 2:22:58 AMFeb 14

to Takashi Toyoshima, Daseul Lee, Christine Hollingsworth, Nathan Memmott, Mike Wasserman, Chromium IPC Reviews, Daseul Lee, Chromium Metrics Reviews, Chromium LUCI CQ, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, ipc-securi...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Mingyu Lei voted and added 1 comment

Votes added by Mingyu Lei

Commit-Queue

+2

1 comment

File tools/metrics/histograms/metadata/ai/histograms.xml

Line 131, Patchset 15 (Latest): <owner>fer...@chromium.org</owner>

Takashi Toyoshima . resolved

optional: You may specify a component's owner file, src/chrome/browser/ai/OWNERS, as the secondary or tertiary owner so that the information can be up-to-date semi-automatically.

Mingyu Lei

Thanks! I will update all the histograms here in a separate CL.

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

Chromium LUCI CQ (Gerrit)

unread,

Feb 14, 2025, 2:26:31 AMFeb 14

to Mingyu Lei, Takashi Toyoshima, Daseul Lee, Christine Hollingsworth, Nathan Memmott, Mike Wasserman, Chromium IPC Reviews, Daseul Lee, Chromium Metrics Reviews, Raphael Kubo da Costa, Kentaro Hara, AyeAye, chromium...@chromium.org, ipc-securi...@chromium.org, android-web...@chromium.org, ashleynewson+w...@chromium.org, peter+watch...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Prompt API: rename availability enum values

Following the spec changes, this CL updates the enum value returned
by the new `availability()` API for the prompt API. The old
`capabilities()` API will remain unchanged for backward compatibility.

- readily -> available
- after-download -> downloadable
- no -> unavailable

The downloading state will be implemented in the later CL.

NO_IFTTT=false alarm

Bug: 395509560

Change-Id: I8b081b6132cf4deec2d63d6e5535da82781771ea

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

Reviewed-by: Daseul Lee <ds...@chromium.org>

Reviewed-by: Takashi Toyoshima <toyo...@chromium.org>

Commit-Queue: Mingyu Lei <le...@chromium.org>

Reviewed-by: Mike Wasserman <m...@chromium.org>

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

Files:

M chrome/browser/ai/ai_language_model_unittest.cc
M chrome/browser/ai/ai_manager.cc
M chrome/browser/ai/ai_manager_unittest.cc
M chrome/browser/ai/ai_on_device_model_component_observer.h
M chrome/browser/ai/ai_rewriter_unittest.cc
M chrome/browser/ai/ai_summarizer_unittest.cc
M chrome/browser/ai/ai_writer_unittest.cc
M content/browser/ai/echo_ai_manager_impl.cc
M third_party/blink/public/mojom/ai/ai_manager.mojom
M third_party/blink/renderer/bindings/generated_in_modules.gni
M third_party/blink/renderer/bindings/idl_in_modules.gni
M third_party/blink/renderer/modules/ai/BUILD.gn
A third_party/blink/renderer/modules/ai/ai_availability.cc
A third_party/blink/renderer/modules/ai/ai_availability.h
A third_party/blink/renderer/modules/ai/ai_availability.idl
M third_party/blink/renderer/modules/ai/ai_capability_availability.cc
M third_party/blink/renderer/modules/ai/ai_capability_availability.h
M third_party/blink/renderer/modules/ai/ai_capability_availability.idl
M third_party/blink/renderer/modules/ai/ai_language_model_capabilities.cc
M third_party/blink/renderer/modules/ai/ai_language_model_factory.cc
M third_party/blink/renderer/modules/ai/ai_language_model_factory.h
M third_party/blink/renderer/modules/ai/ai_language_model_factory.idl
M third_party/blink/renderer/modules/ai/ai_metrics.cc
M third_party/blink/renderer/modules/ai/ai_metrics.h
M third_party/blink/renderer/modules/ai/ai_rewriter_factory.cc
M third_party/blink/renderer/modules/ai/ai_summarizer_capabilities.cc
M third_party/blink/renderer/modules/ai/ai_summarizer_factory.cc
M third_party/blink/renderer/modules/ai/ai_writer_factory.cc
M third_party/blink/renderer/modules/ai/exception_helpers.cc
M third_party/blink/web_tests/wpt_internal/ai/language-model-api-availability.https.any.js
M third_party/blink/web_tests/wpt_internal/ai/language-model-api-prompt-monitor-callback-exception.https.any.js
M third_party/blink/web_tests/wpt_internal/ai/resources/utils.js
M tools/metrics/histograms/metadata/ai/enums.xml
M tools/metrics/histograms/metadata/ai/histograms.xml

Change size: L

Delta: 34 files changed, 329 insertions(+), 175 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Takashi Toyoshima, +1 by Daseul Lee, +1 by Mike Wasserman

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

Prompt API: rename availability enum values [chromium/src : main]

gwsq (Gerrit)

Message from gwsq

Related details

Mike Wasserman (Gerrit)

Mike Wasserman added 7 comments

Related details

Mingyu Lei (Gerrit)

Mingyu Lei added 7 comments

Related details

Mingyu Lei (Gerrit)

Mingyu Lei voted Commit-Queue+1

Related details

Mike Wasserman (Gerrit)

Mike Wasserman voted Code-Review+1

Related details

Daseul Lee (Gerrit)

Daseul Lee voted and added 1 comment

Votes added by Daseul Lee

1 comment

Related details

Takashi Toyoshima (Gerrit)

Takashi Toyoshima voted and added 2 comments

Votes added by Takashi Toyoshima

2 comments

Related details

Mingyu Lei (Gerrit)

Mingyu Lei voted and added 1 comment

Votes added by Mingyu Lei

1 comment

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information