[Extensions] Remove base::Unretained comment in NativeMessageProcessHost [chromium/src : main]
Kelvin Jiang (Gerrit)
Kelvin Jiang voted and added 2 comments![]( "Open in Gerrit")
Votes added by Kelvin Jiang
| Auto-Submit | +1 |
| Commit-Queue | +1 |
2 comments
Quick code health CL + a question (maybe I resolve that in a quick follow up)
task_runner_->PostTask(
FROM_HERE, base::BindOnce(&NativeMessageProcessHost::LaunchHostProcess,
weak_factory_.GetWeakPtr()));When do we actually need to post a task to be run later in the same thread? I feel like things could probably work with just calling `LaunchHostProcess()` directly?
Related details
- Emilia Paz
- James Cook
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. |
James Cook (Gerrit)
James Cook voted and added 2 comments![]( "Open in Gerrit")
Votes added by James Cook
| Code-Review | +1 |
| Commit-Queue | +2 |
2 comments
LGTM
task_runner_->PostTask(
FROM_HERE, base::BindOnce(&NativeMessageProcessHost::LaunchHostProcess,
weak_factory_.GetWeakPtr()));When do we actually need to post a task to be run later in the same thread? I feel like things could probably work with just calling `LaunchHostProcess()` directly?
Hmm. The threading in this class and the parent class isn't entirely clear to me. I agree the `DCHECK(task_runner_->BelongsToCurrentThread())` above seems to imply we're already on the IO thread, since `task_runner_ = content::GetIOThreadTaskRunner({});` above.
But frankly, I wouldn't mess with it. Maybe there's some part of this system that has to be async. <shrug>
Related details
- Emilia Paz
- Kelvin Jiang
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
[Extensions] Remove base::Unretained comment in NativeMessageProcessHost
Code under comment no longer uses base::Unretained, remove the comment.
- M chrome/browser/extensions/api/messaging/native_message_process_host.cc
Code-Review: +1 by James Cook
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |