DevTools: Add Smart Card Emulation Handler Skeleton. [chromium/src : main]
Paulina Gacek (Gerrit)
Paulina Gacek added 1 comment![]( "Open in Gerrit")
session->CreateAndAddHandler<protocol::SmartCardEmulationHandler>();Paulina GacekMy understanding was that the emulation is meant to be global and not related to a specific render frame? has that changed? if it is still global, I think this should be in content/browser/devtools/browser_devtools_agent_host.cc
Alex RudenkoThank you for the comment! While Smart Card devices are physically global, the API implementation (`navigator.smartCard`) and the Mojo interface are bound to the specific RenderFrame.
Paulina GacekIs the emulation also global or would it be per render frame?
Per frame.
Related details
- Alex Rudenko
- Zgroza (Luke) Klimek
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. |
Andrey Kosyakov (Gerrit)
Andrey Kosyakov added 6 comments![]( "Open in Gerrit")
base::WeakPtrFactory<SmartCardEmulationHandler> weak_factory_{this};unused?
base::WeakPtr<RenderFrameHostImpl> host_;raw_ptr<>, please! We're managing this explicitly in all handlers (as in, there would be SetRenderer calls before the host goes away).
.SwapImplForTesting(original_broker_impl_);No ForTesting calls in prod code, please!
if (!enabled_) {So... it's the same whether enabled_ or not?
auto& receiver = rfh_impl->browser_interface_broker_receiver_for_testing();ditto.
return DispatchResponse::ServerError(Just skip these in protocol_config.json instead, the backend will report an error just as well.
Related details
- Alex Rudenko
- Patryk Chodur
- Paulina Gacek
- Zgroza (Luke) Klimek
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. |
Andrey Kosyakov (Gerrit)
Andrey Kosyakov added 1 comment![]( "Open in Gerrit")
It's usually way more practical so write CDP tests in JavaScript. Also, all your implementation is in content/ layer, so I assume it may be possible to test it in the content layer as well? Please see //third_party/blink/web_tests/http/tests/inspector-protocol for examples of content-side CDP tests. It may be possible to run these in chrome as well, the JS-side framework would be the same, but there's a tiny bit of boilerplate required on the C++ side, see chrome/browser/headless/headless_mode_protocol_browsertest.cc.
Paulina Gacek (Gerrit)
Paulina Gacek added 3 comments![]( "Open in Gerrit")
base::WeakPtrFactory<SmartCardEmulationHandler> weak_factory_{this};Paulina Gacekunused?
Done
raw_ptr<>, please! We're managing this explicitly in all handlers (as in, there would be SetRenderer calls before the host goes away).
Done
So... it's the same whether enabled_ or not?
- Alex Rudenko
- Andrey Kosyakov
- Patryk Chodur
- Zgroza (Luke) Klimek
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. |
Paulina Gacek (Gerrit)
Paulina Gacek added 7 comments![]( "Open in Gerrit")
It's usually way more practical so write CDP tests in JavaScript. Also, all your implementation is in content/ layer, so I assume it may be possible to test it in the content layer as well? Please see //third_party/blink/web_tests/http/tests/inspector-protocol for examples of content-side CDP tests. It may be possible to run these in chrome as well, the JS-side framework would be the same, but there's a tiny bit of boilerplate required on the C++ side, see chrome/browser/headless/headless_mode_protocol_browsertest.cc.
Thanks for the suggestion! The Smart Card API must be run within an Isolated Web App (IWA) context. I used IsolatedWebAppBrowserTestHarness (from chrome/) because it handles the complex IWA setup and permissions automatically. Porting this to the content/ layer or web_tests would require significant mocking of the IWA infrastructure.
base::WeakPtrFactory<SmartCardEmulationHandler> weak_factory_{this};Paulina Gacekunused?
Done
I started being used in this CL: https://chromium-review.googlesource.com/c/chromium/src/+/7264773. I moved the declaration there, thanks!
No ForTesting calls in prod code, please!
Done
No ForTesting calls in prod code, please!
Done
auto& receiver = rfh_impl->browser_interface_broker_receiver_for_testing();Paulina Gacekditto.
Done
auto& receiver = rfh_impl->browser_interface_broker_receiver_for_testing();Paulina Gacekditto.
Done
Just skip these in protocol_config.json instead, the backend will report an error just as well.
Done
Related details
- Alex Rudenko
- Andrey Kosyakov
- Patryk Chodur
- Zgroza (Luke) Klimek
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. |
Paulina Gacek (Gerrit)
Paulina Gacek added 1 comment![]( "Open in Gerrit")
Andrey Kosyakov (Gerrit)
Andrey Kosyakov added 1 comment![]( "Open in Gerrit")
Paulina GacekIt's usually way more practical so write CDP tests in JavaScript. Also, all your implementation is in content/ layer, so I assume it may be possible to test it in the content layer as well? Please see //third_party/blink/web_tests/http/tests/inspector-protocol for examples of content-side CDP tests. It may be possible to run these in chrome as well, the JS-side framework would be the same, but there's a tiny bit of boilerplate required on the C++ side, see chrome/browser/headless/headless_mode_protocol_browsertest.cc.
Thanks for the suggestion! The Smart Card API must be run within an Isolated Web App (IWA) context. I used IsolatedWebAppBrowserTestHarness (from chrome/) because it handles the complex IWA setup and permissions automatically. Porting this to the content/ layer or web_tests would require significant mocking of the IWA infrastructure.
Wouldn't this take care of it?
https://source.chromium.org/chromium/chromium/src/+/main:content/shell/common/shell_switches.h;l=45?
(note we've got a virtual test suite setup to use this https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/virtual/isolated-context/README.md, https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/VirtualTestSuites;l=1748?)
Related details
- Alex Rudenko
- Patryk Chodur
- Paulina Gacek
- Zgroza (Luke) Klimek
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. |