Web MIDI: introduce MidiService class

Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

See crbug.com/672793 for rough design and plan.

BUG=672793
Committed: https://crrev.com/967340a92a250683594345735212482575c720ae
Cr-Commit-Position: refs/heads/master@{#438765}

[Takashi Toyoshima, 2016-12-09 13:10:22]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 13:10:40]

Dry run: 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-12-09 13:14:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 13:14:26]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Takashi Toyoshima, 2016-12-09 13:22:05]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 13:22:16]

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

[Takashi Toyoshima, 2016-12-09 13:30:45]

Description was changed from

==========
Web MIDI: introduce MidiService class

BUG=672793
==========

to

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

BUG=672793
==========

[Takashi Toyoshima, 2016-12-09 13:30:45]

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

[Takashi Toyoshima, 2016-12-09 13:32:09]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 13:32:22]

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

[Takashi Toyoshima, 2016-12-09 13:33:10]

Description was changed from

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

BUG=672793
==========

to

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

See crbug.com/672793 for rough design and plan

BUG=672793
==========

[Takashi Toyoshima, 2016-12-09 13:33:24]

Description was changed from

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

See crbug.com/672793 for rough design and plan

BUG=672793
==========

to

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

See crbug.com/672793 for rough design and plan.

BUG=672793
==========

[Takashi Toyoshima, 2016-12-09 13:34:01]

PTAL.
Very sorry for many review requests.

[commit-bot: I haz the power, 2016-12-09 14:52:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 14:52:19]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[yhirano, 2016-12-13 04:02:30]

https://codereview.chromium.org/2566673002/diff/40001/media/midi/midi_service.h
File media/midi/midi_service.h (right):

https://codereview.chromium.org/2566673002/diff/40001/media/midi/midi_service...
media/midi/midi_service.h:25: class MIDI_EXPORT MidiService {
+final

https://codereview.chromium.org/2566673002/diff/40001/media/midi/midi_service...
media/midi/midi_service.h:28: explicit MidiService(MidiManager* manager_ =
nullptr);
Can this parameter be a std::unique_ptr?

[Takashi Toyoshima, 2016-12-13 05:57:00]

https://codereview.chromium.org/2566673002/diff/40001/media/midi/midi_service.h
File media/midi/midi_service.h (right):

https://codereview.chromium.org/2566673002/diff/40001/media/midi/midi_service...
media/midi/midi_service.h:25: class MIDI_EXPORT MidiService {
On 2016/12/13 04:02:30, yhirano wrote:
> +final

Done.

https://codereview.chromium.org/2566673002/diff/40001/media/midi/midi_service...
media/midi/midi_service.h:28: explicit MidiService(MidiManager* manager_ =
nullptr);
On 2016/12/13 04:02:30, yhirano wrote:
> Can this parameter be a std::unique_ptr?

Done.

[Takashi Toyoshima, 2016-12-13 05:57:06]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 05:57:37]

Dry run: 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-12-13 07:58:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 07:58:09]

Dry run: This issue passed the CQ dry run.

[Takashi Toyoshima, 2016-12-13 11:24:32]

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

[Takashi Toyoshima, 2016-12-13 11:24:47]

+jochen for content/

[yhirano, 2016-12-13 11:28:00]

https://codereview.chromium.org/2566673002/diff/60001/content/browser/media/m...
File content/browser/media/midi_host_unittest.cc (right):

https://codereview.chromium.org/2566673002/diff/60001/content/browser/media/m...
content/browser/media/midi_host_unittest.cc:10: #include "base/macros.h"
+base/memory/ptr_util.h

https://codereview.chromium.org/2566673002/diff/60001/media/midi/midi_service.h
File media/midi/midi_service.h (right):

https://codereview.chromium.org/2566673002/diff/60001/media/midi/midi_service...
media/midi/midi_service.h:27: explicit MidiService(std::unique_ptr<MidiManager>
manager_ =
|manager_| should be |manager|.

https://codereview.chromium.org/2566673002/diff/60001/media/midi/midi_service...
media/midi/midi_service.h:28: base::WrapUnique<MidiManager>(nullptr));
Does |= nullptr| work? (It works on my local env).

[Takashi Toyoshima, 2016-12-13 12:08:02]

thanks

https://codereview.chromium.org/2566673002/diff/60001/content/browser/media/m...
File content/browser/media/midi_host_unittest.cc (right):

https://codereview.chromium.org/2566673002/diff/60001/content/browser/media/m...
content/browser/media/midi_host_unittest.cc:10: #include "base/macros.h"
On 2016/12/13 11:28:00, yhirano wrote:
> +base/memory/ptr_util.h

Done.

https://codereview.chromium.org/2566673002/diff/60001/media/midi/midi_service.h
File media/midi/midi_service.h (right):

https://codereview.chromium.org/2566673002/diff/60001/media/midi/midi_service...
media/midi/midi_service.h:27: explicit MidiService(std::unique_ptr<MidiManager>
manager_ =
On 2016/12/13 11:28:00, yhirano wrote:
> |manager_| should be |manager|.

Done.

https://codereview.chromium.org/2566673002/diff/60001/media/midi/midi_service...
media/midi/midi_service.h:28: base::WrapUnique<MidiManager>(nullptr));
Hum... I may have misread a compiler error. It seems to compile too.

[yhirano, 2016-12-13 12:15:02]

lgtm

[Takashi Toyoshima, 2016-12-14 03:58:24]

jochen: can you review user side mechanical changes under content?
Or probably I could land this with TBR.

[jochen (gone - plz use gerrit), 2016-12-14 11:40:18]

lgtm

[Takashi Toyoshima, 2016-12-14 11:49:20]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2016-12-14 11:49:37]

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-12-14 13:32:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-14 13:32:57]

Failed to apply patch for content/browser/browser_main_loop.h:
While running git apply --index -p1;
  error: patch failed: content/browser/browser_main_loop.h:291
  error: content/browser/browser_main_loop.h: patch does not apply

Patch:       content/browser/browser_main_loop.h
Index: content/browser/browser_main_loop.h
diff --git a/content/browser/browser_main_loop.h
b/content/browser/browser_main_loop.h
index
b3c357de44f5a9bb3c05ddc55308f3112f631ef4..09471c22c9b2f334cce04d0a143829a3e4132234
100644
--- a/content/browser/browser_main_loop.h
+++ b/content/browser/browser_main_loop.h
@@ -52,7 +52,7 @@ class DeviceMonitorMac;
 }  // namespace media
 
 namespace midi {
-class MidiManager;
+class MidiService;
 }  // namespace midi
 
 namespace mojo {
@@ -145,7 +145,7 @@ class CONTENT_EXPORT BrowserMainLoop {
   device::TimeZoneMonitor* time_zone_monitor() const {
     return time_zone_monitor_.get();
   }
-  midi::MidiManager* midi_manager() const { return midi_manager_.get(); }
+  midi::MidiService* midi_service() const { return midi_service_.get(); }
   base::Thread* indexed_db_thread() const { return indexed_db_thread_.get(); }
 
   bool is_tracing_startup_for_duration() const {
@@ -291,7 +291,7 @@ class CONTENT_EXPORT BrowserMainLoop {
   std::unique_ptr<AudioDeviceThread> audio_thread_;
   media::ScopedAudioManagerPtr audio_manager_;
 
-  std::unique_ptr<midi::MidiManager> midi_manager_;
+  std::unique_ptr<midi::MidiService> midi_service_;
 
 #if defined(OS_WIN)
   std::unique_ptr<media::SystemMessageWindowWin> system_message_window_;

[Takashi Toyoshima, 2016-12-15 04:31:14]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-12-15 04:31:15]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2566673002/#ps100001
(title: "merge master")

[commit-bot: I haz the power, 2016-12-15 04:32:03]

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-12-15 06:18:36]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481776274609450,
"parent_rev": "c80dccaf9cf69e976ca87ff533b27fac7beb5a2d", "commit_rev":
"d8858c1983985c5c8c9f8d63139dbeda0ba86864"}

[commit-bot: I haz the power, 2016-12-15 06:19:04]

Description was changed from

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

See crbug.com/672793 for rough design and plan.

BUG=672793
==========

to

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

See crbug.com/672793 for rough design and plan.

BUG=672793

Review-Url: https://codereview.chromium.org/2566673002
==========

[commit-bot: I haz the power, 2016-12-15 06:19:06]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-12-15 06:22:36]

Description was changed from

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

See crbug.com/672793 for rough design and plan.

BUG=672793

Review-Url: https://codereview.chromium.org/2566673002
==========

to

==========
Web MIDI: introduce MidiService class

In this patch, the class does nothing interesting,
but would have a dynamic instance management in
following changes.

Migration would happen per platform behind a
field study.

See crbug.com/672793 for rough design and plan.

BUG=672793

Committed: https://crrev.com/967340a92a250683594345735212482575c720ae
Cr-Commit-Position: refs/heads/master@{#438765}
==========

[commit-bot: I haz the power, 2016-12-15 06:22:38]

Patchset 6 (id:??) landed as
https://crrev.com/967340a92a250683594345735212482575c720ae
Cr-Commit-Position: refs/heads/master@{#438765}