Don't overwrite WebRTC AEC dump file.

content/browser/renderer_host/render_process_host_impl.cc

Index: content/browser/renderer_host/render_process_host_impl.cc
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 03ce20c727f0826b9fafbc8ec4319ce21cc0bc36..ce77912607cb3df45559ff1df4c23260af8c3cca 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -197,6 +197,13 @@ void GetContexts(
 }
 
 #if defined(ENABLE_WEBRTC)
+// |first_aec_dump_open| is used for only opening the file so that it overwrites
+// an existing file the first time. Consecutive opens will append.

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

'file the first create' ?

[Henrik Grunell, 2014/02/11 16:26:55]

Done.

+// TODO(grunell): How this flag is used together with the two functions is
+// confusing. We actually assume that CreateAecDumpFileForProcess will be called
+// with the same |file_path| for all processes. Refactoring needed.
+static bool first_aec_dump_open = true;

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

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:55]

Done.

+
 // Creates a file used for diagnostic echo canceller recordings for handing
 // over to the renderer.
 IPC::PlatformFileForTransit CreateAecDumpFileForProcess(
@@ -204,23 +211,29 @@ IPC::PlatformFileForTransit CreateAecDumpFileForProcess(
     base::ProcessHandle process) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   base::PlatformFileError error = base::PLATFORM_FILE_OK;
+  int flags = first_aec_dump_open ?
+      base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_WRITE :
+      base::PLATFORM_FILE_OPEN_ALWAYS | base::PLATFORM_FILE_APPEND;
   base::PlatformFile aec_dump_file = base::CreatePlatformFile(
       file_path,
-      base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_WRITE,
+      flags,
       NULL,
       &error);
   if (error != base::PLATFORM_FILE_OK) {
     VLOG(1) << "Could not open AEC dump file, error=" << error;
     return IPC::InvalidPlatformFileForTransit();
   }
+  first_aec_dump_open = false;
   return IPC::GetFileHandleForProcess(aec_dump_file, process, true);
 }
 
-// Does nothing. Just to avoid races between enable and disable.
+// We assume that this function is called for all render process hosts when
+// called. The first one called will reset the state so that when enabled
+// again, the file will be opened so that it overwrite an existing file.
 void DisableAecDumpOnFileThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  first_aec_dump_open = true;
 }
-
 #endif
 
 // the global list of all renderer processes
@@ -1531,9 +1544,6 @@ void RenderProcessHostImpl::EnableAecDump(const base::FilePath& file) {
 }
 
 void RenderProcessHostImpl::DisableAecDump() {
-  // Posting on the FILE thread and then replying back on the UI thread is only
-  // for avoiding races between enable and disable. Nothing is done on the FILE
-  // thread.
   BrowserThread::PostTaskAndReply(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DisableAecDumpOnFileThread),