This code has been refactored recently so a regression is possible.

Hi, art.ermolin@, thanks for the report, could you help me to understand more about this issue? I suppose we have 2 problems as following?
  1, connect(port) --> got connection_id1
     disconnect(connection_id1) OK,
     then connect(port) again fails? Must unplug-plug cable to recover?
  2, connect(port) --> got connection_id1,
     connect(port) again --> got connection_id2,
     then disconnect(connection_id1) and disconnect(connection_id2) OK,
     then getConnections(cb), the cb will never be triggered?

Hi! Thanks for reply.

Our device works through FTDI driver.
In eralier version of chrome problem with unplug-plug was present and i think its our device interface implementation bug.
How it was:
   1. connect(port1, settings) --> got connection_id1, read/write works normally
     settings:
         bitrate: 115200,
         bufferSize: 10000,
         persistent: true, 
         sendTimeout: 10000,
         receiveTimeout: 0
   2. disconnect(connection_id1) --> OK # device not unplugged
   3. connect(port1, settings) --> got connection_id2
   4. read(connection_id2)/write(connection_id2) --> not working
   5. getInfo(connection_id2) --> got resetted settings like
        bufferSize: 10000
        connectionId: ...
        name: ""
        paused: false
        persistent: true
   
My workaround for this problem in earlier versions:
   1-4. ...
   5. getInfo(connection_id2) --> check info, if settings reset
   6. update(connection_id2, settings) --> works with device normally


In version 62 workaround not working:
   1-4. ...
   5. getInfo(connection_id2) --> throw lastError: "Serial connection not found"
   6. getConnections(callback) --> never call callback(connections) and don't throw any errors

I don't remember what was with getConnections in earlier version but call getConnections(callback) in app background console gives the same result --> callback(connections) never called.

chrome.serial.getConnections(function(data) { /* If connections pool is empty this code never executed */ });

Checked the issue on reported version 62.0.3202.62 using windows 10 using below steps and unable to reproduce this.

1. Added USB device picker extension https://chrome.google.com/webstore/detail/usb-device-info/igkmggljimacfdfalpeelenjeicmfnll?utm_source=chrome-app-launcher-info-dialog 
2. Added a device to list. Now opened background page of app from chrome://extensions
3. Typed chrome.serial.getDevices(function(devices) { console.log(devices); }) in console of background page and error is seen. Attaching screenshot of same.

@Reporter: Could you please check and let us know whether we are missing anything in steps? and also please tell us on which extensio/app you are seeing this issue. This would help in identifying the CL that caused the regression.

Thanks!

Deleted: Issue 777552.png 76.9 KB

	Issue 777552.png 76.9 KB View Download

Serial API access require extra "serial" permission for application.
Section "permissions": https://developer.chrome.com/apps/serial.

"USB Device Info" works only with USB API.

I find workaround for my device on version 62.0 by adding timeout before disconnect:
   1. connect(port1, settings) --> got connection_id1
   2. send(connection_id1, ...)
      receive(connection_id1, ...) --> OK
   !!! 3. setTimeout(500) # timeout < 100 not working
   4. disconnect(connection_id1) --> OK
   5. connect(port1, settings) --> got connection_id2
   6. send(connection_id2, ...)
      receive(connection_id2, ...) --> OK
   ...

I think main problem in my device protocol implementation.
Something go wrong when opened connection disconnected instantly after send/receive.
"getConnections(callback)" don't apply only if broken close happend.
With a high probability bug reproduced only with my device.

You can downgrade priority of ticket.
I try to build chromium and debug problem when i will be free.

leon.han@, I suggest adding some tests for Mojo connection failure to make sure we're not forgetting to call callbacks in some cases.

c#8: Yeah agree, I'll try to add some related tests while investigating the root cause of this problem.

c#7: I see, disconnecting immediately after send/receive will make something wrong, then the next time connect() will get an invalid/unavailable connection id and also the getConnections() will never return. Thank you for these information.

I can confirm I am also having problems with getConnections callback never being called, not only on Windows 10 (tested on 2 surface laptops) but also in several Macbooks (tested 7 so far), after an update to 62.

I am trying to create a reproducible example, but I've noticed the problem because users of our product (a serial board, accessible via the chrome app https://chrome.google.com/webstore/detail/quirkbot-for-chrome/ackaalhbfjagidmjlhlokoblhbnahegd?hl=en) started reporting that the board "stopped working" after updating to chrome 62.

This had HUGE impact for all company, as it seems, all users in all platforms won't be able to use our hardware if they update to 62.

Sorry for offtop
pa...@azucrina.org what are you plannig to do when chrome drop support for "chrome applications" in 2018?
https://developer.chrome.com/apps/migration

#c11:  we will release a native desktop app for the serial functionality, and also use webmidi for parts of the communication in browser. Unfortunately WebUSB is not an option for our hardware, so we can't use that.

Here is a reproducible example of chrome.serial.getConnections not working after a port is disconnected.

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

Steps to reproduce the problem:
1. Unzip the unpackaged app and load it into Chrome
2. Inspect the 'background page' from recently loaded app (app name: "getConnections BUG")
3. Observer the output on the console

What is the expected behavior?
Console should output:
- getConnections works, 1
- getConnections works, 2
- getConnections works, 3
- getConnections works, 4

What went wrong?
"getConnections works" should be displayed 4 times in the console
"getConnections works, 4" is never displayed in the console.
It seems that after disconnecting all the ports, chrome.serial.getConnections does not fire it's callback anymore.

Did this work before? Yes 61.0

