Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on Posix platforms.

Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on Posix platforms.

Removing the special case for fd > 1023 which can lead to audio glitches.

Code originally written by ossu@.

BUG=722720
Review-Url: https://codereview.chromium.org/2888403005
Cr-Commit-Position: refs/heads/master@{#473856}
Committed: https://chromium.googlesource.com/chromium/src/+/b464b3efcea3392d5254e4bb55053916801fcef7

[Henrik Grunell, 2017-05-20 10:29:19]

Description was changed from

==========
Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select().

Removing the special case for fd > 1023 which could lead to audio glitches.

BUG=722720
==========

to

==========
Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on
Posix platforms.

Removing the special case for fd > 1023 which could lead to audio glitches.

BUG=722720
==========

[Henrik Grunell, 2017-05-20 10:29:19]

grunell@chromium.org changed reviewers:
+ ossu@chromium.org, thestig@chromium.org, tommi@chromium.org

[Henrik Grunell, 2017-05-20 10:29:33]

Description was changed from

==========
Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on
Posix platforms.

Removing the special case for fd > 1023 which could lead to audio glitches.

BUG=722720
==========

to

==========
Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on
Posix platforms.

Removing the special case for fd > 1023 which can lead to audio glitches.

BUG=722720
==========

[Henrik Grunell, 2017-05-20 10:34:59]

Description was changed from

==========
Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on
Posix platforms.

Removing the special case for fd > 1023 which can lead to audio glitches.

BUG=722720
==========

to

==========
Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on
Posix platforms.

Removing the special case for fd > 1023 which can lead to audio glitches.

Code originally written by ossu@.

BUG=722720
==========

[tommi (sloooow) - chröme, 2017-05-21 07:59:33]

lgtm

[Henrik Grunell, 2017-05-22 08:04:42]

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

[commit-bot: I haz the power, 2017-05-22 08:05:09]

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, 2017-05-22 09:04:57]

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

[commit-bot: I haz the power, 2017-05-22 09:04:58]

Dry run: This issue passed the CQ dry run.

[ossu-chromium, 2017-05-22 11:59:37]

Maybe I'm a bit biased, but lgtm! :)

[Henrik Grunell, 2017-05-22 16:08:38]

Lei - ptal.

[Lei Zhang, 2017-05-22 23:20:32]

lgtm with a nit.

ossu: Curious, why didn't you just submit the CL yourself?

https://codereview.chromium.org/2888403005/diff/40001/base/sync_socket_posix.cc
File base/sync_socket_posix.cc (right):

https://codereview.chromium.org/2888403005/diff/40001/base/sync_socket_posix....
base/sync_socket_posix.cc:162: const TimeDelta this_timeout = finish_time -
base::TimeTicks::Now();
nit: no need for base:: inside namespace base. Like line 152 above.

[Henrik Grunell, 2017-05-23 08:30:12]

https://codereview.chromium.org/2888403005/diff/40001/base/sync_socket_posix.cc
File base/sync_socket_posix.cc (right):

https://codereview.chromium.org/2888403005/diff/40001/base/sync_socket_posix....
base/sync_socket_posix.cc:162: const TimeDelta this_timeout = finish_time -
base::TimeTicks::Now();
On 2017/05/22 23:20:32, Lei Zhang wrote:
> nit: no need for base:: inside namespace base. Like line 152 above.

Done.

[ossu-chromium, 2017-05-23 09:07:01]

On 2017/05/22 23:20:32, Lei Zhang wrote:
> ossu: Curious, why didn't you just submit the CL yourself?

Well, grunell@ did all the actual troubleshooting and identified the root cause.
I just had some extra cycles to try a different implementation on a Friday
afternoon, but didn't spend any time on cleaning it up. So it made sense to have
him continue from my draft.

[Henrik Grunell, 2017-05-23 09:25:33]

The CQ bit was checked by grunell@chromium.org

[Henrik Grunell, 2017-05-23 09:25:33]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@chromium.org,
thestig@chromium.org, ossu@chromium.org
Link to the patchset: https://codereview.chromium.org/2888403005/#ps60001
(title: "Code review (thestig@).")

[commit-bot: I haz the power, 2017-05-23 09:25:45]

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, 2017-05-23 11:15:31]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495531533420380,
"parent_rev": "a50429778eb03e6ad02e4f0500379d77a73c1b82", "commit_rev":
"b464b3efcea3392d5254e4bb55053916801fcef7"}

[commit-bot: I haz the power, 2017-05-23 11:15:43]

Description was changed from

==========
Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on
Posix platforms.

Removing the special case for fd > 1023 which can lead to audio glitches.

Code originally written by ossu@.

BUG=722720
==========

to

==========
Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on
Posix platforms.

Removing the special case for fd > 1023 which can lead to audio glitches.

Code originally written by ossu@.

BUG=722720

Review-Url: https://codereview.chromium.org/2888403005
Cr-Commit-Position: refs/heads/master@{#473856}
Committed:
https://chromium.googlesource.com/chromium/src/+/b464b3efcea3392d5254e4bb5505...
==========

[commit-bot: I haz the power, 2017-05-23 11:15:44]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b464b3efcea3392d5254e4bb5505...