diff options
| -rw-r--r-- | chromium/media/midi/midi_manager_win.cc | 31 | ||||
| -rw-r--r-- | chromium/media/midi/midi_manager_win.h | 4 | ||||
| -rw-r--r-- | chromium/media/midi/task_service.cc | 35 | ||||
| -rw-r--r-- | chromium/media/midi/task_service.h | 47 |
4 files changed, 76 insertions, 41 deletions
diff --git a/chromium/media/midi/midi_manager_win.cc b/chromium/media/midi/midi_manager_win.cc index 7d507bdabda..cb0bdc0ee32 100644 --- a/chromium/media/midi/midi_manager_win.cc +++ b/chromium/media/midi/midi_manager_win.cc @@ -102,8 +102,8 @@ constexpr size_t kSysExSizeLimit = 256 * 1024; constexpr size_t kBufferLength = 32 * 1024; // Global variables to identify MidiManager instance. -constexpr int kInvalidInstanceId = -1; -int g_active_instance_id = kInvalidInstanceId; +constexpr int64_t kInvalidInstanceId = -1; +int64_t g_active_instance_id = kInvalidInstanceId; MidiManagerWin* g_manager_instance = nullptr; // Obtains base::Lock instance pointer to lock instance_id. @@ -113,8 +113,15 @@ base::Lock* GetInstanceIdLock() { } // Issues unique MidiManager instance ID. -int IssueNextInstanceId() { - static int id = kInvalidInstanceId; +int64_t IssueNextInstanceId(base::Optional<int64_t> override_id) { + static int64_t id = kInvalidInstanceId; + if (override_id) { + int64_t result = ++id; + id = *override_id; + return result; + } + if (id == std::numeric_limits<int64_t>::max()) + return kInvalidInstanceId; return ++id; } @@ -685,9 +692,14 @@ MidiManagerWin::PortManager::HandleMidiOutCallback(HMIDIOUT hmo, } } +// static +void MidiManagerWin::OverflowInstanceIdForTesting() { + IssueNextInstanceId(std::numeric_limits<int64_t>::max()); +} + MidiManagerWin::MidiManagerWin(MidiService* service) : MidiManager(service), - instance_id_(IssueNextInstanceId()), + instance_id_(IssueNextInstanceId(base::nullopt)), port_manager_(std::make_unique<PortManager>()) { base::AutoLock lock(*GetInstanceIdLock()); CHECK_EQ(kInvalidInstanceId, g_active_instance_id); @@ -705,6 +717,9 @@ MidiManagerWin::~MidiManagerWin() { void MidiManagerWin::StartInitialization() { { base::AutoLock lock(*GetInstanceIdLock()); + if (instance_id_ == kInvalidInstanceId) + return CompleteInitialization(mojom::Result::INITIALIZATION_ERROR); + CHECK_EQ(kInvalidInstanceId, g_active_instance_id); g_active_instance_id = instance_id_; CHECK_EQ(nullptr, g_manager_instance); @@ -720,7 +735,11 @@ void MidiManagerWin::StartInitialization() { } void MidiManagerWin::Finalize() { - // Unregisters on the I/O thread. OnDevicesChanged() won't be called any more. + + if (instance_id_ == kInvalidInstanceId) + return; + + // Unregisters on the I/O thread. OnDevicesChanged() won't be called any more CHECK(thread_runner_->BelongsToCurrentThread()); base::SystemMonitor::Get()->RemoveDevicesChangedObserver(this); { diff --git a/chromium/media/midi/midi_manager_win.h b/chromium/media/midi/midi_manager_win.h index f3f6093f603..3d23a59fa50 100644 --- a/chromium/media/midi/midi_manager_win.h +++ b/chromium/media/midi/midi_manager_win.h @@ -28,6 +28,8 @@ class MidiManagerWin final public: class PortManager; + MIDI_EXPORT static void OverflowInstanceIdForTesting(); + explicit MidiManagerWin(MidiService* service); ~MidiManagerWin() override; @@ -84,7 +86,7 @@ class MidiManagerWin final const std::vector<uint8_t>& data); // Holds an unique instance ID. - const int instance_id_; + const int64_t instance_id_; // Keeps a TaskRunner for the I/O thread. scoped_refptr<base::SingleThreadTaskRunner> thread_runner_; diff --git a/chromium/media/midi/task_service.cc b/chromium/media/midi/task_service.cc index 3bb3baf01b5..563d63fbf91 100644 --- a/chromium/media/midi/task_service.cc +++ b/chromium/media/midi/task_service.cc @@ -12,19 +12,10 @@ namespace midi { -namespace { - -constexpr TaskService::InstanceId kInvalidInstanceId = -1; - -} // namespace - constexpr TaskService::RunnerId TaskService::kDefaultRunnerId; +constexpr TaskService::InstanceId TaskService::kInvalidInstanceId; -TaskService::TaskService() - : no_tasks_in_flight_cv_(&tasks_in_flight_lock_), - tasks_in_flight_(0), - next_instance_id_(0), - bound_instance_id_(kInvalidInstanceId) { +TaskService::TaskService() : no_tasks_in_flight_cv_(&tasks_in_flight_lock_) { DETACH_FROM_SEQUENCE(instance_binding_sequence_checker_); } @@ -45,10 +36,17 @@ bool TaskService::BindInstance() { base::AutoLock lock(lock_); if (bound_instance_id_ != kInvalidInstanceId) return false; - bound_instance_id_ = next_instance_id_++; + + // If the InstanceId reaches to the limit, just fail rather than doing + // something nicer for such impractical case. + if (std::numeric_limits<InstanceId>::max() == next_instance_id_) + return false; + + bound_instance_id_ = ++next_instance_id_; DCHECK(!default_task_runner_); default_task_runner_ = base::ThreadTaskRunnerHandle::Get(); + return true; } @@ -58,16 +56,17 @@ bool TaskService::UnbindInstance() { base::AutoLock lock(lock_); if (bound_instance_id_ == kInvalidInstanceId) return false; + + DCHECK_EQ(next_instance_id_, bound_instance_id_); bound_instance_id_ = kInvalidInstanceId; DCHECK(default_task_runner_); default_task_runner_ = nullptr; } - // From now on RunTask will never run any task bound to the instance id. // But invoked tasks might be still running here. To ensure no task runs on // quitting this method, wait for all tasks to complete. - base::AutoLock tasks_in_flight_auto_lock(tasks_in_flight_lock_); + base::AutoLock tasks_in_flight_lock(tasks_in_flight_lock_); // TODO(https://crbug.com/796830): Remove sync operations on the I/O thread. base::ScopedAllowBaseSyncPrimitivesOutsideBlockingScope allow_wait; while (tasks_in_flight_ > 0) @@ -126,6 +125,10 @@ void TaskService::PostBoundDelayedTask(RunnerId runner_id, delay); } +void TaskService::OverflowInstanceIdForTesting() { + next_instance_id_ = std::numeric_limits<InstanceId>::max(); +} + scoped_refptr<base::SingleThreadTaskRunner> TaskService::GetTaskRunner( RunnerId runner_id) { base::AutoLock lock(lock_); @@ -154,7 +157,7 @@ void TaskService::RunTask(InstanceId instance_id, RunnerId runner_id, base::OnceClosure task) { { - base::AutoLock tasks_in_flight_auto_lock(tasks_in_flight_lock_); + base::AutoLock tasks_in_flight_lock(tasks_in_flight_lock_); ++tasks_in_flight_; } @@ -162,7 +165,7 @@ void TaskService::RunTask(InstanceId instance_id, std::move(task).Run(); { - base::AutoLock tasks_in_flight_auto_lock(tasks_in_flight_lock_); + base::AutoLock tasks_in_flight_lock(tasks_in_flight_lock_); --tasks_in_flight_; DCHECK_GE(tasks_in_flight_, 0); if (tasks_in_flight_ == 0) diff --git a/chromium/media/midi/task_service.h b/chromium/media/midi/task_service.h index 0845b966bdc..4ee02fe3435 100644 --- a/chromium/media/midi/task_service.h +++ b/chromium/media/midi/task_service.h @@ -5,13 +5,18 @@ #ifndef MEDIA_MIDI_TASK_SERVICE_H_ #define MEDIA_MIDI_TASK_SERVICE_H_ +#include <memory> +#include <vector> + #include "base/callback_forward.h" +#include "base/compiler_specific.h" #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/sequence_checker.h" #include "base/single_thread_task_runner.h" #include "base/synchronization/condition_variable.h" #include "base/synchronization/lock.h" +#include "base/thread_annotations.h" #include "base/threading/thread.h" #include "base/time/time.h" #include "media/midi/midi_export.h" @@ -23,7 +28,7 @@ namespace midi { class MIDI_EXPORT TaskService final { public: using RunnerId = size_t; - using InstanceId = int; + using InstanceId = int64_t; static constexpr RunnerId kDefaultRunnerId = 0; @@ -36,9 +41,10 @@ class MIDI_EXPORT TaskService final { // invoked any more. // Returns true if call is bound or unbound correctly. Otherwise returns // false, that happens when the BindInstance() is called twice without - // unbinding the previous instance. - bool BindInstance(); - bool UnbindInstance(); + // unbinding the previous instance, or the UnbindInstance() is called without + // any successful BindInstance() call. + bool BindInstance() WARN_UNUSED_RESULT; + bool UnbindInstance() WARN_UNUSED_RESULT; // Checks if the current thread belongs to the specified runner. bool IsOnTaskRunner(RunnerId runner_id); @@ -58,7 +64,11 @@ class MIDI_EXPORT TaskService final { base::OnceClosure task, base::TimeDelta delay); + void OverflowInstanceIdForTesting(); + private: + static constexpr TaskService::InstanceId kInvalidInstanceId = -1; + // Returns a SingleThreadTaskRunner reference. Each TaskRunner will be // constructed on demand. scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner(RunnerId runner_id); @@ -71,31 +81,32 @@ class MIDI_EXPORT TaskService final { // Returns true if |instance_id| is equal to |bound_instance_id_|. bool IsInstanceIdStillBound(InstanceId instance_id); + // Holds InstanceId for the next bound instance, accessed on the BindInstance + // call thread without any protection. + InstanceId next_instance_id_ = kInvalidInstanceId; + // Keeps a TaskRunner for the thread that calls BindInstance() as a default // task runner to run posted tasks. - scoped_refptr<base::SingleThreadTaskRunner> default_task_runner_; + scoped_refptr<base::SingleThreadTaskRunner> default_task_runner_ + GUARDED_BY(lock_); // Holds threads to host SingleThreadTaskRunners. - std::vector<std::unique_ptr<base::Thread>> threads_; + std::vector<std::unique_ptr<base::Thread>> threads_ GUARDED_BY(lock_); - // Protects |tasks_in_flight_|. - base::Lock tasks_in_flight_lock_; + // Holds InstanceId for the current bound instance. + InstanceId bound_instance_id_ GUARDED_BY(lock_) = kInvalidInstanceId; + + base::Lock lock_; // Signalled when the number of tasks in flight is 0 and ensures that // UnbindInstance() does not return until all tasks have completed. - base::ConditionVariable no_tasks_in_flight_cv_; + base::ConditionVariable no_tasks_in_flight_cv_ + GUARDED_BY(tasks_in_flight_lock_); // Number of tasks in flight. - int tasks_in_flight_; - - // Holds InstanceId for the next bound instance. - InstanceId next_instance_id_; - - // Holds InstanceId for the current bound instance. - InstanceId bound_instance_id_; + int tasks_in_flight_ GUARDED_BY(tasks_in_flight_lock_) = 0; - // Protects all members other than |tasks_in_flight_|. - base::Lock lock_; + base::Lock tasks_in_flight_lock_; // Verifies all UnbindInstance() calls occur on the same sequence as // BindInstance(). |