Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(286)

Unified Diff: media/midi/midi_manager.cc

Issue 2443113002: Fix possible multiple calls to MidiManager::StartInitialization() in StartSession() (Closed)
Patch Set: styles Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « media/midi/midi_manager.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/midi/midi_manager.cc
diff --git a/media/midi/midi_manager.cc b/media/midi/midi_manager.cc
index 308982d4edff72d047a44b8dcc22282bd32dbe31..95a48401c09781a26f3fceff88153bff45c03b0e 100644
--- a/media/midi/midi_manager.cc
+++ b/media/midi/midi_manager.cc
@@ -38,13 +38,6 @@ enum class Usage {
MAX = INITIALIZED,
};
-// Used in StartSession.
-enum class Completion {
- COMPLETE_SYNCHRONOUSLY,
- INVOKE_INITIALIZATION,
- COMPLETE_ASYNCHRONOUSLY,
-};
-
void ReportUsage(Usage usage) {
UMA_HISTOGRAM_ENUMERATION("Media.Midi.Usage",
static_cast<Sample>(usage),
@@ -54,7 +47,9 @@ void ReportUsage(Usage usage) {
} // namespace
MidiManager::MidiManager()
- : initialized_(false), finalized_(false), result_(Result::NOT_INITIALIZED) {
+ : initialization_state_(InitializationState::NOT_STARTED),
+ finalized_(false),
+ result_(Result::NOT_INITIALIZED) {
ReportUsage(Usage::CREATED);
}
@@ -91,8 +86,7 @@ void MidiManager::Shutdown() {
void MidiManager::StartSession(MidiManagerClient* client) {
ReportUsage(Usage::SESSION_STARTED);
- Completion completion = Completion::COMPLETE_SYNCHRONOUSLY;
- Result result = Result::NOT_INITIALIZED;
+ bool needs_initialization = false;
{
base::AutoLock auto_lock(lock_);
@@ -103,7 +97,13 @@ void MidiManager::StartSession(MidiManagerClient* client) {
return;
}
- if (initialized_) {
+ // Do not accept a new request if Shutdown() was already called.
+ if (finalized_) {
+ client->CompleteStartSession(Result::INITIALIZATION_ERROR);
+ return;
+ }
+
+ if (initialization_state_ == InitializationState::COMPLETED) {
// Platform dependent initialization was already finished for previously
// initialized clients.
if (result_ == Result::OK) {
@@ -111,33 +111,29 @@ void MidiManager::StartSession(MidiManagerClient* client) {
clients_.insert(client);
}
// Complete synchronously with |result_|;
- result = result_;
- } else {
- bool too_many_pending_clients_exist =
- pending_clients_.size() >= kMaxPendingClientCount;
- // Do not accept a new request if the pending client list contains too
- // many clients, or Shutdown() was already called.
- if (too_many_pending_clients_exist || finalized_) {
- result = Result::INITIALIZATION_ERROR;
- } else {
- // Call StartInitialization() only for the first request.
- if (pending_clients_.empty()) {
- completion = Completion::INVOKE_INITIALIZATION;
- session_thread_runner_ = base::ThreadTaskRunnerHandle::Get();
- } else {
- completion = Completion::COMPLETE_ASYNCHRONOUSLY;
- }
- pending_clients_.insert(client);
- }
+ client->CompleteStartSession(result_);
+ return;
}
- if (completion == Completion::COMPLETE_SYNCHRONOUSLY) {
- client->CompleteStartSession(result);
+ // Do not accept a new request if the pending client list contains too
+ // many clients.
+ if (pending_clients_.size() >= kMaxPendingClientCount) {
+ client->CompleteStartSession(Result::INITIALIZATION_ERROR);
return;
}
+
+ if (initialization_state_ == InitializationState::NOT_STARTED) {
+ // Set fields protected by |lock_| here and call StartInitialization()
+ // later.
+ needs_initialization = true;
+ session_thread_runner_ = base::ThreadTaskRunnerHandle::Get();
+ initialization_state_ = InitializationState::STARTED;
+ }
+
+ pending_clients_.insert(client);
}
- if (completion == Completion::INVOKE_INITIALIZATION) {
+ if (needs_initialization) {
// Lazily initialize the MIDI back-end.
TRACE_EVENT0("midi", "MidiManager::StartInitialization");
// CompleteInitialization() will be called asynchronously when platform
@@ -241,8 +237,8 @@ void MidiManager::CompleteInitializationInternal(Result result) {
base::AutoLock auto_lock(lock_);
DCHECK(clients_.empty());
- DCHECK(!initialized_);
- initialized_ = true;
+ DCHECK_EQ(initialization_state_, InitializationState::STARTED);
+ initialization_state_ = InitializationState::COMPLETED;
result_ = result;
for (auto* client : pending_clients_) {
« no previous file with comments | « media/midi/midi_manager.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698