Avoid log spam in AudioSyncReader.

Avoid log spam in AudioSyncReader.

When the remote socket has been closed, Send will always fail, leading
to frequent logging. This change limits the logging to a maximum of one
log call for the lifetime of the AudioSyncReader. One log should be
enough, since the likely cause of the failure is that the remote socket
is closed (and this condition cannot be fixed). The log call is also
updated to display OS error information.

Also includes corresponding changes in AudioInputSyncWriter.

BUG=711400
Review-Url: https://codereview.chromium.org/2822123002
Cr-Commit-Position: refs/heads/master@{#465578}
Committed: https://chromium.googlesource.com/chromium/src/+/f38129c81c161fdd52874bf178b9110427a8c2ed

[Max Morin, 2017-04-18 07:39:03]

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

[commit-bot: I haz the power, 2017-04-18 07:39:23]

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

[Max Morin, 2017-04-18 07:42:40]

maxmorin@chromium.org changed reviewers:
+ tommi@chromium.org

[Max Morin, 2017-04-18 07:42:42]

Tommi: PTAL.

[commit-bot: I haz the power, 2017-04-18 08:38:03]

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

[commit-bot: I haz the power, 2017-04-18 08:38:04]

Dry run: This issue passed the CQ dry run.

[tommi (sloooow) - chröme, 2017-04-18 09:39:28]

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/audio_input_sync_writer.cc (right):

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/audio_input_sync_writer.cc:334:
had_socket_error_ = true;
Can we ever recover from this?
If not, can we have a DCHECK below?
If we can, where should the flag be cleared?

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/audio_sync_reader.cc (right):

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/audio_sync_reader.cc:180: had_socket_error_
= true;
same here

[Max Morin, 2017-04-18 09:55:28]

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/audio_input_sync_writer.cc (right):

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/audio_input_sync_writer.cc:334:
had_socket_error_ = true;
On 2017/04/18 09:39:28, tommi - chröme wrote:
> Can we ever recover from this?
> If not, can we have a DCHECK below?
> If we can, where should the flag be cleared?

In the case in the bug (other end of sync socket is closed) we cannot recover,
but it's an ok state for the program to be in (while closing the stream).
Looking at http://man7.org/linux/man-pages/man2/write.2.html, I think ENOSPC is
an error we could recover from. Do you think we should clear the flag if Send
succeeds?

[Max Morin, 2017-04-19 07:24:07]

Tommi: Ping

[tommi (sloooow) - chröme, 2017-04-19 10:44:35]

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/audio_input_sync_writer.cc (right):

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/audio_input_sync_writer.cc:334:
had_socket_error_ = true;
On 2017/04/18 09:55:28, Max Morin wrote:
> On 2017/04/18 09:39:28, tommi - chröme wrote:
> > Can we ever recover from this?
> > If not, can we have a DCHECK below?
> > If we can, where should the flag be cleared?
> 
> In the case in the bug (other end of sync socket is closed) we cannot recover,
> but it's an ok state for the program to be in (while closing the stream).
> Looking at http://man7.org/linux/man-pages/man2/write.2.html, I think ENOSPC
is
> an error we could recover from. Do you think we should clear the flag if Send
> succeeds?

Yes I think that would be a safer change to make at this point. We'll still
reduce the spam but if we recover, errors that might occur at a later stage,
will be reported.

[Max Morin, 2017-04-19 11:51:01]

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/audio_input_sync_writer.cc (right):

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/audio_input_sync_writer.cc:334:
had_socket_error_ = true;
On 2017/04/19 10:44:35, tommi - chröme wrote:
> On 2017/04/18 09:55:28, Max Morin wrote:
> > On 2017/04/18 09:39:28, tommi - chröme wrote:
> > > Can we ever recover from this?
> > > If not, can we have a DCHECK below?
> > > If we can, where should the flag be cleared?
> > 
> > In the case in the bug (other end of sync socket is closed) we cannot
recover,
> > but it's an ok state for the program to be in (while closing the stream).
> > Looking at http://man7.org/linux/man-pages/man2/write.2.html, I think ENOSPC
> is
> > an error we could recover from. Do you think we should clear the flag if
Send
> > succeeds?
> 
> Yes I think that would be a safer change to make at this point. We'll still
> reduce the spam but if we recover, errors that might occur at a later stage,
> will be reported.

Makes sense. I changed it.

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/audio_sync_reader.cc (right):

https://codereview.chromium.org/2822123002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/audio_sync_reader.cc:180: had_socket_error_
= true;
On 2017/04/18 09:39:28, tommi - chröme wrote:
> same here

Done.

[tommi (sloooow) - chröme, 2017-04-19 12:11:59]

lgtm

[Max Morin, 2017-04-19 12:13:14]

Thanks!

[Max Morin, 2017-04-19 12:13:18]

The CQ bit was checked by maxmorin@chromium.org

[commit-bot: I haz the power, 2017-04-19 12:13:34]

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-04-19 13:32:59]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492603998620120,
"parent_rev": "1f851d02ff8f6825b592fc8f980cd48b640e10f4", "commit_rev":
"f38129c81c161fdd52874bf178b9110427a8c2ed"}

[commit-bot: I haz the power, 2017-04-19 13:33:10]

Description was changed from

==========
Avoid log spam in AudioSyncReader.

When the remote socket has been closed, Send will always fail, leading
to frequent logging. This change limits the logging to a maximum of one
log call for the lifetime of the AudioSyncReader. One log should be
enough, since the likely cause of the failure is that the remote socket
is closed (and this condition cannot be fixed). The log call is also
updated to display OS error information.

Also includes corresponding changes in AudioInputSyncWriter.

BUG=711400
==========

to

==========
Avoid log spam in AudioSyncReader.

When the remote socket has been closed, Send will always fail, leading
to frequent logging. This change limits the logging to a maximum of one
log call for the lifetime of the AudioSyncReader. One log should be
enough, since the likely cause of the failure is that the remote socket
is closed (and this condition cannot be fixed). The log call is also
updated to display OS error information.

Also includes corresponding changes in AudioInputSyncWriter.

BUG=711400

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

[commit-bot: I haz the power, 2017-04-19 13:33:11]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f38129c81c161fdd52874bf178b9...