Chrome version: 62.0.3202.62 (Official Build) (64-bit)
OS Version: Mac OS X 10.12.6

Should I file a new bug, or is it going to be tracked #777552?

Deleted: getConnections BUG.zip 2.3 KB

getConnections BUG.zip 2.3 KB Download

c#13: Thanks paulo@ a lot for the report and all the helpful information! It's true that 'after disconnecting all the ports, chrome.serial.getConnections does not fire it's callback anymore.'.
I've uploaded a fix CL https://chromium-review.googlesource.com/c/chromium/src/+/741522, sorry..

Hi, art.ermolin@,
I still can't figure out clearly why the workaround(sleep before disconnect()) in c#7 can enable the next time connect/send/receive to work well.
As for the '5. getInfo(connection_id2) --> throw lastError: "Serial connection not found"' in c#4, that means the connection has been removed due to some internal errors, unfortunately I'm also not very clear about the root cause by now.
And, the getConnections() problem you mentioned would also be solved by the CL in c#14.

c#14: Thanks for the prompt update leon....@intel.com. From your experience how long does these fixes take to reach a next update?

c#15 Maybe delay before disconnect remove internal errors based on my device.

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

commit 8864b268ce2c9cdb6f6d77ceea774c56b6e82a8a
Author: Han Leon <leon.han@intel.com>
Date: Sat Oct 28 04:08:56 2017

[DeviceService] Fix a bug that chrome.serial.getConnections never return

This CL fixes the bug:
  1. chrome.serial.connect(port) --> got connection_id1
  2. chrome.serial.disconnect(connection_id1) --> ok
  3. chrome.serial.getConnections(cb) --> cb never be triggered!

BUG= 777552 
TEST=browser_tests --gtest_filter=SerialApiTest.SerialFakeHardware

Change-Id: I34c941c2141ab6555e1f26b59c547763acfea22d
Reviewed-on: https://chromium-review.googlesource.com/741522
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#512382}
[modify] https://crrev.com/8864b268ce2c9cdb6f6d77ceea774c56b6e82a8a/chrome/test/data/extensions/api_test/serial/api/background.js
[modify] https://crrev.com/8864b268ce2c9cdb6f6d77ceea774c56b6e82a8a/extensions/browser/api/serial/serial_api.cc

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

commit 272cff2ee467d8fb94f3bb7a20f43570f4d6958c
Author: Han Leon <leon.han@intel.com>
Date: Sat Oct 28 06:09:41 2017

Format a js file correctly

This is a follow-up of CL
https://chromium-review.googlesource.com/c/chromium/src/+/741522/

git cl format --js
chrome/test/data/extensions/api_test/serial/api/background.js

BUG= 777552 
TBR=reillyg@chromium.org

Change-Id: Icd4379286f862a1a17cca3b03e1f865591882714
Reviewed-on: https://chromium-review.googlesource.com/742230
Reviewed-by: Han Leon <leon.han@intel.com>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#512386}
[modify] https://crrev.com/272cff2ee467d8fb94f3bb7a20f43570f4d6958c/chrome/test/data/extensions/api_test/serial/api/background.js

c#16: I'm not very clear about the release issues, but this fix will be included in M64 at least, as for M63 or M62, I'm not sure whether we can request to merge this fix back into them? 
Maybe Reilly(CC'd) can help to give some answers precisely? Thanks.

Yes, it would be great to have to merged earlier...

M64 is scheduled for early 2018, right? That’s when chrome is retiring the
apps :/

Requesting merge to M-63.

This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
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 issue is reported on M62 (not M63 regression). M63 is already promoted to Beta and we're only taking critical merges in. Can't this wait until M64?

Currently all our users are impacted by these changes. We have around 10k +
boards in the market that potently can’t be used if users update to m62.

This seriously break the compatibility of chrome.serial, on already
stressed scenario of of it being deprecated due to the end of chrome apps.
While we are getting ready for the deprecation, this came as a surprise,
more than two months before scheduled.

So yes, for my business sake, I would like it to be merged asap.

Spoke to govind@ offline. This issue was reported after M-62 went to stable and so this is a regression fix we should merge to M-63. The change is low risk.

Approving merge to M63 branch 3239 based on comment #26. Please merge ASAP so we can take it in for this week Beta release. Thank you.

Does this also need merge to M62 future respin (if any)?

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

commit f60e5f9d293ff49eb8e65ea15a8557fe38419375
Author: Han Leon <leon.han@intel.com>
Date: Mon Oct 30 20:53:58 2017

[DeviceService] Fix a bug that chrome.serial.getConnections never return

This CL fixes the bug:
  1. chrome.serial.connect(port) --> got connection_id1
  2. chrome.serial.disconnect(connection_id1) --> ok
  3. chrome.serial.getConnections(cb) --> cb never be triggered!

BUG= 777552 
TEST=browser_tests --gtest_filter=SerialApiTest.SerialFakeHardware

Change-Id: I34c941c2141ab6555e1f26b59c547763acfea22d
Reviewed-on: https://chromium-review.googlesource.com/741522
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Original-Commit-Position: refs/heads/master@{#512382}(cherry picked from commit 8864b268ce2c9cdb6f6d77ceea774c56b6e82a8a)
Reviewed-on: https://chromium-review.googlesource.com/744761
Cr-Commit-Position: refs/branch-heads/3239@{#300}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/f60e5f9d293ff49eb8e65ea15a8557fe38419375/chrome/test/data/extensions/api_test/serial/api/background.js
[modify] https://crrev.com/f60e5f9d293ff49eb8e65ea15a8557fe38419375/extensions/browser/api/serial/serial_api.cc