Fix possible multiple calls to MidiManager::StartInitialization() in StartSession()

Fix possible multiple calls to MidiManager::StartInitialization() in StartSession()

We assume the caller is the first client and call StartInitialization() if
|pending_clients_| is empty, however |pending_clients_| may be cleared before
initialization finishes and cause successive StartSession() calls to call
StartInitialization() again. This leads to unwanted behavior depending on
platform-specific implementation.

Now we use an enum field to track initialization state, and flatten logic in
StartSession() to make things simple.

BUG=658662
Committed: https://crrev.com/a25e7a3d87a75cd00cc849f41a7ef9261a2e18d4
Cr-Commit-Position: refs/heads/master@{#427047}

[Shao-Chuan Lee, 2016-10-24 08:54:33]

shaochuan@chromium.org changed reviewers:
+ toyoshim@chromium.org

[Shao-Chuan Lee, 2016-10-24 08:54:34]

PTAL

https://codereview.chromium.org/2443113002/diff/20001/media/midi/midi_manager.cc
File media/midi/midi_manager.cc (right):

https://codereview.chromium.org/2443113002/diff/20001/media/midi/midi_manager...
media/midi/midi_manager.cc:100: // Do not accept a new request if Shutdown() was
already called.
I guess this check should be moved in front of the COMPLETED check, otherwise
the client would assume this MidiManager is still alive.

[Takashi Toyoshima, 2016-10-24 10:07:29]

thanks.
lgtm % one style nit.

https://codereview.chromium.org/2443113002/diff/20001/media/midi/midi_manager.cc
File media/midi/midi_manager.cc (right):

https://codereview.chromium.org/2443113002/diff/20001/media/midi/midi_manager...
media/midi/midi_manager.cc:100: // Do not accept a new request if Shutdown() was
already called.
Thanks, this looks a nice improvement!

https://codereview.chromium.org/2443113002/diff/20001/media/midi/midi_manager.h
File media/midi/midi_manager.h (right):

https://codereview.chromium.org/2443113002/diff/20001/media/midi/midi_manager...
media/midi/midi_manager.h:193: enum class InitializationState {
https://google.github.io/styleguide/cppguide.html#Declaration_Order

Can you define this enum class immediately after the private section starting at
177?

[Shao-Chuan Lee, 2016-10-24 10:09:38]

https://codereview.chromium.org/2443113002/diff/20001/media/midi/midi_manager.h
File media/midi/midi_manager.h (right):

https://codereview.chromium.org/2443113002/diff/20001/media/midi/midi_manager...
media/midi/midi_manager.h:193: enum class InitializationState {
On 2016/10/24 10:07:29, toyoshim (slow til Nov.2) wrote:
> https://google.github.io/styleguide/cppguide.html#Declaration_Order
> 
> Can you define this enum class immediately after the private section starting
at
> 177?

Done.

[Shao-Chuan Lee, 2016-10-24 10:09:46]

The CQ bit was checked by shaochuan@chromium.org

[Shao-Chuan Lee, 2016-10-24 10:09:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
toyoshim@chromium.org
Link to the patchset: https://codereview.chromium.org/2443113002/#ps40001
(title: "styles")

[commit-bot: I haz the power, 2016-10-24 10:09:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-24 11:34:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-24 11:36:56]

Description was changed from

==========
Fix possible multiple calls to MidiManager::StartInitialization() in
StartSession()

We assume the caller is the first client and call StartInitialization() if
|pending_clients_| is empty, however |pending_clients_| may be cleared before
initialization finishes and cause successive StartSession() calls to call
StartInitialization() again. This leads to unwanted behavior depending on
platform-specific implementation.

Now we use an enum field to track initialization state, and flatten logic in
StartSession() to make things simple.

BUG=658662
==========

to

==========
Fix possible multiple calls to MidiManager::StartInitialization() in
StartSession()

We assume the caller is the first client and call StartInitialization() if
|pending_clients_| is empty, however |pending_clients_| may be cleared before
initialization finishes and cause successive StartSession() calls to call
StartInitialization() again. This leads to unwanted behavior depending on
platform-specific implementation.

Now we use an enum field to track initialization state, and flatten logic in
StartSession() to make things simple.

BUG=658662

Committed: https://crrev.com/a25e7a3d87a75cd00cc849f41a7ef9261a2e18d4
Cr-Commit-Position: refs/heads/master@{#427047}
==========

[commit-bot: I haz the power, 2016-10-24 11:36:57]

Patchset 3 (id:??) landed as
https://crrev.com/a25e7a3d87a75cd00cc849f41a7ef9261a2e18d4
Cr-Commit-Position: refs/heads/master@{#427047}