| Commit-Queue | +1 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
std::move(callback).Run(context_token);I am slightly worried about this and what the callback can do with the token - looking at the code for what calls this - its probably fine but can get worse later.
non blocking comment but something to think about for how we can enforce that the callbacks continue to remain safe.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
std::move(callback).Run(context_token);I am slightly worried about this and what the callback can do with the token - looking at the code for what calls this - its probably fine but can get worse later.
non blocking comment but something to think about for how we can enforce that the callbacks continue to remain safe.
Totally agreed, thanks Sophie! I put a TODO here to make this desire explicit
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
nit: Mind saying something like we now synchronously generate the token.
std::move(callback).Run(context_token);Jason HuI am slightly worried about this and what the callback can do with the token - looking at the code for what calls this - its probably fine but can get worse later.
non blocking comment but something to think about for how we can enforce that the callbacks continue to remain safe.
Totally agreed, thanks Sophie! I put a TODO here to make this desire explicit
I was thinking of dropping the callback from mojom but it seems it's used here.
https://source.chromium.org/chromium/chromium/src/+/main:ui/webui/resources/cr_components/composebox/composebox.ts;drc=d570742b45b6f71bef4dfcce5f8c1710a4d4fdcb;l=678
But I agree, it looks a bit weird. Wanted to change it to say something like `TokenCallback` but I can't since it's mojom autogenerated code.
FROM_HERE, base::BindOnce(std::move(callback), true));nit: `/*<param_name>=*/`
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
nit: Mind saying something like we now synchronously generate the token.
Done
FROM_HERE, base::BindOnce(std::move(callback), true));Jason Hunit: `/*<param_name>=*/`
Done
FROM_HERE, base::BindOnce(std::move(callback), true));Jason Hunit: `/*<param_name>=*/`
Done
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |