Move AEC dump verification to last in WebRTWebRtcAudioDebugRecordingsBrowserTest tests.

An attempt to reduce flakiness, based on that the AEC dump requires longer time before being closed than the input and output recordings due to IPC and more thread jumps. See also the added comment in the file for more info.

Bug: 783790
Change-Id: I1f2cd838a749eb1ec76e6e32e283b26e89ff293e
Reviewed-on: https://chromium-review.googlesource.com/813715
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522445}

content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc

@@ -106,6 +106,12 @@ class WebRtcAudioDebugRecordingsBrowserTest
 // HTML and Javascript is bypassed since it would trigger a file picker dialog.
 // Instead, the dialog callback FileSelected() is invoked directly. In fact,
 // there's never a webrtc-internals page opened at all since that's not needed.
+// Note: Both stopping the streams (at hangup()) and disabling the recordings
+// are asynchronous without response when finished. This means that closing the
+// files is asynchronous and being able to delete the files in the test is
+// therefore timing dependent and flaky prone. If it becomes flaky, it's
+// probably good enough to change the test to retry deleting after a short
+// sleep.
 IN_PROC_BROWSER_TEST_F(WebRtcAudioDebugRecordingsBrowserTest,
                        MAYBE_CallWithAudioDebugRecordings) {
   if (!HasAudioOutputDevices()) {
@@ -138,27 +144,16 @@ IN_PROC_BROWSER_TEST_F(WebRtcAudioDebugRecordingsBrowserTest,
 
   WebRTCInternals::GetInstance()->DisableAudioDebugRecordings();
 
-  // Verify that the expected AEC dump file exists and contains some data.
-  base::ProcessId render_process_id = base::kNullProcessId;
-  EXPECT_TRUE(GetRenderProcessHostId(&render_process_id));
-  base::FilePath file_path =
-      GetExpectedAecDumpFileName(base_file_path, render_process_id);
-  EXPECT_TRUE(base::PathExists(file_path));
-  int64_t file_size = 0;
-  EXPECT_TRUE(base::GetFileSize(file_path, &file_size));
-  EXPECT_GT(file_size, 0);
-  EXPECT_TRUE(base::DeleteFile(file_path, false));
-
   // Verify that the expected input audio file exists and contains some data.
   std::vector<base::FilePath> input_files =
       GetRecordingFileNames(FILE_PATH_LITERAL("input"), base_file_path);
   EXPECT_EQ(input_files.size(), 1u);
-  file_size = 0;
+  int64_t file_size = 0;
   EXPECT_TRUE(base::GetFileSize(input_files[0], &file_size));
   EXPECT_GT(file_size, kWaveHeaderSizeBytes);
   EXPECT_TRUE(base::DeleteFile(input_files[0], false));
 
-  // Verify that the expected output audio files exists and contains some data.
+  // Verify that the expected output audio files exist and contain some data.
   // Two files are expected, one for each peer in the call.
   std::vector<base::FilePath> output_files =
       GetRecordingFileNames(FILE_PATH_LITERAL("output"), base_file_path);
@@ -170,6 +165,17 @@ IN_PROC_BROWSER_TEST_F(WebRtcAudioDebugRecordingsBrowserTest,
     EXPECT_TRUE(base::DeleteFile(file_path, false));
   }
 
+  // Verify that the expected AEC dump file exists and contains some data.
+  base::ProcessId render_process_id = base::kNullProcessId;
+  EXPECT_TRUE(GetRenderProcessHostId(&render_process_id));
+  base::FilePath file_path =
+      GetExpectedAecDumpFileName(base_file_path, render_process_id);
+  EXPECT_TRUE(base::PathExists(file_path));
+  file_size = 0;
+  EXPECT_TRUE(base::GetFileSize(file_path, &file_size));
+  EXPECT_GT(file_size, 0);
+  EXPECT_TRUE(base::DeleteFile(file_path, false));
+
   // Verify that no other files exist and remove temp dir.
   EXPECT_TRUE(base::IsDirectoryEmpty(temp_dir_path));
   EXPECT_TRUE(base::DeleteFile(temp_dir_path, false));
@@ -286,26 +292,9 @@ IN_PROC_BROWSER_TEST_F(WebRtcAudioDebugRecordingsBrowserTest,
 
   WebRTCInternals::GetInstance()->DisableAudioDebugRecordings();
 
-  RenderProcessHost::iterator it =
-      content::RenderProcessHost::AllHostsIterator();
-  base::FilePath file_path;
   int64_t file_size = 0;
 
-  for (; !it.IsAtEnd(); it.Advance()) {
-    base::ProcessId render_process_id =
-        base::GetProcId(it.GetCurrentValue()->GetHandle());
-    EXPECT_NE(base::kNullProcessId, render_process_id);
-
-    // Verify that the expected AEC dump file exists and contains some data.
-    file_path = GetExpectedAecDumpFileName(base_file_path, render_process_id);
-    EXPECT_TRUE(base::PathExists(file_path));
-    file_size = 0;
-    EXPECT_TRUE(base::GetFileSize(file_path, &file_size));
-    EXPECT_GT(file_size, 0);
-    EXPECT_TRUE(base::DeleteFile(file_path, false));
-  }
-
-  // Verify that the expected input audio files exist and contains some data.
+  // Verify that the expected input audio files exist and contain some data.
   std::vector<base::FilePath> input_files =
       GetRecordingFileNames(FILE_PATH_LITERAL("input"), base_file_path);
   EXPECT_EQ(input_files.size(), 2u);
@@ -316,7 +305,7 @@ IN_PROC_BROWSER_TEST_F(WebRtcAudioDebugRecordingsBrowserTest,
     EXPECT_TRUE(base::DeleteFile(file_path, false));
   }
 
-  // Verify that the expected output audio files exists and contains some data.
+  // Verify that the expected output audio files exist and contain some data.
   // Four files are expected, one for each peer in each call. (Two calls * two
   // peers.)
   std::vector<base::FilePath> output_files =
@@ -329,6 +318,23 @@ IN_PROC_BROWSER_TEST_F(WebRtcAudioDebugRecordingsBrowserTest,
     EXPECT_TRUE(base::DeleteFile(file_path, false));
   }
 
+  // Verify that the expected AEC dump files exist and contain some data.
+  RenderProcessHost::iterator it =
+      content::RenderProcessHost::AllHostsIterator();
+  base::FilePath file_path;
+  for (; !it.IsAtEnd(); it.Advance()) {
+    base::ProcessId render_process_id =
+        base::GetProcId(it.GetCurrentValue()->GetHandle());
+    EXPECT_NE(base::kNullProcessId, render_process_id);
+
+    file_path = GetExpectedAecDumpFileName(base_file_path, render_process_id);
+    EXPECT_TRUE(base::PathExists(file_path));
+    file_size = 0;
+    EXPECT_TRUE(base::GetFileSize(file_path, &file_size));
+    EXPECT_GT(file_size, 0);
+    EXPECT_TRUE(base::DeleteFile(file_path, false));
+  }
+
   // Verify that no other files exist and remove temp dir.
   EXPECT_TRUE(base::IsDirectoryEmpty(temp_dir_path));
   EXPECT_TRUE(base::DeleteFile(temp_dir_path, false));