[Translation API] Remove TranslationAPI feature flag. [chromium/src : main]
0 views
Skip to first unread message
Isaac Ahouma (Gerrit)
Dec 22, 2025, 9:52:01 AM (6 days ago) Dec 22
to Mike Wasserman, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Mike Wasserman
Isaac Ahouma added 1 comment![]( "Open in Gerrit")
File components/on_device_translation/features.cc
File-level comment, Patchset 27:
Isaac Ahouma . unresolved
@m...@chromium.org I removed these changes, those to service_controlled and to the test files as I think these need to be kept if we are keeping the runtime enabled features as discussed. Do you agree?
Related details
Attention is currently required from:
- Mike Wasserman
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. |
Mike Wasserman (Gerrit)
Dec 22, 2025, 1:43:35 PM (6 days ago) Dec 22
to Isaac Ahouma, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Isaac Ahouma
Mike Wasserman added 4 comments![]( "Open in Gerrit")
File chrome/browser/on_device_translation/on_device_translation_browsertest.cc
Line 1860, Patchset 27:
embedded_https_test_server().SetSSLConfig(Mike Wasserman . unresolved
Why are these server + resolve and the url host checks below needed?
File chrome/browser/on_device_translation/service_controller_manager.h
Line 66, Patchset 27:
// Sets a temporary override for the maximum service count for testing.
// This method should only be called in test contexts.
void SetMaxServiceCountForTesting(std::optional<size_t> count);
// Gets the maximum number of services allowed.
size_t GetMaxServiceCount() const;Mike Wasserman . unresolved
I think it's fine to revert these changes and keep the feature param. Otherwise:
nit: inline accessors, make the value non-optional + non-test-specific + init here.
```
size_t get_max_service_count_for_testing() { return max_service_count_; }
void set_max_service_count_for_testing(size_t count) { max_service_count_ = count; }
...
size_t max_service_count_ = 10;
```
File components/on_device_translation/features.cc
Isaac Ahouma . unresolved
@m...@chromium.org I removed these changes, those to service_controlled and to the test files as I think these need to be kept if we are keeping the runtime enabled features as discussed. Do you agree?
Mike Wasserman
Please do revert feature params changes, we'll just remove the about_flags entry for now. Feature params are still viable for command-line and test overrides.
File third_party/blink/renderer/platform/runtime_enabled_features.json5
Line 5423, Patchset 27:
base_feature_status: "enabled",
copied_from_base_feature_if: "overridden",Mike Wasserman . unresolved
Apparently we can remove these and retain finch kill switch functionality?
Related details
Attention is currently required from:
- Isaac Ahouma
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. |
Isaac Ahouma (Gerrit)
Dec 22, 2025, 1:59:50 PM (6 days ago) Dec 22
to Mike Wasserman, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Mike Wasserman
Isaac Ahouma added 4 comments![]( "Open in Gerrit")
File chrome/browser/on_device_translation/on_device_translation_browsertest.cc
Line 1860, Patchset 27:
embedded_https_test_server().SetSSLConfig(Mike Wasserman . resolved
Why are these server + resolve and the url host checks below needed?
Isaac Ahouma
Done
File chrome/browser/on_device_translation/service_controller_manager.h
Line 66, Patchset 27:
// Sets a temporary override for the maximum service count for testing.
// This method should only be called in test contexts.
void SetMaxServiceCountForTesting(std::optional<size_t> count);
// Gets the maximum number of services allowed.
size_t GetMaxServiceCount() const;Mike Wasserman . resolved
I think it's fine to revert these changes and keep the feature param. Otherwise:
nit: inline accessors, make the value non-optional + non-test-specific + init here.
```
size_t get_max_service_count_for_testing() { return max_service_count_; }
void set_max_service_count_for_testing(size_t count) { max_service_count_ = count; }
...
size_t max_service_count_ = 10;
```
Isaac Ahouma
Marked as resolved.
File components/on_device_translation/features.cc
Mike Wasserman@m...@chromium.org I removed these changes, those to service_controlled and to the test files as I think these need to be kept if we are keeping the runtime enabled features as discussed. Do you agree?
Please do revert feature params changes, we'll just remove the about_flags entry for now. Feature params are still viable for command-line and test overrides.
Isaac Ahouma
Resolved.
File third_party/blink/renderer/platform/runtime_enabled_features.json5
Line 5423, Patchset 27:
base_feature_status: "enabled",
copied_from_base_feature_if: "overridden",Mike Wasserman . resolved
Apparently we can remove these and retain finch kill switch functionality?
Isaac Ahouma
This is an older patchset. I removed these changes based on what we found out about the killswitch. I just wrote my above comment as an FYI. Sorry for not clarifying that.
Related details
Attention is currently required from:
- Mike Wasserman
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mike Wasserman (Gerrit)
Dec 22, 2025, 2:41:54 PM (6 days ago) Dec 22
to Isaac Ahouma, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Isaac Ahouma
Mike Wasserman added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 28 (Latest):
Mike Wasserman . resolved
gotcha. code changes lg (modulo rebase). please updated cl desc
Commit Message
Line 7, Patchset 28 (Latest):
[Translation API] Remove TranslationAPI feature flag.Mike Wasserman . unresolved
Update CL description for just removing chrome://flags entry and stale param/variation
Related details
Attention is currently required from:
- Isaac Ahouma
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. |
Isaac Ahouma (Gerrit)
Dec 23, 2025, 9:42:31 AM (5 days ago) Dec 23
to Mike Wasserman, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Mike Wasserman
Isaac Ahouma added 1 comment![]( "Open in Gerrit")
Commit Message
Update CL description for just removing chrome://flags entry and stale param/variation
Attention is currently required from:
- Mike Wasserman
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mike Wasserman (Gerrit)
Dec 23, 2025, 2:25:44 PM (5 days ago) Dec 23
to Isaac Ahouma, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Isaac Ahouma
Mike Wasserman voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Isaac Ahouma
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Isaac Ahouma (Gerrit)
Dec 26, 2025, 2:47:06 PM (2 days ago) Dec 26
to Khushal Sagar, Mike Wasserman, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Khushal Sagar and Nasko Oskov
Isaac Ahouma added 2 comments![]( "Open in Gerrit")
File content/browser/browser_interface_binders.cc
File-level comment, Patchset 29 (Latest):
Isaac Ahouma . unresolved
Hi @na...@chromium.org, Adding you for a review on this since you are a code owner for this file. Thanks
File third_party/blink/renderer/platform/runtime_enabled_features.json5
File-level comment, Patchset 29 (Latest):
Isaac Ahouma . unresolved
Hi @khusha...@chromium.org, Adding you for a review on this since you are a code owner for this file. Thanks
Related details
Attention is currently required from:
- Khushal Sagar
- Nasko Oskov
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. |
Nasko Oskov (Gerrit)
Dec 26, 2025, 6:00:01 PM (2 days ago) Dec 26
to Isaac Ahouma, Khushal Sagar, Mike Wasserman, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Isaac Ahouma and Khushal Sagar
Nasko Oskov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nasko Oskov
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Isaac Ahouma
- Khushal Sagar
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. |
Khushal Sagar (Gerrit)
Dec 26, 2025, 6:10:09 PM (2 days ago) Dec 26
to Isaac Ahouma, Mike Wasserman, Enterprise Policy Reviews, AyeAye, Chromium LUCI CQ, network-ser...@chromium.org, jmedle...@chromium.org, asvitki...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Isaac Ahouma
Khushal Sagar voted and added 1 comment![]( "Open in Gerrit")
Votes added by Khushal Sagar
| Code-Review | +1 |
1 comment
File third_party/blink/renderer/platform/runtime_enabled_features.json5
Line 5414, Patchset 29 (Latest):
{
name: "TranslationAPI",
status: {
"Win": "stable",
"Mac": "stable",
"Linux": "stable",
"ChromeOS": "stable",
"default": "",
},
base_feature_status: "enabled",
copied_from_base_feature_if: "overridden",
},Khushal Sagar . unresolved
you can remove this entry itself since all usage is being removed from the codebase.
Related details
Attention is currently required from:
- Isaac Ahouma
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. |
Reply all
Reply to author
Forward
0 new messages