New issue
Advanced search Search tips

Issue 781220 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

onmidimessage not being fired for some devices (Quirkbot)

Reported by pa...@azucrina.org, Nov 3 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.75 Safari/537.36

Steps to reproduce the problem:
1. Plug in the usb midi compliant device (Quirkbot - http://strawbees.com/product/quirkbot/) - the device firmware is programmed to send a midi message every time it receives one (echo).
2. Run code on the console that add listeners to all midi inputs and logs when a message is received.
3. Run code to send a midi message to all the midi outputs.
4. Observe the console to see if messages are logged

What is the expected behavior?
We should be able to see incoming midi messages, with the same payload as the sent messages.

What went wrong?
The midi messages are never received.

However MIDIMonitor (https://www.snoize.com/MIDIMonitor/) and a Node.js program (attached here) can receive the message just fine, indicating that the problem is not on the device.

Attached a screenshot of the error, and a counter example, where with another device, the messages are received in both Chrome and MIDIMonitor.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 62.0.3202.75  Channel: stable
OS Version: OS X 10.12.6
Flash Version: 

- Tested with a another device (in fact the same hardware, but running a different firmware) and then messages were received both in Chrome and MIDIMonitor.

- Tested on Chromebook and Windows, and did not have the problem there: messages were received.

- The device is midi compliant, I represent the manufacturer (Strawbees) and have personally wrote the software that runs on the board. I have extensive knowledge of the inner workings of the board and how to interact with it via usb.

- We are planning on using WebMIDI as a communication channel for the device (already deployed on tens of thousands of units) so this problem is a big deal for us.
 
midi-echo.js
530 bytes View Download
error.png
355 KB View Download
correct.png
431 KB View Download
node-monitor.js
214 bytes View Download
Labels: Needs-Triage-M62
Labels: -Pri-2 Needs-Feedback Pri-3
Is this really firmware specific issue?
If so, is it possible to clarify what is different on the firmware that isn't compatible with Web MIDI?

It sounds very hard to identify the problem without reproducing the problem locally. But could you try some investigations for my best guess?

1. Could you provide complete content of chrome://version on failure case? Currently, we have two Web MIDI running modes in the OS X port, and the problem might depend on the mode. "Variations" information at the version page allows me to identify which mode runs on your machine.

2. Could you check MIDIPort.state and MIDIPort.connection of the input port that can not receive the expected messages? The .state should be "open" to listen incoming MIDI messages. This should be automatically changed to "open" when the event handler is attached, in your case, on setting a function to onmidimessage. But I'm afraid that it gets to be another state due to some hardware state changes for some reasons. In this case, you can workaround this by calling MIDIPort.open() explicitly to reopen the device to listen messages.

Also since this is not a regression bug, device-specific, and I haven't seen a similar problem on other devices until today, let me set P3 until we understand what is wrong.
Labels: -Hotlist-Interop -Needs-Triage-M62
Remove Needs-Triage-M62 since this is not a regression.

Also, Hotlist-Interop should be a wrong label. This is automatically set because the report says "Does this work in other browsers? Yes". But since other browsers do not implement Web MIDI yet, this should not be "Yes".

Comment 4 by pa...@azucrina.org, Nov 22 2017

> #2

It seems that the issue only happens when chrome://flags/#enable-midi-manager-dynamic-instantiation is DISABLED.

Ports are open and connected on both cases.

Google Chrome	62.0.3202.94 (Official Build) (64-bit)
Revision	4fd852a98d66564c88736c017b0a0b0478e885ad-refs/branch-heads/3202@{#789}
OS	Mac OS X
JavaScript	V8 6.2.414.42
Flash	27.0.0.187 /Users/paulobarcelos/Library/Application Support/Google/Chrome/PepperFlash/27.0.0.187/PepperFlashPlayer.plugin
User Agent	Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36
Command Line	/Applications/Google Chrome.app/Contents/MacOS/Google Chrome --flag-switches-begin --disable-features=MidiManagerDynamicInstantiation --flag-switches-end
Executable Path	/Applications/Google Chrome.app/Contents/MacOS/Google Chrome
Profile Path	/Users/paulobarcelos/Library/Application Support/Google/Chrome/Default
Variations	c134752e-ca7d8d80
c68ab9a3-3f4a17df
3095aa95-3f4a17df
7c1bc906-f55a7974
47e5d3db-3d47f4f4
1210a805-ecd831c
b1edbc38-ca7d8d80
d43bf3e5-bd7cd813
ba3f87da-45bda656
79616653-3f4a17df
ed7ba060-91c810ef
9e201a2b-6e3ce1c
5b3ed0a1-3f4a17df
68812885-4d2fac87
f347910c-3d47f4f4
9773d3bd-f23d1dea
99144bc3-3cc2175e
9e5c75f1-2ad3bd2f
e79c5ffa-28ad44a
f79cb77b-3d47f4f4
27219e67-b2047178
4ea303a6-ecbb250e
d92562a9-ca7d8d80
1c2f7bbf-ca7d8d80
b2f0086-93053e47
ef25c1eb-ca7d8d80
494d8760-21549ebe
f47ae82a-86f22ee5
3ac60855-486e2a9c
f296190c-2502111b
4442aae2-a90023b1
ed1d377-e1cc0f14
75f0f0a0-d7f6b13c
e2b18481-a5822863
e7e71889-e1cc0f14
94e68624-803f8fc4
828a5926-9d7acf42
da4aaa01-ca7d8d80
Project Member

Comment 5 by sheriffbot@chromium.org, Nov 22 2017

Cc: toyoshim@chromium.org
Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "toyoshim@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by pa...@azucrina.org, Nov 22 2017

After exploring further, had some interesting findings...

If chrome://flags/#enable-midi-manager-dynamic-instantiation is ENABLED, I only receive messages from devices that are connected BEFORE I requestMIDIAccess for the first time.

If I first requestMIDIAccess, and then connect the device, the messages are never received.

This should be very easy to reproduce if you have access to a midi device (I’ve tested both with an Arduino micro programmed to send random messages every 1 second).

Steps to reproduce the problem:
- Reload page (or just open a new tab)
- requestMIDIAccess
- Connect the device to the computer
- add a listener to incoming messages
- send messages from the device
- The messages are never received…

However, this works…
- Reload page (or just open a new tab)
- Connect the device to the computer
- requestMIDIAccess
- add a listener to incoming messages
- send messages from the device
- The messages arrive!
Thanks. There is a similar report from Marshall,  http://crbug.com/786950 .
I will investigate it when I have their device. Your report will help me to jump in.
Owner: toyoshim@chromium.org
Hum... a bad news is we are moving to use DISABLED mode on stable and beta channels to avoid Core MIDI level problems, and will use hard coded DISABLED mode on dev and canary. But the canary mode will be modified slightly from the original DISABLED mode. So results may be changed.

I will investigate Marshall case first. Hope the fix for the Marshall also remove this problem.

Comment 10 by pa...@azucrina.org, Nov 24 2017

> #9
When can we expect to see this modified version on canary so we can test?
If you are interested, we can also send you a Quirkbot device to experiment with. 


I will ping you here when the code is checked into the Canary. The code is still on code review.
https://chromium-review.googlesource.com/c/chromium/src/+/788721

For the first page that uses Web MIDI, the behavior should be completely same with the ENABLED mode, but the backend will not finalize the Core MIDI even after all pages stop using Web MIDI, and reuses the same Core MIDI client for the next Web MIDI use.
Labels: -Pri-3 Pri-1
Status: Started (was: Unconfirmed)
I succeeded to reproduce the issue. Let me take a further look.
Labels: M-63
OK, I found the root cause.... It was a stupid bug introduced at m62.
Hope the fix can be approved to be merged in m63 beta branch.

https://chromium-review.googlesource.com/c/chromium/src/+/789211 in CQ.
Cc: ricea@chromium.org yhirano@chromium.org
Labels: ReleaseBlock-Stable
Not sure if I am a right person to set this flag, but just in case.

Cc-ing several falks;
yhirano@ who reviewed the original change
ricea@ who just reviewed my fix for a quick merge.
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 24 2017

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

commit de45d3ea7491f9a7ef8d4488f4f70d5f5ae2c7b8
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Fri Nov 24 17:44:04 2017

Web MIDI: MidiManagerMac should calculate index beforehands

The newly connected input port needs to calculate the device index,
it - sources_.begin(), but it should be done before pushing the new
instance to the sources_ because it invalidate the iterator.

Bug:  781220 
Change-Id: Id41bb2f006e88390a1c94c84d2f8ee54e03fd592
Reviewed-on: https://chromium-review.googlesource.com/789211
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519141}
[modify] https://crrev.com/de45d3ea7491f9a7ef8d4488f4f70d5f5ae2c7b8/media/midi/midi_manager_mac.cc

Labels: Merge-Request-63
Project Member

Comment 18 by sheriffbot@chromium.org, Nov 24 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: We are only 10 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
This is a regression and can break third party's web service that relies on Web MIDI API on macOS.

The issue was a trivial mistake that uses an iterator after an invalidation, and fix is very trivial and safe to be merged.

https://chromium-review.googlesource.com/c/chromium/src/+/789211/2/media/midi/midi_manager_mac.cc
> paulo

The last Canary for mac should include my fix. Please check it on Chrome Canary 64.0.3278.0 or later.

Comment 21 by pa...@azucrina.org, Nov 27 2017

> #20
Great! I can confirm that it works as it should. Thanks!

Comment 22 by cmasso@google.com, Nov 27 2017

Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
Merge approved! Please make sure to verify the fix on branch 3239 after merging it.
Project Member

Comment 23 by bugdroid1@chromium.org, Nov 28 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/52868cae7fcd067f975d20b00b091b52a3345546

commit 52868cae7fcd067f975d20b00b091b52a3345546
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Tue Nov 28 06:10:40 2017

Web MIDI: MidiManagerMac should calculate index beforehands

The newly connected input port needs to calculate the device index,
it - sources_.begin(), but it should be done before pushing the new
instance to the sources_ because it invalidate the iterator.

Bug:  781220 
Change-Id: Id41bb2f006e88390a1c94c84d2f8ee54e03fd592
Reviewed-on: https://chromium-review.googlesource.com/789211
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#519141}(cherry picked from commit de45d3ea7491f9a7ef8d4488f4f70d5f5ae2c7b8)
Reviewed-on: https://chromium-review.googlesource.com/792711
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#578}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/52868cae7fcd067f975d20b00b091b52a3345546/media/midi/midi_manager_mac.cc

Status: Fixed (was: Started)
the fix is merged to m63 release branch.
Status: Verified (was: Fixed)
I just confirmed that the problem was correctly fixed in the official Beta binary too!
For record, the version was 63.0.3239.70 (Official Build) beta

Sign in to add a comment