Dictation: Perform component extension installation [chromium/src : main]
David Bokan (Gerrit)
David Bokan voted and added 1 comment![]( "Open in Gerrit")
Votes added by David Bokan
| Commit-Queue | +1 |
1 comment
+mcnee@, amyasinghal@ for overall CL
+andreaorru@ for gut check that component extension is being added correctly
Related details
- Amya Singhal
- Andrea Orru
- Kevin McNee
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin McNee (Gerrit)
Kevin McNee voted and added 4 comments![]( "Open in Gerrit")
Votes added by Kevin McNee
| Code-Review | +1 |
4 comments
LGTM
`--enable-features=Dictation:use_component_extension/false`Not necessarily for this CL, but as a future suggestion, since the feature flag will eventually go away, maybe we could have ListenerStreamProvider select the extension to dispatch to instead of broadcasting.
Something like: check the extensions listed in --allowlisted-extension-id followed by the production extension id. Dispatch to the first one that listens for the event.
// Until the connector extension is available consider the feature disabled.Optionally: I wonder if we would get essentially this logic for free by checking EventRouter::HasEventListener for the start stream event?
base::test::ScopedFeatureList CreateEnablingFeatureList();Optional nit: Although it won't make a practical difference, for consistency, shall we use this in chrome/browser/extensions/api/dictation_private/dictation_private_apitest.cc as well?
Related details
- Amya Singhal
- Andrea Orru
- David Bokan
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. |
David Bokan (Gerrit)
David Bokan added 3 comments![]( "Open in Gerrit")
`--enable-features=Dictation:use_component_extension/false`Not necessarily for this CL, but as a future suggestion, since the feature flag will eventually go away, maybe we could have ListenerStreamProvider select the extension to dispatch to instead of broadcasting.
Something like: check the extensions listed in --allowlisted-extension-id followed by the production extension id. Dispatch to the first one that listens for the event.
since the feature flag will eventually go away,
An optimist, I see 😊
Dispatch to the first one that listens for the event.
Is it possible to query the extension for having registered a specific listener type?
Another alternative is to just replace it with a command line flag when removing the base::Feature.
// Until the connector extension is available consider the feature disabled.Optionally: I wonder if we would get essentially this logic for free by checking EventRouter::HasEventListener for the start stream event?
I guess this answers my question above.
It's an option - I see we can also register an observer to watch for an event listener being registered too...
I don't feel too strongly but slightly prefer this approach because 1) it's already written 2) is more explicit 3) seems like it'd be easier to debug / diagnose where an issue lies when things break.
But we can reevaluate later.
base::test::ScopedFeatureList CreateEnablingFeatureList();Optional nit: Although it won't make a practical difference, for consistency, shall we use this in chrome/browser/extensions/api/dictation_private/dictation_private_apitest.cc as well?
- Amya Singhal
- Andrea Orru
- Kevin McNee
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Bokan (Gerrit)
David Bokan added 1 comment![]( "Open in Gerrit")
David Bokan (Gerrit)
David Bokan voted and added 1 comment![]( "Open in Gerrit")
Votes added by David Bokan
| Commit-Queue | +2 |
1 comment
Going to land to avoid rebase conflicts but Andrea, if you could PTAL and let me know if anything jumps out. (otherwise we'll test it live!)
Related details
- Amya Singhal
- Andrea Orru
- Kevin McNee
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Bokan (Gerrit)
David Bokan voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Kevin McNee (Gerrit)
Kevin McNee voted![]( "Open in Gerrit")
- Amya Singhal
- Andrea Orru
- David Bokan
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Dictation: Perform component extension installation
This CL implements the installation of the downloaded component
extension into a profile's extensions. We do this by waiting for the
download, adding the extension, then enabling the feature.
For local development and testing we load the connector extension as a
regular extension. To preserve this capability we add a feature param
which skips the installation process:
`--enable-features=Dictation:use_component_extension/false`
- M chrome/browser/dictation/BUILD.gn
- A chrome/browser/dictation/connector_component_extension.cc
- A chrome/browser/dictation/connector_component_extension.h
- M chrome/browser/dictation/dictation_keyed_service.cc
- M chrome/browser/dictation/dictation_keyed_service.h
- M chrome/browser/dictation/dictation_keyed_service_browsertest.cc
- M chrome/browser/dictation/dictation_keyed_service_unittest.cc
- M chrome/browser/dictation/features.cc
- M chrome/browser/dictation/features.h
- M chrome/browser/dictation/policy_browsertest.cc
- M chrome/browser/dictation/session_ui_impl_browsertest.cc
- M chrome/browser/dictation/test_util.cc
- M chrome/browser/dictation/test_util.h
- M chrome/browser/extensions/api/dictation_private/dictation_private_apitest.cc
- M chrome/test/data/extensions/dictation/README.md
Code-Review: +1 by Kevin McNee
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |