New issue
Advanced search Search tips

Issue 672793 link

Starred by 7 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Task

Blocked on:
issue 751854

Blocking:
issue 582328
issue 486584
issue 497573
issue 517950
issue 617086
issue 631933
issue 645403
issue 660197
issue 665571
issue 694522
issue 718140



Sign in to add a comment

Web MIDI: MidiManager design change

Project Member Reported by toyoshim@chromium.org, Dec 9 2016

Issue description

At this point, MidiManager is designed to run on multiple-threads, and to live forever until the browser is going to be shutdown.

This design has two big problems.

1. All platform dependent implementation need to consider multi-threading on the browser main thread and I/O thread carefully.
2. It's hard to destruct and re-construct MidiManager

And, we have two use cases for the problem 2.

1. Destruct MidiManager when no clients use it (for a battery-life concern)
2. Dynamic backend switch on Android and Windows (because new backends seem to have user-environment dependent problems, and need switching to fail-safe backend)

New design idea is

- BrowserMainLoop owns MidiService (would be a mojo service eventually)
- BrowserMainLoop is instantiated on the browser main thread
- Platform dependent part is implemented as a MidiManager (as we do now)
- But the new MidiManager run only on the I/O thread, that means MidiService would lazily instantiate it on the I/O thread.
- MidiService would destruct the MidiManager when all clients are detached, and reconstruct another MidiManager when it is requested again.
 
Blocking: 486584 665571 645403 497573 617086
Labels: -Pri-3 Pri-2
Owner: toyoshim@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/967340a92a250683594345735212482575c720ae

commit 967340a92a250683594345735212482575c720ae
Author: toyoshim <toyoshim@chromium.org>
Date: Thu Dec 15 06:18:29 2016

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
Cr-Commit-Position: refs/heads/master@{#438765}

[modify] https://crrev.com/967340a92a250683594345735212482575c720ae/content/browser/browser_main_loop.cc
[modify] https://crrev.com/967340a92a250683594345735212482575c720ae/content/browser/browser_main_loop.h
[modify] https://crrev.com/967340a92a250683594345735212482575c720ae/content/browser/media/midi_host.cc
[modify] https://crrev.com/967340a92a250683594345735212482575c720ae/content/browser/media/midi_host.h
[modify] https://crrev.com/967340a92a250683594345735212482575c720ae/content/browser/media/midi_host_unittest.cc
[modify] https://crrev.com/967340a92a250683594345735212482575c720ae/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/967340a92a250683594345735212482575c720ae/media/midi/BUILD.gn
[modify] https://crrev.com/967340a92a250683594345735212482575c720ae/media/midi/midi_manager.h
[add] https://crrev.com/967340a92a250683594345735212482575c720ae/media/midi/midi_service.cc
[add] https://crrev.com/967340a92a250683594345735212482575c720ae/media/midi/midi_service.h

Comment 3 by mdri...@gmail.com, Jan 16 2017

This should probably be P1, as it's blocking a P1 bug.
Cc: palmer@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f1c3f34031ae08400afbe43c70fd71267a870b1

commit 0f1c3f34031ae08400afbe43c70fd71267a870b1
Author: toyoshim <toyoshim@chromium.org>
Date: Fri Feb 10 10:03:32 2017

Web MIDI: add dynamic MidiManager instantiation support for Linux

With this change, MidiService starts supporting dynamic MidiManager
instantiation. First, this enables it for Linux for a trial.

BUG=672793

Review-Url: https://codereview.chromium.org/2673423002
Cr-Commit-Position: refs/heads/master@{#449574}

[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/chrome/app/generated_resources.grd
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/chrome/browser/about_flags.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/content/browser/media/midi_host_unittest.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_alsa.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_alsa.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_android.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_android.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_mac.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_mac.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_mac_unittest.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_unittest.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_usb.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_usb.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_usb_unittest.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_win.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_win.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_winrt.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_manager_winrt.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_service.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_service.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_switches.cc
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/media/midi/midi_switches.h
[modify] https://crrev.com/0f1c3f34031ae08400afbe43c70fd71267a870b1/tools/metrics/histograms/histograms.xml

Blocking: 694138
Blocking: 631933
Blocking: 694522
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e262421e78d33170893fd1985df28658ba45a811

commit e262421e78d33170893fd1985df28658ba45a811
Author: toyoshim <toyoshim@chromium.org>
Date: Fri Feb 24 10:13:14 2017

Web MIDI: device open/close for dynamic manager instantiation on Windows

This patch implements functionalities to open and close devices.
Closing operations will be performed outside the manager instance
so that the manager does not block I/O thread. But it would block
next instance's initialization correctly.

BUG= 497573 ,  617086 , 672793

Review-Url: https://codereview.chromium.org/2701503005
Cr-Commit-Position: refs/heads/master@{#452786}

[modify] https://crrev.com/e262421e78d33170893fd1985df28658ba45a811/media/midi/dynamically_initialized_midi_manager_win.cc

Blocking: 660197
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d4bb28c8ba6e8bf8f53879aa02f0c823a0aa98d

commit 6d4bb28c8ba6e8bf8f53879aa02f0c823a0aa98d
Author: toyoshim <toyoshim@chromium.org>
Date: Wed Mar 01 05:16:18 2017

Web MIDI: implement receiving for dynamic manager instantiation on Windows

This patch makes input ports work perfectly.

BUG= 497573 ,  617086 , 672793

Review-Url: https://codereview.chromium.org/2701783003
Cr-Commit-Position: refs/heads/master@{#453850}

[modify] https://crrev.com/6d4bb28c8ba6e8bf8f53879aa02f0c823a0aa98d/media/midi/dynamically_initialized_midi_manager_win.cc
[modify] https://crrev.com/6d4bb28c8ba6e8bf8f53879aa02f0c823a0aa98d/media/midi/dynamically_initialized_midi_manager_win.h

Project Member

Comment 13 by bugdroid1@chromium.org, Mar 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eaeaadec00a39dcd1c725fc1e5ce1fda7cccd185

commit eaeaadec00a39dcd1c725fc1e5ce1fda7cccd185
Author: toyoshim <toyoshim@chromium.org>
Date: Wed Mar 01 06:36:44 2017

Web MIDI: implement sending for dynamic manager instantiation on Windows

This patch makes output ports work perfectly.

BUG= 497573 ,  617086 , 672793

Review-Url: https://codereview.chromium.org/2698413003
Cr-Commit-Position: refs/heads/master@{#453866}

[modify] https://crrev.com/eaeaadec00a39dcd1c725fc1e5ce1fda7cccd185/media/midi/dynamically_initialized_midi_manager_win.cc
[modify] https://crrev.com/eaeaadec00a39dcd1c725fc1e5ce1fda7cccd185/media/midi/dynamically_initialized_midi_manager_win.h

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d4acd504da2e43ca94a5fe97d14ec01e0f15a69d

commit d4acd504da2e43ca94a5fe97d14ec01e0f15a69d
Author: toyoshim <toyoshim@chromium.org>
Date: Wed Mar 01 08:40:31 2017

Web MIDI: extract manufacturer names on some devices

This does not help us in most cases on Windows 10, but
let me introduce the same code with the legacy backend.

BUG= 497573 ,  617086 , 672793

Review-Url: https://codereview.chromium.org/2715823003
Cr-Commit-Position: refs/heads/master@{#453884}

[modify] https://crrev.com/d4acd504da2e43ca94a5fe97d14ec01e0f15a69d/media/midi/dynamically_initialized_midi_manager_win.cc

Labels: M-59 OS-Android OS-Mac
Status: Started (was: Assigned)
Linux, CrOS, and Windows backend already support the new design and enabled behind a field trial flag on m58.

Other platforms us planned to support it on m59.
Labels: -Type-Bug Type-Task
Project Member

Comment 17 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/efb905341311f9d9dc5ad9a53f6cfe7c0859d18c

commit efb905341311f9d9dc5ad9a53f6cfe7c0859d18c
Author: toyoshim <toyoshim@chromium.org>
Date: Wed Mar 08 03:10:56 2017

Enable MidiManagerDynamicInstantiation feature in variations test

BUG=694138, 672793

Review-Url: https://codereview.chromium.org/2735973002
Cr-Commit-Position: refs/heads/master@{#455363}

[modify] https://crrev.com/efb905341311f9d9dc5ad9a53f6cfe7c0859d18c/testing/variations/fieldtrial_testing_config.json

Blocking: 517950
For Linux, ChromeOS, and Windows,

Now 50% of dev and canary users are using the new mode at m58+, and I already set up server side configurations for beta channels to enable it for 50% users at m58.
Components: Blink>WebMIDI
Project Member

Comment 21 by bugdroid1@chromium.org, Apr 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3793199ef338f7a3f19ba5d6d35ceb8ba63c8f33

commit 3793199ef338f7a3f19ba5d6d35ceb8ba63c8f33
Author: toyoshim <toyoshim@chromium.org>
Date: Tue Apr 25 05:20:29 2017

Web MIDI dynamic instantiation experiment-controlled rollouts: step 4

Enable dynamic instantiation mode by default on Linux, ChromeOS,
and Windows. Other platforms still need to rely on the existing
experiment-controlled flag and will be launched separately.

BUG=672793, 714511 

Review-Url: https://codereview.chromium.org/2835083002
Cr-Commit-Position: refs/heads/master@{#466881}

[modify] https://crrev.com/3793199ef338f7a3f19ba5d6d35ceb8ba63c8f33/media/midi/midi_service.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4399414ef40580162c65f96e76b4055edbc6e590

commit 4399414ef40580162c65f96e76b4055edbc6e590
Author: toyoshim <toyoshim@chromium.org>
Date: Tue Apr 25 06:52:01 2017

Web MIDI: remove old Windows backend

The new backend supporting dynamic instantiation on Windows was rolled
out to the stable channel via a field trial flag.
This new backend solves some long-living problems, e.g., grabbing
devices forever, battery concerns, and stability.

Since the feature was enabled by default, and have worked well,
I want to remove the old backend now.

Another change that renames new backend will follow.

BUG=672793

Review-Url: https://codereview.chromium.org/2841493003
Cr-Commit-Position: refs/heads/master@{#466904}

[modify] https://crrev.com/4399414ef40580162c65f96e76b4055edbc6e590/media/midi/BUILD.gn
[modify] https://crrev.com/4399414ef40580162c65f96e76b4055edbc6e590/media/midi/OWNERS
[modify] https://crrev.com/4399414ef40580162c65f96e76b4055edbc6e590/media/midi/dynamically_initialized_midi_manager_win.cc
[delete] https://crrev.com/f0aff0ca64ae1f50b4deeea5966b0ad025323696/media/midi/midi_manager_win.cc
[delete] https://crrev.com/f0aff0ca64ae1f50b4deeea5966b0ad025323696/media/midi/midi_manager_win.h

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/60350512cce320ab73d0f1275fdd164cba5a37e7

commit 60350512cce320ab73d0f1275fdd164cba5a37e7
Author: toyoshim <toyoshim@chromium.org>
Date: Tue Apr 25 14:20:10 2017

Web MIDI: rename DynamicallyInitializedMidiManagerWin

Since the old backend was already removed, rename the new backend
DynamicallyInitializedMidiManagerWin to MidiManagerWin.

BUG=672793

Review-Url: https://codereview.chromium.org/2834423002
Cr-Commit-Position: refs/heads/master@{#466980}

[modify] https://crrev.com/60350512cce320ab73d0f1275fdd164cba5a37e7/media/midi/BUILD.gn
[rename] https://crrev.com/60350512cce320ab73d0f1275fdd164cba5a37e7/media/midi/midi_manager_win.cc
[rename] https://crrev.com/60350512cce320ab73d0f1275fdd164cba5a37e7/media/midi/midi_manager_win.h

Labels: OS-Chrome OS-Linux OS-Windows
yukawa: I just removed these labels because implementation was finished for these platforms. But maybe it's ok to keep them for a while to track experimental-controlled launch.

Update:
50%-50% A/B testing on beta channel showed pretty nice results. All coming crash reports for the m58 beta were for the old implementation.

As a next step, we enable the feature on ToT by default, and start A/B testing on the m59 stable channel with 1% users. We will change the enabled ratio to 100% soon.
Project Member

Comment 26 by bugdroid1@chromium.org, Apr 26 2017

Labels: merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/814d90a45d7608581b0e70e52abb964e713a6dba

commit 814d90a45d7608581b0e70e52abb964e713a6dba
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Wed Apr 26 07:33:07 2017

Web MIDI dynamic instantiation experiment-controlled rollouts: step 4

Enable dynamic instantiation mode by default on Linux, ChromeOS,
and Windows. Other platforms still need to rely on the existing
experiment-controlled flag and will be launched separately.

BUG=672793, 714511 
TBR=yhirano@chromium.org

Review-Url: https://codereview.chromium.org/2835083002
Cr-Commit-Position: refs/heads/master@{#466881}
(cherry picked from commit 3793199ef338f7a3f19ba5d6d35ceb8ba63c8f33)

Review-Url: https://codereview.chromium.org/2842963002 .
Cr-Commit-Position: refs/branch-heads/3071@{#218}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/814d90a45d7608581b0e70e52abb964e713a6dba/media/midi/midi_service.cc

Now the feature is enabled by default on Linux/ChromeOS/Windows an TOT without any flag.

On Stable channel, we started distributing enabled configuration for all users. All users will pick up the new backend in coming a few days.
Blocking: 718140
Project Member

Comment 29 by bugdroid1@chromium.org, Jun 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0492fc281b7eb603be6e7f3f00afb7d9802ef3d5

commit 0492fc281b7eb603be6e7f3f00afb7d9802ef3d5
Author: toyoshim <toyoshim@chromium.org>
Date: Tue Jun 13 12:31:46 2017

Web MIDI: implement TaskService

Current Web MIDI code base has platform specific implementation
that provides graceful asynchronous task handling on Linux and
Windows. It allows MidiManager to post member method to another
threads safely even if the MidiManager instance can be destructed
dynamically.

To expand this functionalities to all platform, I will introduce
a common infrastructure, TaskService, to be used on all platforms.

BUG=672793

Review-Url: https://codereview.chromium.org/2741713002
Cr-Commit-Position: refs/heads/master@{#478982}

[modify] https://crrev.com/0492fc281b7eb603be6e7f3f00afb7d9802ef3d5/media/midi/BUILD.gn
[modify] https://crrev.com/0492fc281b7eb603be6e7f3f00afb7d9802ef3d5/media/midi/midi_service.cc
[modify] https://crrev.com/0492fc281b7eb603be6e7f3f00afb7d9802ef3d5/media/midi/midi_service.h
[add] https://crrev.com/0492fc281b7eb603be6e7f3f00afb7d9802ef3d5/media/midi/task_service.cc
[add] https://crrev.com/0492fc281b7eb603be6e7f3f00afb7d9802ef3d5/media/midi/task_service.h
[add] https://crrev.com/0492fc281b7eb603be6e7f3f00afb7d9802ef3d5/media/midi/task_service_unittest.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Jun 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c73f9dc095133071e0d3dd918a9135443aeb8032

commit c73f9dc095133071e0d3dd918a9135443aeb8032
Author: toyoshim <toyoshim@chromium.org>
Date: Wed Jun 14 14:14:39 2017

Web MIDI: use midi::TaskService in MidiManagerAlsa

This patch replaces existing MidiManagerAlsa implementation
to support asynchronous method posts with midi::TaskService
that should be used on all platforms now to support such
functionalities.

BUG=672793

Review-Url: https://codereview.chromium.org/2923163003
Cr-Commit-Position: refs/heads/master@{#479374}

[modify] https://crrev.com/c73f9dc095133071e0d3dd918a9135443aeb8032/media/midi/midi_manager_alsa.cc
[modify] https://crrev.com/c73f9dc095133071e0d3dd918a9135443aeb8032/media/midi/midi_manager_alsa.h

Labels: -OS-Linux -OS-Windows -OS-Chrome -OS-Mac -M-59 M-62
Supported on mac at m62
https://bugs.chromium.org/p/chromium/issues/detail?id=694138#c8
Blockedon: 751854
Project Member

Comment 33 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/816c0dac0fde7f19a4f29f320cc311b5b619c759

commit 816c0dac0fde7f19a4f29f320cc311b5b619c759
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Thu Sep 07 08:30:18 2017

MidiService: introduce MidiManagerFactory for testing

MidiService can take MidiManager in constructor for testing, but
this does not make it possible to use MidiService instance in
unit tests.

To solve this problem, this patch introduces MidiManagerFactory
and change MidiService constructor to take MidiManagerFactory
instead of MidiManager for testing.

This will be needed to rewrite MidiManagerUsb to use TaskService.

Bug: 672793
Change-Id: Id7d50183cfbb3a8bc8d7857e51d434d91314c3f4
Reviewed-on: https://chromium-review.googlesource.com/643348
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500245}
[modify] https://crrev.com/816c0dac0fde7f19a4f29f320cc311b5b619c759/content/browser/media/midi_host_unittest.cc
[modify] https://crrev.com/816c0dac0fde7f19a4f29f320cc311b5b619c759/media/midi/midi_manager_unittest.cc
[modify] https://crrev.com/816c0dac0fde7f19a4f29f320cc311b5b619c759/media/midi/midi_service.cc
[modify] https://crrev.com/816c0dac0fde7f19a4f29f320cc311b5b619c759/media/midi/midi_service.h

Project Member

Comment 34 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a0e08af08ce2a3c60fefe89774e5f3a889635e5

commit 0a0e08af08ce2a3c60fefe89774e5f3a889635e5
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Mon Sep 11 10:09:44 2017

MidiManagerUsbTest: run tests with a MidiService

MidiManagerUsbTest runs without MidiService today, and MidiManagerUsb
can not rely on MidiService infrastructure, i.e. TaskService to pass
unit tests.

Recently MidiService::ManagerFactory was introduced, and it allows
MidiManagerUsbTest to run with MidiService.

This patch modifies MidiManagerUsbTest to use the ManagerFactory
and realize that MidiService can be available from MidiManagerUsb
even in unit tests. MidiManagerUsb side changes to use MidiService
will follow.

Bug: 672793
Change-Id: I60010d6344f976151935911bd0f28c1c58cea30a
Reviewed-on: https://chromium-review.googlesource.com/656750
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500876}
[modify] https://crrev.com/0a0e08af08ce2a3c60fefe89774e5f3a889635e5/media/midi/midi_manager_usb_unittest.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Sep 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/806606e8d5ca08b45f840b56f13e269774ee850c

commit 806606e8d5ca08b45f840b56f13e269774ee850c
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Wed Sep 13 10:02:54 2017

media/midi: use BindOnce and OnceCallback explicitly

Mechanical changes to use BindOnce and OnceCallback
explicitly at everywhere in media/midi.

Bug: 672793
Change-Id: Ibef3543d3b91f6617f5116a845b03e0ccb823629
Reviewed-on: https://chromium-review.googlesource.com/657357
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501585}
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_manager.cc
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_manager_android.cc
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_manager_usb.cc
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_manager_usb.h
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_manager_usb_unittest.cc
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_manager_win.cc
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_manager_win.h
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_manager_winrt.cc
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_scheduler.cc
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/midi_scheduler.h
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/task_service.h
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/task_service_unittest.cc
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/usb_midi_device.h
[modify] https://crrev.com/806606e8d5ca08b45f840b56f13e269774ee850c/media/midi/usb_midi_device_factory_android.cc

Project Member

Comment 36 by bugdroid1@chromium.org, Sep 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7eb8b9aa5d2ff6bfc7a02b4076c3e946fbabd31

commit e7eb8b9aa5d2ff6bfc7a02b4076c3e946fbabd31
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Wed Sep 13 11:50:41 2017

MidiService::TimestampToTimeDeltaDelay

Provide TimestampToTimeDeltaDelay as a static helper method of
MidiService, and use it to calculate a delay time for a timestamp
to post a delayed task.

Bug: 672793
Change-Id: I7307bb816c262073ceae557ad9345c451051d414
Reviewed-on: https://chromium-review.googlesource.com/656924
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501595}
[modify] https://crrev.com/e7eb8b9aa5d2ff6bfc7a02b4076c3e946fbabd31/media/midi/midi_manager_alsa.cc
[modify] https://crrev.com/e7eb8b9aa5d2ff6bfc7a02b4076c3e946fbabd31/media/midi/midi_manager_win.cc
[modify] https://crrev.com/e7eb8b9aa5d2ff6bfc7a02b4076c3e946fbabd31/media/midi/midi_scheduler.cc
[modify] https://crrev.com/e7eb8b9aa5d2ff6bfc7a02b4076c3e946fbabd31/media/midi/midi_scheduler.h
[modify] https://crrev.com/e7eb8b9aa5d2ff6bfc7a02b4076c3e946fbabd31/media/midi/midi_service.cc
[modify] https://crrev.com/e7eb8b9aa5d2ff6bfc7a02b4076c3e946fbabd31/media/midi/midi_service.h

Project Member

Comment 37 by bugdroid1@chromium.org, Sep 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5be46d3ff6d9f048e063445c2571f69e42e64601

commit 5be46d3ff6d9f048e063445c2571f69e42e64601
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Thu Sep 14 09:24:40 2017

MidiManagerUsb: use TaskService to support dynamic instantiation mode

To enable dynamic instantiation mode even on Android, this patch
modifies MidiManagerUsb to use TaskService instead of MidiScheduler.

This will allow to run async tasks safely even on dynamic instantiation
environments.

Also this patch gathers timestamp to delta delay calculation code
into MidiService. Eventually, we stop sharing implementation behind
the parent MidiManager class, but have common code or services in
MidiService as much as possible.

Bug: 672793
Change-Id: Ic6b5285562655fed81673fb69b6eca17db067574
Reviewed-on: https://chromium-review.googlesource.com/645606
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501910}
[modify] https://crrev.com/5be46d3ff6d9f048e063445c2571f69e42e64601/media/midi/midi_manager_usb.cc
[modify] https://crrev.com/5be46d3ff6d9f048e063445c2571f69e42e64601/media/midi/midi_manager_usb.h

Project Member

Comment 38 by bugdroid1@chromium.org, Sep 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d3e6e645dc73e13a855f06aa0ad851565ca2a03

commit 8d3e6e645dc73e13a855f06aa0ad851565ca2a03
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Mon Sep 25 06:47:48 2017

MidiManagerAndroid: use TaskService to support dynamic instantiation mode

To enable dynamic instantiation mode even on Android, this patch
modifies MidiManagerAndroid to use TaskService instead of MidiScheduler.

This will allow to run async tasks safely even on dynamic instantiation
environments.

Bug: 672793
Change-Id: Ic5eca40390d68f0f6967087633387c4cff0efa24
Reviewed-on: https://chromium-review.googlesource.com/654569
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503991}
[modify] https://crrev.com/8d3e6e645dc73e13a855f06aa0ad851565ca2a03/media/midi/midi_manager_android.cc
[modify] https://crrev.com/8d3e6e645dc73e13a855f06aa0ad851565ca2a03/media/midi/midi_manager_android.h

Project Member

Comment 39 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3257f3ad6db041623950339354dd6a52273a7699

commit 3257f3ad6db041623950339354dd6a52273a7699
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Thu Sep 28 10:41:36 2017

MidiManagerAndroid: remove unnecessarily called UnbindInstance()

UnbindInstance() is called in Finalize(), and should not be called
in the destructor. This redundant call is just ignored and does not
cause a real problem here, but DCHECK will fire.

Bug: 672793
Change-Id: Ic674f38ecc7e0796302cdf135a06492b5ee048a3
Reviewed-on: https://chromium-review.googlesource.com/689362
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504960}
[modify] https://crrev.com/3257f3ad6db041623950339354dd6a52273a7699/media/midi/midi_manager_android.cc

Project Member

Comment 40 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0fd5dc1d83ccab2a21cb727c26d678c1a3edd0c9

commit 0fd5dc1d83ccab2a21cb727c26d678c1a3edd0c9
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Fri Oct 13 06:04:48 2017

Web MIDI: enable dynamic instantiation mode by default

To prepare for dynamic instantiation launch on macOS, flip the
default behavior to use the dynamic instantiation mode by default
even on macOS and Android.

The feature will be launched at m62 on macOS, and at 63 on Android.

Bug: 672793
Change-Id: Ic4f8dc985c07cd388842ad72381bae1be078ae62
Reviewed-on: https://chromium-review.googlesource.com/712494
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508614}
[modify] https://crrev.com/0fd5dc1d83ccab2a21cb727c26d678c1a3edd0c9/media/midi/midi_switches.cc

Blocking: 582328
Labels: -OS-Android OS-Windows
All enabled backends support dynamic instantiation mode now, but still WinRT backend does not.
Blocking: -694138
Project Member

Comment 44 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cc949e9877ba93f05feaaf0bc5f75cb90365b68

commit 3cc949e9877ba93f05feaaf0bc5f75cb90365b68
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Tue Dec 19 09:08:34 2017

MidiManager: remove dynamic instantiation study

Now that the mode is enabled on all platforms, let's remove
the study flag, and enable it always on production code.

Unit tests still use the legacy mode in some cases, but it
should be modified in follow-up changes soon.

Bug: 672793
Change-Id: I99cf9d15e9cf8c3305d5bf73f2e7350159f45e56
Tbr: mpearson@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/821803
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524976}
[modify] https://crrev.com/3cc949e9877ba93f05feaaf0bc5f75cb90365b68/chrome/browser/about_flags.cc
[modify] https://crrev.com/3cc949e9877ba93f05feaaf0bc5f75cb90365b68/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/3cc949e9877ba93f05feaaf0bc5f75cb90365b68/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/3cc949e9877ba93f05feaaf0bc5f75cb90365b68/media/midi/midi_service.cc
[modify] https://crrev.com/3cc949e9877ba93f05feaaf0bc5f75cb90365b68/media/midi/midi_switches.cc
[modify] https://crrev.com/3cc949e9877ba93f05feaaf0bc5f75cb90365b68/media/midi/midi_switches.h
[modify] https://crrev.com/3cc949e9877ba93f05feaaf0bc5f75cb90365b68/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 45 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa56d65151e1b2325aee2ca721750d1c06122104

commit aa56d65151e1b2325aee2ca721750d1c06122104
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Tue Jan 09 08:26:21 2018

Web MIDI: remove dynamic instantiation field study code

This patch removes dynamic instantation mode flag from the
MidiService implementation, and adopts dynamic instantiation
mode for all existing unit tests.

This patch tries to minimize production code changes other than
just removing the flag so that we should not overlook wrong
code changes during modifying unit tests.

Follow-up changes will remove unnecessary structures and
threading support code from MidiService and MidiManager.

Bug: 672793
Change-Id: Ife34c9cb742a974f6e559605003c13d020b4a091
Reviewed-on: https://chromium-review.googlesource.com/822630
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527937}
[modify] https://crrev.com/aa56d65151e1b2325aee2ca721750d1c06122104/content/browser/media/midi_host_unittest.cc
[modify] https://crrev.com/aa56d65151e1b2325aee2ca721750d1c06122104/media/midi/midi_manager_unittest.cc
[modify] https://crrev.com/aa56d65151e1b2325aee2ca721750d1c06122104/media/midi/midi_manager_usb_unittest.cc
[modify] https://crrev.com/aa56d65151e1b2325aee2ca721750d1c06122104/media/midi/midi_service.cc
[modify] https://crrev.com/aa56d65151e1b2325aee2ca721750d1c06122104/media/midi/midi_service.h

Project Member

Comment 46 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71a10a33cd1ed8d8487eba87ec16fed7d6ef21c2

commit 71a10a33cd1ed8d8487eba87ec16fed7d6ef21c2
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Tue Jan 09 12:33:52 2018

MidiManager: cleanup unnecessary code that was for legacy mode

After the dynamic instantiation mode was enabled by default,
MidiManager construction, destruction, and public interfaces are
called on the I/O thread. This can simplify Shutdown() to run
synchronously.

Bug: 672793
Change-Id: I872bd5d06aa8cb984fb6867647ed0adf921e1d78
Reviewed-on: https://chromium-review.googlesource.com/826346
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527973}
[modify] https://crrev.com/71a10a33cd1ed8d8487eba87ec16fed7d6ef21c2/media/midi/midi_manager.cc
[modify] https://crrev.com/71a10a33cd1ed8d8487eba87ec16fed7d6ef21c2/media/midi/midi_manager.h

Project Member

Comment 47 by bugdroid1@chromium.org, Jan 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62dd3d6d8724bc44f1991ef476064cc8c975dcba

commit 62dd3d6d8724bc44f1991ef476064cc8c975dcba
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Wed Jan 17 09:19:59 2018

MidiManagerWinrt: Use midi::TaskService instead of MidiScheduler

To support dynamic instantiation mode even on this experimental
backend, use midi::TaskService instead of MidiScheduler.

Since this is the last consumer of MidiScheduler, this patch also
remove the MidiScheduler implementation from media/midi.

Bug: 672793
Change-Id: Ie50aa5ba403fb4c8a3ed28b18fb526ccad217d91
Reviewed-on: https://chromium-review.googlesource.com/839700
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529681}
[modify] https://crrev.com/62dd3d6d8724bc44f1991ef476064cc8c975dcba/media/midi/BUILD.gn
[modify] https://crrev.com/62dd3d6d8724bc44f1991ef476064cc8c975dcba/media/midi/midi_manager_winrt.cc
[modify] https://crrev.com/62dd3d6d8724bc44f1991ef476064cc8c975dcba/media/midi/midi_manager_winrt.h
[delete] https://crrev.com/c0ad29faf608449f55d35b8c5ddfc74139ab508a/media/midi/midi_scheduler.cc
[delete] https://crrev.com/c0ad29faf608449f55d35b8c5ddfc74139ab508a/media/midi/midi_scheduler.h
[modify] https://crrev.com/62dd3d6d8724bc44f1991ef476064cc8c975dcba/media/midi/task_service.cc
[modify] https://crrev.com/62dd3d6d8724bc44f1991ef476064cc8c975dcba/media/midi/task_service.h
[modify] https://crrev.com/62dd3d6d8724bc44f1991ef476064cc8c975dcba/media/midi/task_service_unittest.cc

Project Member

Comment 48 by bugdroid1@chromium.org, Sep 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01567e2e304544d17f45a97ca374ee97df2edf9a

commit 01567e2e304544d17f45a97ca374ee97df2edf9a
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Thu Sep 27 11:20:52 2018

MidiManager: Fix a potential race on macOS

CompleteInitialization can be posted at a place after DeleteSoon's
task, and it can potentially cause a UAF crash. This happens only on
browser's shutdown sequence, and it won't practically. But just in case
and to make ASAN bots happy:)

Bug:  880665 , 672793
Change-Id: I8435290b4df7068d456368624935d3007a2c52d7
Reviewed-on: https://chromium-review.googlesource.com/1238297
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594667}
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_manager.cc
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_manager.h
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_manager_android.cc
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_manager_mac.cc
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_manager_mac.h
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_manager_usb.cc
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_manager_usb.h
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_manager_winrt.cc
[modify] https://crrev.com/01567e2e304544d17f45a97ca374ee97df2edf9a/media/midi/midi_service.cc

Project Member

Comment 49 by bugdroid1@chromium.org, Oct 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6c85490c4fa9fe5baed9465ef8b8475b0607048

commit c6c85490c4fa9fe5baed9465ef8b8475b0607048
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Mon Oct 01 07:43:39 2018

MidiManager: Trivial cleanups

This patch makes several trivial changes:
 - Enforce GUARDED_BY against |clients_| and modify test only
   methods for that.
 - Initialize members in the header rather than constructor
   as we can as possible.
 - |finalized_| and |initialization_state_| do not have to be
   protected by |lock_|.
 - Enable GUARDED_BY in MidiManagerWinrt.

Bug: 672793
Change-Id: I726882c502b1d5b2414c5baa33ca31c3f630072e
Reviewed-on: https://chromium-review.googlesource.com/1249061
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595398}
[modify] https://crrev.com/c6c85490c4fa9fe5baed9465ef8b8475b0607048/media/midi/midi_manager.cc
[modify] https://crrev.com/c6c85490c4fa9fe5baed9465ef8b8475b0607048/media/midi/midi_manager.h
[modify] https://crrev.com/c6c85490c4fa9fe5baed9465ef8b8475b0607048/media/midi/midi_manager_unittest.cc
[modify] https://crrev.com/c6c85490c4fa9fe5baed9465ef8b8475b0607048/media/midi/midi_manager_winrt.cc
[modify] https://crrev.com/c6c85490c4fa9fe5baed9465ef8b8475b0607048/media/midi/midi_manager_winrt.h

Project Member

Comment 50 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9151f22fca5ac0021a522fcba015d6b1de5dd4b8

commit 9151f22fca5ac0021a522fcba015d6b1de5dd4b8
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Wed Oct 03 09:30:11 2018

MidiManager: Deprecate Shutdown and Finalize methods

Now that the dynamic instantiation mode is the only mode that
MidiService and MidiManager support, we can ensure that any
MidiManager instance will be constructed and destructed on the
same IO thread. This will allow us to remove complicated shutdown
system that relies on Shutdown() and Finalize(). Now destructor
should just work.

Bug: 672793
Change-Id: I50cc843dd997f9de68492c13d7d845810020dcb9
Reviewed-on: https://chromium-review.googlesource.com/c/1249421
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596163}
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager.cc
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager.h
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_alsa.cc
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_alsa.h
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_android.cc
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_android.h
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_mac.cc
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_mac.h
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_unittest.cc
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_usb.cc
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_usb.h
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_win.cc
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_win.h
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_winrt.cc
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_manager_winrt.h
[modify] https://crrev.com/9151f22fca5ac0021a522fcba015d6b1de5dd4b8/media/midi/midi_service.cc

Sign in to add a comment