[task] Remove CurrentThread::SetTaskRunner [chromium/src : main]
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray voted and added 1 comment![]( "Open in Gerrit")
Votes added by Etienne Pierre-Doray
| Commit-Queue | +1 |
1 comment
Related details
- Gabriel Charette
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gabriel Charette (Gerrit)
Gabriel Charette voted and added 2 comments![]( "Open in Gerrit")
Votes added by Gabriel Charette
| Code-Review | +1 |
2 comments
explicit MainThreadDefaultHandle(Add a SingleThreadTaskRunnerCurrentDefaultHandleTest?
base::CurrentThread::Get()->SetTaskRunner(task_runner);Ouch, that was terrible, glad we removed that possibility..!
Well I guess it's still possible with CurrentHandleOverrideForTesting 😞, worth making that private+friend..?
Related details
- Etienne Pierre-Doray
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. |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray added 3 comments![]( "Open in Gerrit")
+amehfooz@ for ui/message_center/
explicit MainThreadDefaultHandle(Add a SingleThreadTaskRunnerCurrentDefaultHandleTest?
Do you mean to avoid friendship ScopedMockTimeMessageLoopTaskRunner?
I don't think it's worth for one-of, and we don't actually want to let other tests (more) easily use this?
base::CurrentThread::Get()->SetTaskRunner(task_runner);Ouch, that was terrible, glad we removed that possibility..!
Well I guess it's still possible with CurrentHandleOverrideForTesting 😞, worth making that private+friend..?
At least CurrentHandleOverrideForTesting forces scoping.
3 out of 4 CurrentHandleOverrideForTesting just use it with TestMockTimeTaskRunner, which is the same thing as ScopedMockTimeMessageLoopTaskRunner.
https://source.chromium.org/search?q=CurrentHandleOverrideForTesting%20-file:base&sq=&ss=chromium
We already encourage TaskEnvironment over ScopedMockTimeMessageLoopTaskRunner, but since this isn't supported for e.g. browser tests, ScopedMockTimeMessageLoopTaskRunner is still useful.
Idk if it's worth forcing people to add friendship for new usage; there's also a PRESUBMIT.py for ScopedMockTimeMessageLoopTaskRunner, but not for CurrentHandleOverrideForTesting (I could add that).
Related details
- Ahmed Mehfooz
- Gabriel Charette
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. |
Gabriel Charette (Gerrit)
Gabriel Charette added 2 comments![]( "Open in Gerrit")
explicit MainThreadDefaultHandle(Etienne Pierre-DorayAdd a SingleThreadTaskRunnerCurrentDefaultHandleTest?
Do you mean to avoid friendship ScopedMockTimeMessageLoopTaskRunner?
I don't think it's worth for one-of, and we don't actually want to let other tests (more) easily use this?
No, I mean a unittest (that's an existing test fixture name)
base::CurrentThread::Get()->SetTaskRunner(task_runner);Etienne Pierre-DorayOuch, that was terrible, glad we removed that possibility..!
Well I guess it's still possible with CurrentHandleOverrideForTesting 😞, worth making that private+friend..?
At least CurrentHandleOverrideForTesting forces scoping.
3 out of 4 CurrentHandleOverrideForTesting just use it with TestMockTimeTaskRunner, which is the same thing as ScopedMockTimeMessageLoopTaskRunner.
https://source.chromium.org/search?q=CurrentHandleOverrideForTesting%20-file:base&sq=&ss=chromium
We already encourage TaskEnvironment over ScopedMockTimeMessageLoopTaskRunner, but since this isn't supported for e.g. browser tests, ScopedMockTimeMessageLoopTaskRunner is still useful.
Idk if it's worth forcing people to add friendship for new usage; there's also a PRESUBMIT.py for ScopedMockTimeMessageLoopTaskRunner, but not for CurrentHandleOverrideForTesting (I could add that).
- Ahmed Mehfooz
- Etienne Pierre-Doray
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. |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray added 2 comments![]( "Open in Gerrit")
Etienne Pierre-DorayAdd a SingleThreadTaskRunnerCurrentDefaultHandleTest?
Gabriel CharetteDo you mean to avoid friendship ScopedMockTimeMessageLoopTaskRunner?
I don't think it's worth for one-of, and we don't actually want to let other tests (more) easily use this?
No, I mean a unittest (that's an existing test fixture name)
- Ahmed Mehfooz
- Gabriel Charette
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ahmed Mehfooz (Gerrit)
Ahmed Mehfooz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ahmed Mehfooz
| Code-Review | +1 |
1 comment
- Etienne Pierre-Doray
- Gabriel Charette
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray voted Commit-Queue+2![]( "Open in Gerrit")
- Gabriel Charette
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
[task] Remove CurrentThread::SetTaskRunner
This CL removes CurrentThread::SetTaskRunner(), and uses better
alternatives in the 2 existing calls to it.
Drive-by: uses optional to hold
CurrentDefaultHandle/MainThreadDefaultHandle, leveraging pass key
pattern.
- M base/task/current_thread.cc
- M base/task/current_thread.h
- M base/task/sequence_manager/thread_controller_with_message_pump_impl.cc
- M base/task/sequence_manager/thread_controller_with_message_pump_impl.h
- M base/task/single_thread_task_runner.cc
- M base/task/single_thread_task_runner.h
- M base/task/single_thread_task_runner_unittest.cc
- M base/test/scoped_mock_time_message_loop_task_runner.cc
- M base/test/scoped_mock_time_message_loop_task_runner.h
- M base/test/scoped_mock_time_message_loop_task_runner_unittest.cc
- M ui/message_center/message_center_impl_unittest.cc
Code-Review: +1 by Ahmed Mehfooz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |