Fix a bunch of mojo_public_*_unittests with the new EDK.

The unittests had baked in assumptions about implementation timings of the EDK.

This makes the following tests pass: mojo_public_bindings_unittests, mojo_public_environment_unittests, mojo_public_system_unittests and mojo_public_utility_unittests.

BUG=561803

Review URL: https://codereview.chromium.org/1552983003

Cr-Commit-Position: refs/heads/master@{#367421}

mojo/fetcher/data_fetcher_unittest.cc

@@ -82,6 +82,12 @@ class DataFetcherTest : public testing::Test {
 
     uint32_t num_bytes = 0;
     Handle body_handle = response->body.release();
+
+    MojoHandleSignalsState hss;
+    ASSERT_EQ(MOJO_RESULT_OK,
+              MojoWait(body_handle.value(), MOJO_HANDLE_SIGNAL_READABLE,
+                       MOJO_DEADLINE_INDEFINITE, &hss));
+
     MojoResult result = MojoReadData(body_handle.value(), nullptr, &num_bytes,
                                      MOJO_READ_DATA_FLAG_QUERY);
     ASSERT_EQ(MOJO_RESULT_OK, result);

mojo/message_pump/message_pump_mojo_unittest.cc

@@ -33,9 +33,29 @@ class CountingMojoHandler : public MessagePumpMojoHandler {
                    NULL,
                    MOJO_READ_MESSAGE_FLAG_NONE);
     ++success_count_;
+    if (success_count_ == success_callback_count_ &&
+        !success_callback_.is_null()) {
+      success_callback_.Run();
+      success_callback_.Reset();
+    }
   }
+
+  void set_success_callback(const base::Closure& callback,
+                            int success_count) {
+    success_callback_ = callback;
+    success_callback_count_ = success_count;
+  }
+
   void OnHandleError(const Handle& handle, MojoResult result) override {
     ++error_count_;
+    if (!error_callback_.is_null()) {
+      error_callback_.Run();
+      error_callback_.Reset();
+    }
+  }
+
+  void set_error_callback(const base::Closure& callback) {
+    error_callback_ = callback;
   }
 
   int success_count() { return success_count_; }
@@ -45,6 +65,11 @@ class CountingMojoHandler : public MessagePumpMojoHandler {
   int success_count_;
   int error_count_;
 
+  base::Closure error_callback_;
+  int success_callback_count_;
+
+  base::Closure success_callback_;
+
   DISALLOW_COPY_AND_ASSIGN(CountingMojoHandler);
 };
 
@@ -60,6 +85,8 @@ class CountingObserver : public MessagePumpMojo::Observer {
 TEST(MessagePumpMojo, RunUntilIdle) {
   base::MessageLoop message_loop(MessagePumpMojo::Create());
   CountingMojoHandler handler;
+  base::RunLoop run_loop;
+  handler.set_success_callback(run_loop.QuitClosure(), 2);
   MessagePipe handles;
   MessagePumpMojo::current()->AddHandler(&handler,
                                          handles.handle0.get(),
@@ -69,8 +96,11 @@ TEST(MessagePumpMojo, RunUntilIdle) {
       handles.handle1.get(), NULL, 0, NULL, 0, MOJO_WRITE_MESSAGE_FLAG_NONE);
   WriteMessageRaw(
       handles.handle1.get(), NULL, 0, NULL, 0, MOJO_WRITE_MESSAGE_FLAG_NONE);
-  base::RunLoop run_loop;
-  run_loop.RunUntilIdle();
+  MojoHandleSignalsState hss;
+  ASSERT_EQ(MOJO_RESULT_OK,
+            MojoWait(handles.handle0.get().value(), MOJO_HANDLE_SIGNAL_READABLE,
+                      MOJO_DEADLINE_INDEFINITE, &hss));
+  run_loop.Run();
   EXPECT_EQ(2, handler.success_count());
 }
 
@@ -81,24 +111,34 @@ TEST(MessagePumpMojo, Observer) {
   MessagePumpMojo::current()->AddObserver(&observer);
 
   CountingMojoHandler handler;
+  base::RunLoop run_loop;
+  handler.set_success_callback(run_loop.QuitClosure(), 1);
   MessagePipe handles;
   MessagePumpMojo::current()->AddHandler(&handler,
                                          handles.handle0.get(),
                                          MOJO_HANDLE_SIGNAL_READABLE,
                                          base::TimeTicks());
   WriteMessageRaw(
       handles.handle1.get(), NULL, 0, NULL, 0, MOJO_WRITE_MESSAGE_FLAG_NONE);
-  base::RunLoop run_loop;
-  run_loop.RunUntilIdle();
+
+  MojoHandleSignalsState hss;
+  ASSERT_EQ(MOJO_RESULT_OK,
+            MojoWait(handles.handle0.get().value(), MOJO_HANDLE_SIGNAL_READABLE,
+                      MOJO_DEADLINE_INDEFINITE, &hss));
+  run_loop.Run();
   EXPECT_EQ(1, handler.success_count());
   EXPECT_EQ(1, observer.will_signal_handler_count);
   EXPECT_EQ(1, observer.did_signal_handler_count);
   MessagePumpMojo::current()->RemoveObserver(&observer);
 
+  base::RunLoop run_loop2;
+  handler.set_success_callback(run_loop2.QuitClosure(), 2);
   WriteMessageRaw(
       handles.handle1.get(), NULL, 0, NULL, 0, MOJO_WRITE_MESSAGE_FLAG_NONE);
-  base::RunLoop run_loop2;
-  run_loop2.RunUntilIdle();
+  ASSERT_EQ(MOJO_RESULT_OK,
+            MojoWait(handles.handle0.get().value(), MOJO_HANDLE_SIGNAL_READABLE,
+                      MOJO_DEADLINE_INDEFINITE, &hss));
+  run_loop2.Run();
   EXPECT_EQ(2, handler.success_count());
   EXPECT_EQ(1, observer.will_signal_handler_count);
   EXPECT_EQ(1, observer.did_signal_handler_count);
@@ -107,16 +147,15 @@ TEST(MessagePumpMojo, Observer) {
 TEST(MessagePumpMojo, UnregisterAfterDeadline) {
   base::MessageLoop message_loop(MessagePumpMojo::Create());
   CountingMojoHandler handler;
+  base::RunLoop run_loop;
+  handler.set_error_callback(run_loop.QuitClosure());
   MessagePipe handles;
   MessagePumpMojo::current()->AddHandler(
       &handler,
       handles.handle0.get(),
       MOJO_HANDLE_SIGNAL_READABLE,
       base::TimeTicks::Now() - base::TimeDelta::FromSeconds(1));
-  for (int i = 0; i < 2; ++i) {
-    base::RunLoop run_loop;
-    run_loop.RunUntilIdle();
-  }
+  run_loop.Run();
   EXPECT_EQ(1, handler.error_count());
 }
 

mojo/public/c/system/tests/core_unittest.cc

@@ -156,6 +156,10 @@ TEST(CoreTest, BasicMessagePipe) {
   // Close |h0|.
   EXPECT_EQ(MOJO_RESULT_OK, MojoClose(h0));
 
+  EXPECT_EQ(MOJO_RESULT_OK,
+            MojoWait(h1, MOJO_HANDLE_SIGNAL_PEER_CLOSED,
+                     MOJO_DEADLINE_INDEFINITE, &state));
+
   // |h1| should no longer be readable or writable.
   EXPECT_EQ(
       MOJO_RESULT_FAILED_PRECONDITION,
@@ -250,7 +254,8 @@ TEST(CoreTest, BasicDataPipe) {
 
   // |hc| should still be readable.
   EXPECT_EQ(MOJO_RESULT_OK,
-            MojoWait(hc, MOJO_HANDLE_SIGNAL_READABLE, 0, &state));
+            MojoWait(hc, MOJO_HANDLE_SIGNAL_PEER_CLOSED,
+                     MOJO_DEADLINE_INDEFINITE, &state));
 
   EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_PEER_CLOSED,
             state.satisfied_signals);

mojo/public/cpp/bindings/lib/validation_errors.cc

@@ -66,8 +66,9 @@ void ReportValidationError(ValidationError error, const char* description) {
   }
 }
 
-ValidationErrorObserverForTesting::ValidationErrorObserverForTesting()
-    : last_error_(VALIDATION_ERROR_NONE) {
+ValidationErrorObserverForTesting::ValidationErrorObserverForTesting(
+    const Callback<void()>& callback)
+    : last_error_(VALIDATION_ERROR_NONE), callback_(callback) {
   MOJO_DCHECK(!g_validation_error_observer);
   g_validation_error_observer = this;
 }

mojo/public/cpp/bindings/lib/validation_errors.h

@@ -5,6 +5,7 @@
 #ifndef MOJO_PUBLIC_CPP_BINDINGS_LIB_VALIDATION_ERRORS_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_LIB_VALIDATION_ERRORS_H_
 
+#include "mojo/public/cpp/bindings/callback.h"
 #include "mojo/public/cpp/system/macros.h"
 
 namespace mojo {
@@ -70,14 +71,18 @@ void ReportValidationError(ValidationError error,
 // validation.
 class ValidationErrorObserverForTesting {
  public:
-  ValidationErrorObserverForTesting();
+  explicit ValidationErrorObserverForTesting(const Callback<void()>& callback);
   ~ValidationErrorObserverForTesting();
 
   ValidationError last_error() const { return last_error_; }
-  void set_last_error(ValidationError error) { last_error_ = error; }
+  void set_last_error(ValidationError error) {
+    last_error_ = error;
+    callback_.Run();
+  }
 
  private:
   ValidationError last_error_;
+  Callback<void()> callback_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(ValidationErrorObserverForTesting);
 };

mojo/public/cpp/bindings/tests/associated_interface_unittest.cc

@@ -173,36 +173,47 @@ TEST_F(AssociatedInterfaceTest, InterfacesAtBothEnds) {
   AssociatedInterfacePtr<IntegerSender> ptr1;
   ptr1.Bind(std::move(ptr_info));
 
+  base::RunLoop run_loop, run_loop2;
   bool ptr0_callback_run = false;
-  ptr0->Echo(123, [&ptr0_callback_run](int32_t value) {
+  ptr0->Echo(123, [&ptr0_callback_run, &run_loop](int32_t value) {
     EXPECT_EQ(123, value);
     ptr0_callback_run = true;
+    run_loop.Quit();
   });
 
   bool ptr1_callback_run = false;
-  ptr1->Echo(456, [&ptr1_callback_run](int32_t value) {
+  ptr1->Echo(456, [&ptr1_callback_run, &run_loop2](int32_t value) {
     EXPECT_EQ(456, value);
     ptr1_callback_run = true;
+    run_loop2.Quit();
   });
 
-  PumpMessages();
+  run_loop.Run();
+  run_loop2.Run();
   EXPECT_TRUE(ptr0_callback_run);
   EXPECT_TRUE(ptr1_callback_run);
 
   bool ptr0_error_callback_run = false;
-  ptr0.set_connection_error_handler(
-      [&ptr0_error_callback_run]() { ptr0_error_callback_run = true; });
+  base::RunLoop run_loop3;
+  ptr0.set_connection_error_handler([&ptr0_error_callback_run, &run_loop3]() {
+    ptr0_error_callback_run = true;
+    run_loop3.Quit();
+  });
 
   impl0.binding()->Close();
-  PumpMessages();
+  run_loop3.Run();
   EXPECT_TRUE(ptr0_error_callback_run);
 
   bool impl1_error_callback_run = false;
+  base::RunLoop run_loop4;
   impl1.binding()->set_connection_error_handler(
-      [&impl1_error_callback_run]() { impl1_error_callback_run = true; });
+      [&impl1_error_callback_run, &run_loop4]() {
+        impl1_error_callback_run = true;
+        run_loop4.Quit();
+      });
 
   ptr1.reset();
-  PumpMessages();
+  run_loop4.Run();
   EXPECT_TRUE(impl1_error_callback_run);
 }
 
@@ -520,20 +531,31 @@ TEST_F(AssociatedInterfaceTest, PassAssociatedInterfaces) {
       GetProxy(&sender0, connection_ptr.associated_group()));
 
   int32_t echoed_value = 0;
-  sender0->Echo(123, [&echoed_value](int32_t value) { echoed_value = value; });
-  PumpMessages();
+  base::RunLoop run_loop;
+  sender0->Echo(123, [&echoed_value, &run_loop](int32_t value) {
+    echoed_value = value;
+    run_loop.Quit();
+  });
+  run_loop.Run();
   EXPECT_EQ(123, echoed_value);
 
   IntegerSenderAssociatedPtr sender1;
+  base::RunLoop run_loop2;
   connection_ptr->AsyncGetSender(
-      [&sender1](AssociatedInterfacePtrInfo<IntegerSender> ptr_info) {
+      [&sender1, &run_loop2](
+          AssociatedInterfacePtrInfo<IntegerSender> ptr_info) {
         sender1.Bind(std::move(ptr_info));
+        run_loop2.Quit();
       });
-  PumpMessages();
+  run_loop2.Run();
   EXPECT_TRUE(sender1);
 
-  sender1->Echo(456, [&echoed_value](int32_t value) { echoed_value = value; });
-  PumpMessages();
+  base::RunLoop run_loop3;
+  sender1->Echo(456, [&echoed_value, &run_loop3](int32_t value) {
+    echoed_value = value;
+    run_loop3.Quit();
+  });
+  run_loop3.Run();
   EXPECT_EQ(456, echoed_value);
 }