Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(390)

Unified Diff: content/browser/renderer_host/render_process_host_impl.cc

Issue 152923005: Don't overwrite WebRTC AEC dump file. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Code review Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..131bae037ca69f478bcfaa1483aabae710a5e190 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)
+// |create_new_dump_file| is used for only opening the file so that it
+// overwrites an existing file the first create. Consecutive opens will append.
+// 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 create_new_dump_file = true;
+
// 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 = create_new_dump_file ?
+ 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();
}
+ create_new_dump_file = 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));
+ create_new_dump_file = 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),
« no previous file with comments | « no previous file | content/browser/resources/media/dump_creator.js » ('j') | content/browser/resources/media/dump_creator.js » ('J')

Powered by Google App Engine
This is Rietveld 408576698