Attempt to de-flake ChildProcessTaskPortProviderTest.DeadTaskPort.

The ChildProcessTaskPortProvider does not provide any callback for when
a child process has exited. The test previously used the seqno on the
Mach port used for DEAD_NAME notifications to determine if the
process-died message had been delivered. But this is racy against
libdispatch delivering the callout to actual updating the bookkeeping.

Switch to to just looping the run loop until a timeout or exist
condition has been met.

Bug: 986288
Change-Id: Ic965b80878eb330caf6e83b307bed705dad4df1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1804521Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696863}

content/browser/child_process_task_port_provider_mac_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/synchronization/waitable_event.h"
 #include "base/task/post_task.h"
 #include "base/test/task_environment.h"
+#include "base/test/test_timeouts.h"
 #include "content/common/child_process.mojom.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -49,7 +50,6 @@ class ChildProcessTaskPortProviderTest : public testing::Test,
   ChildProcessTaskPortProviderTest()
       : event_(base::WaitableEvent::ResetPolicy::AUTOMATIC) {
     provider_.AddObserver(this);
-    last_seqno_ = GetNotificationPortSequenceNumber();
   }
   ~ChildProcessTaskPortProviderTest() override {
     provider_.RemoveObserver(this);
@@ -57,14 +57,15 @@ class ChildProcessTaskPortProviderTest : public testing::Test,
 
   void WaitForTaskPort() { event_.Wait(); }
 
-  // There is no observer callback for when a process dies, so use the kernel's
-  // sequence number on the notification port receive right to determine if the
-  // DEAD_NAME notification has been delivered. If the seqno is different, then
-  // assume it is.
-  void WaitForNotificationPortSeqnoChange() {
-    base::RunLoop run_loop;
-    CheckSequenceNumberAndQuitIfChanged(run_loop.QuitWhenIdleClosure());
-    run_loop.Run();
+  // There is no observer callback for when a process dies, so spin the run loop
+  // until the desired exit |condition| is met.
+  void WaitForCondition(base::RepeatingCallback<bool(void)> condition) {
+    base::TimeTicks start = base::TimeTicks::Now();
+    do {
+      base::RunLoop().RunUntilIdle();
+      if (condition.Run())
+        break;
+    } while ((base::TimeTicks::Now() - start) < TestTimeouts::action_timeout());
   }
 
   mach_port_urefs_t GetSendRightRefCount(mach_port_t send_right) {
@@ -95,37 +96,10 @@ class ChildProcessTaskPortProviderTest : public testing::Test,
   }
 
  private:
-  mach_port_seqno_t GetNotificationPortSequenceNumber() {
-    mach_port_status_t status;
-    mach_msg_type_number_t count = sizeof(status);
-    kern_return_t kr = mach_port_get_attributes(
-        mach_task_self(), provider_.notification_port_.get(),
-        MACH_PORT_RECEIVE_STATUS, reinterpret_cast<mach_port_info_t>(&status),
-        &count);
-    EXPECT_EQ(KERN_SUCCESS, kr);
-    return status.mps_seqno;
-  }
-
-  void CheckSequenceNumberAndQuitIfChanged(base::OnceClosure quit_closure) {
-    mach_port_seqno_t seqno = GetNotificationPortSequenceNumber();
-    if (seqno == last_seqno_) {
-      task_environment_.GetMainThreadTaskRunner()->PostDelayedTask(
-          FROM_HERE,
-          base::BindOnce(&ChildProcessTaskPortProviderTest::
-                             CheckSequenceNumberAndQuitIfChanged,
-                         base::Unretained(this), std::move(quit_closure)),
-          base::TimeDelta::FromMilliseconds(10));
-    } else {
-      last_seqno_ = seqno;
-      std::move(quit_closure).Run();
-    }
-  }
-
   base::test::TaskEnvironment task_environment_;
   ChildProcessTaskPortProvider provider_;
   base::WaitableEvent event_;
   std::vector<base::ProcessHandle> received_processes_;
-  mach_port_seqno_t last_seqno_;
 };
 
 static constexpr mach_port_t kMachPortNull = MACH_PORT_NULL;
@@ -167,7 +141,11 @@ TEST_F(ChildProcessTaskPortProviderTest, ChildLifecycle) {
   // "Kill" the process and verify that the association is deleted.
   receive_right.reset();
 
-  WaitForNotificationPortSeqnoChange();
+  WaitForCondition(base::BindRepeating(
+      [](ChildProcessTaskPortProvider* provider) -> bool {
+        return provider->TaskForPid(99) == MACH_PORT_NULL;
+      },
+      base::Unretained(provider())));
 
   EXPECT_EQ(kMachPortNull, provider()->TaskForPid(99));
 
@@ -176,8 +154,7 @@ TEST_F(ChildProcessTaskPortProviderTest, ChildLifecycle) {
   EXPECT_EQ(1u, GetDeadNameRefCount(send_right.get()));
 }
 
-// Test is flaky. See https://crbug.com/986288.
-TEST_F(ChildProcessTaskPortProviderTest, DISABLED_DeadTaskPort) {
+TEST_F(ChildProcessTaskPortProviderTest, DeadTaskPort) {
   EXPECT_EQ(kMachPortNull, provider()->TaskForPid(6));
 
   // Create a fake task port for the fake process.
@@ -237,7 +214,11 @@ TEST_F(ChildProcessTaskPortProviderTest, DISABLED_DeadTaskPort) {
 
   // Clean up the second receive right.
   receive_right2.reset();
-  WaitForNotificationPortSeqnoChange();
+  WaitForCondition(base::BindRepeating(
+      [](ChildProcessTaskPortProvider* provider) -> bool {
+        return provider->TaskForPid(123) == MACH_PORT_NULL;
+      },
+      base::Unretained(provider())));
   EXPECT_EQ(kMachPortNull, provider()->TaskForPid(123));
 }