Deflake WebRtcInternalsTest

The tests were flaky because of a race with
WebRtcEventLogManager::task_runner_. The problem is that
SequencedTaskRunner's destructor does not block, allowing us to destroy
a WebRtcEventLogManager objects while its |task_runner_| is still
executing a task that operates on the WebRtcEventLogManager object.

We solve this by making WebRtcEventLogManager work synchronously when
it's used in WebRTCInternals' unit tests (but nowhere else).

Bug: 796047
Change-Id: I76606b9394c1e5c02a3626fd8d3824bea587d1e4
Reviewed-on: https://chromium-review.googlesource.com/850697
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527083}

content/browser/webrtc/webrtc_event_log_manager.cc

@@ -102,6 +102,11 @@ void WebRtcEventLogManager::SetLocalLogsObserver(
                      base::Unretained(this), observer, std::move(reply)));
 }
 
+void WebRtcEventLogManager::SetTaskRunnerForTesting(
+    const scoped_refptr<base::SequencedTaskRunner>& task_runner) {
+  task_runner_ = task_runner;
+}
+
 void WebRtcEventLogManager::OnLocalLogStarted(PeerConnectionKey peer_connection,
                                               base::FilePath file_path) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());

content/browser/webrtc/webrtc_event_log_manager.h

@@ -115,6 +115,9 @@ class CONTENT_EXPORT WebRtcEventLogManager
   WebRtcEventLogManager();
   ~WebRtcEventLogManager() override;
 
+  void SetTaskRunnerForTesting(
+      const scoped_refptr<base::SequencedTaskRunner>& task_runner);
+
  private:
   using PeerConnectionKey = WebRtcEventLogPeerConnectionKey;
 

content/browser/webrtc/webrtc_internals_unittest.cc

@@ -9,6 +9,7 @@
 
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "content/browser/webrtc/webrtc_internals_ui_observer.h"
 #include "content/public/test/test_browser_thread.h"
@@ -74,7 +75,9 @@ class MockWakeLock : public device::mojom::WakeLock {
 
 class WebRtcEventLogManagerForTesting : public WebRtcEventLogManager {
  public:
-  WebRtcEventLogManagerForTesting() = default;
+  WebRtcEventLogManagerForTesting() {
+    SetTaskRunnerForTesting(base::ThreadTaskRunnerHandle::Get());
+  }
   ~WebRtcEventLogManagerForTesting() override = default;
 };
 
@@ -168,6 +171,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_AddRemoveObserver) {
   EXPECT_EQ("", observer.command());
 
   webrtc_internals.OnRemovePeerConnection(3, 4);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -194,6 +199,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_EnsureNoLogWhenNoObserver) {
   ASSERT_FALSE(dict->GetList("log", &log));
 
   webrtc_internals.OnRemovePeerConnection(3, 4);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -231,6 +238,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_EnsureLogIsRemovedWhenObserverIsRemoved) {
   ASSERT_FALSE(dict->GetList("log", &log));
 
   webrtc_internals.OnRemovePeerConnection(3, 4);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -257,6 +266,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_SendAddPeerConnectionUpdate) {
 
   webrtc_internals.RemoveObserver(&observer);
   webrtc_internals.OnRemovePeerConnection(1, 2);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -280,6 +291,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_SendRemovePeerConnectionUpdate) {
   VerifyInt(dict, "lid", 2);
 
   webrtc_internals.RemoveObserver(&observer);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -313,6 +326,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_SendUpdatePeerConnectionUpdate) {
 
   webrtc_internals.OnRemovePeerConnection(1, 2);
   webrtc_internals.RemoveObserver(&observer);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -338,6 +353,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_AddGetUserMedia) {
                          video_constraint);
 
   webrtc_internals.RemoveObserver(&observer);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -360,6 +377,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_SendAllUpdateWithGetUserMedia) {
                          video_constraint);
 
   webrtc_internals.RemoveObserver(&observer);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -406,6 +425,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_SendAllUpdatesWithPeerConnectionUpdate) {
   std::string time;
   EXPECT_TRUE(dict->GetString("time", &time));
   EXPECT_FALSE(time.empty());
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -434,6 +455,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_OnAddStats) {
   VerifyInt(dict, "pid", pid);
   VerifyInt(dict, "lid", lid);
   VerifyList(dict, "reports", list);
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -451,6 +474,8 @@ TEST_F(WebRtcInternalsTest,
 
   EXPECT_EQ("audioDebugRecordingsFileSelectionCancelled", observer.command());
   EXPECT_EQ(nullptr, observer.value());
+
+  base::RunLoop().RunUntilIdle();
 }
 
 // Flaky: crbug.com/796047.
@@ -501,6 +526,8 @@ TEST_F(WebRtcInternalsTest, DISABLED_WakeLock) {
   webrtc_internals.OnRemovePeerConnection(pid, lid[0]);
   EXPECT_EQ(0, webrtc_internals.num_open_connections());
   EXPECT_FALSE(webrtc_internals.HasWakeLock());
+
+  base::RunLoop().RunUntilIdle();
 }
 
 }  // namespace content