Don't overwrite WebRTC AEC dump file.

Don't overwrite WebRTC AEC dump file.

This is a non-perfect, temporary fix for the bug. It should be revisited after the M34 cut. There's e.g. discussions about tying this to getUserMedia. Bug for this: https://code.google.com/p/chromium/issues/detail?id=342770

BUG=342689
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251041

[Henrik Grunell, 2014-02-11 14:37:28]

Ami: main reviewer.
Scott: render_process_host_impl.cc (owner stamp trust Ami).

[sky, 2014-02-11 15:07:23]

https://codereview.chromium.org/152923005/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/152923005/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:201: // an existing
file the first time. Consecutive opens will append.
'file the first create' ?

https://codereview.chromium.org/152923005/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:205: static bool
first_aec_dump_open = true;
How about naming this a bit more clearly. Say create_new_dump_file or the
inverse, append_to_existing_dump_file?

[Henrik Grunell, 2014-02-11 16:26:54]

https://codereview.chromium.org/152923005/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/152923005/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:201: // an existing
file the first time. Consecutive opens will append.
On 2014/02/11 15:07:23, sky wrote:
> 'file the first create' ?

Done.

https://codereview.chromium.org/152923005/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:205: static bool
first_aec_dump_open = true;
On 2014/02/11 15:07:23, sky wrote:
> How about naming this a bit more clearly. Say create_new_dump_file or the
> inverse, append_to_existing_dump_file?

Done.

[Ami GONE FROM CHROMIUM, 2014-02-11 17:37:27]

Why not always append and make it the user's business to stop, move old file
aside, and restart, when s/he wants to separate dumps?

https://codereview.chromium.org/152923005/diff/50001/content/browser/resource...
File content/browser/resources/media/dump_creator.js (right):

https://codereview.chromium.org/152923005/diff/50001/content/browser/resource...
content/browser/resources/media/dump_creator.js:45: ' be invalid. To restart the
dump, the tab with the call recorded' +
s/recorded/being recorded/

[Henrik Grunell, 2014-02-12 07:54:04]

I think append always is a good suggestion. It also keeps this CL at a minimum
of changes. Done. (Also updated the info text.)

https://codereview.chromium.org/152923005/diff/50001/content/browser/resource...
File content/browser/resources/media/dump_creator.js (right):

https://codereview.chromium.org/152923005/diff/50001/content/browser/resource...
content/browser/resources/media/dump_creator.js:45: ' be invalid. To restart the
dump, the tab with the call recorded' +
On 2014/02/11 17:37:28, Ami Fischman wrote:
> s/recorded/being recorded/

Done.

[Henrik Grunell, 2014-02-12 07:59:13]

CC Victoria and Andrew.

[Ami GONE FROM CHROMIUM, 2014-02-12 19:24:37]

LGTM

[Henrik Grunell, 2014-02-12 20:02:50]

Scott, please have another look.

[sky, 2014-02-12 21:25:55]

Does this mean the log file grows unbounded?

[Ami GONE FROM CHROMIUM, 2014-02-12 21:37:44]

@sky: pretty much.  Just like all log files :)
(and note that this a developer debug thing)


On Wed, Feb 12, 2014 at 1:25 PM, <sky@chromium.org> wrote:

> Does this mean the log file grows unbounded?
>
> https://codereview.chromium.org/152923005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2014-02-12 21:42:00]

Ok, LGTM

[Henrik Grunell, 2014-02-13 09:07:07]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-02-13 09:07:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grunell@chromium.org/152923005/260001

[commit-bot: I haz the power, 2014-02-13 16:01:44]

Change committed as 251041