PPAPI: Make NestedEvent::Signal() thread-safe

Also update Audio and URLLoader tests to use NestedEvent instead of
TestCompletionCallback and WaitForResult().

BUG=


Review URL: https://chromiumcodereview.appspot.com/14072003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193564 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 55 insertions, 61 deletions

diff --git a/ppapi/tests/test_audio.cc b/ppapi/tests/test_audio.cc
index a85e955..b3853ea 100644
--- a/ppapi/tests/test_audio.cc
+++ b/ppapi/tests/test_audio.cc
@@ -23,7 +23,7 @@ const int32_t kMagicValue = 12345;
 TestAudio::TestAudio(TestingInstance* instance)
     : TestCase(instance),
       audio_callback_method_(NULL),
-      test_callback_(),
+      audio_callback_event_(instance->pp_instance()),
       test_done_(false) {
 }
 
@@ -212,29 +212,19 @@ std::string TestAudio::TestAudioCallback1() {
   core_interface_->ReleaseResource(ac);
   ac = 0;
 
-  // |AudioCallbackTest()| calls |test_callback_|, sleeps a bit, then sets
-  // |test_done_|.
-  TestCompletionCallback test_callback(instance_->pp_instance());
-  test_callback_ = test_callback.GetCallback().pp_completion_callback();
+  audio_callback_event_.Reset();
   test_done_ = false;
-  callback_fired_ = false;
 
   audio_callback_method_ = &TestAudio::AudioCallbackTest;
   ASSERT_TRUE(audio_interface_->StartPlayback(audio));
 
   // Wait for the audio callback to be called.
-  test_callback.WaitForResult();
-  ASSERT_EQ(kMagicValue, test_callback.result());
-
+  audio_callback_event_.Wait();
   ASSERT_TRUE(audio_interface_->StopPlayback(audio));
-
-  // |StopPlayback()| should wait for the audio callback to finish.
-  ASSERT_TRUE(callback_fired_);
   test_done_ = true;
 
   // If any more audio callbacks are generated, we should crash (which is good).
   audio_callback_method_ = NULL;
-  test_callback_ = PP_CompletionCallback();
 
   core_interface_->ReleaseResource(audio);
 
@@ -258,10 +248,7 @@ std::string TestAudio::TestAudioCallback2() {
   core_interface_->ReleaseResource(ac);
   ac = 0;
 
-  // |AudioCallbackTest()| calls |test_callback_|, sleeps a bit, then sets
-  // |test_done_|.
-  TestCompletionCallback test_callback(instance_->pp_instance());
-  test_callback_ = test_callback.GetCallback().pp_completion_callback();
+  audio_callback_event_.Reset();
   test_done_ = false;
   callback_fired_ = false;
 
@@ -269,18 +256,14 @@ std::string TestAudio::TestAudioCallback2() {
   ASSERT_TRUE(audio_interface_->StartPlayback(audio));
 
   // Wait for the audio callback to be called.
-  test_callback.WaitForResult();
-  ASSERT_EQ(kMagicValue, test_callback.result());
+  audio_callback_event_.Wait();
 
   core_interface_->ReleaseResource(audio);
 
-  // The final release should wait for the audio callback to finish.
-  ASSERT_TRUE(callback_fired_);
   test_done_ = true;
 
   // If any more audio callbacks are generated, we should crash (which is good).
   audio_callback_method_ = NULL;
-  test_callback_ = PP_CompletionCallback();
 
   PASS();
 }
@@ -303,10 +286,7 @@ std::string TestAudio::TestAudioCallback3() {
   core_interface_->ReleaseResource(ac);
   ac = 0;
 
-  // |AudioCallbackTest()| calls |test_callback_|, sleeps a bit, then sets
-  // |test_done_|.
-  TestCompletionCallback test_callback_1(instance_->pp_instance());
-  test_callback_ = test_callback_1.GetCallback().pp_completion_callback();
+  audio_callback_event_.Reset();
   test_done_ = false;
   callback_fired_ = false;
 
@@ -314,36 +294,22 @@ std::string TestAudio::TestAudioCallback3() {
   ASSERT_TRUE(audio_interface_->StartPlayback(audio));
 
   // Wait for the audio callback to be called.
-  test_callback_1.WaitForResult();
-  ASSERT_EQ(kMagicValue, test_callback_1.result());
+  audio_callback_event_.Wait();
 
   ASSERT_TRUE(audio_interface_->StopPlayback(audio));
 
-  // |StopPlayback()| should wait for the audio callback to finish.
-  ASSERT_TRUE(callback_fired_);
-
-  TestCompletionCallback test_callback_2(instance_->pp_instance());
-  test_callback_ = test_callback_2.GetCallback().pp_completion_callback();
-
   // Repeat one more |StartPlayback| & |StopPlayback| cycle, and verify again
   // that the callback function was invoked.
-  callback_fired_ = false;
+  audio_callback_event_.Reset();
   ASSERT_TRUE(audio_interface_->StartPlayback(audio));
 
   // Wait for the audio callback to be called.
-  test_callback_2.WaitForResult();
-  ASSERT_EQ(kMagicValue, test_callback_2.result());
-
+  audio_callback_event_.Wait();
   ASSERT_TRUE(audio_interface_->StopPlayback(audio));
-
-  // |StopPlayback()| should wait for the audio callback to finish.
-  ASSERT_TRUE(callback_fired_);
-
   test_done_ = true;
 
   // If any more audio callbacks are generated, we should crash (which is good).
   audio_callback_method_ = NULL;
-  test_callback_ = PP_CompletionCallback();
 
   core_interface_->ReleaseResource(audio);
 
@@ -363,7 +329,7 @@ void TestAudio::AudioCallbackTrampoline(void* sample_buffer,
                                         void* user_data) {
   TestAudio* thiz = static_cast<TestAudio*>(user_data);
 
-  // Crash if not on the main thread.
+  // Crash if on the main thread.
   if (thiz->core_interface_->IsMainThread())
     Crash();
 
@@ -381,9 +347,6 @@ void TestAudio::AudioCallbackTest(void* sample_buffer,
   if (test_done_)
     Crash();
 
-  if (!callback_fired_) {
-    memset(sample_buffer, 0, buffer_size_in_bytes);
-    core_interface_->CallOnMainThread(0, test_callback_, kMagicValue);
-    callback_fired_ = true;
-  }
+  memset(sample_buffer, 0, buffer_size_in_bytes);
+  audio_callback_event_.Signal();
 }
diff --git a/ppapi/tests/test_audio.h b/ppapi/tests/test_audio.h
index 9554661..db86d1f 100644
--- a/ppapi/tests/test_audio.h
+++ b/ppapi/tests/test_audio.h
@@ -49,7 +49,7 @@ class TestAudio : public TestCase {
   void AudioCallbackTest(void* sample_buffer, uint32_t buffer_size_in_bytes);
 
   // Used by |TestAudioCallbackN()|.
-  PP_CompletionCallback test_callback_;
+  NestedEvent audio_callback_event_;
 
   bool test_done_;
   bool callback_fired_;
diff --git a/ppapi/tests/test_url_loader.cc b/ppapi/tests/test_url_loader.cc
index 320167a..0978f0e 100644
--- a/ppapi/tests/test_url_loader.cc
+++ b/ppapi/tests/test_url_loader.cc
@@ -792,12 +792,12 @@ std::string TestURLLoader::TestUntendedLoad() {
           total_bytes_to_be_received);
     if (bytes_received == total_bytes_to_be_received)
       break;
-    // TODO(dmichael): This should probably compare pp::MessageLoop::GetCurrent
-    //                 with GetForMainThread. We only need to yield on the main
-    //                 thread.
-    if (callback_type() != PP_BLOCKING) {
-      pp::Module::Get()->core()->CallOnMainThread(10, callback.GetCallback());
-      callback.WaitForResult();
+    // Yield if we're on the main thread, so that URLLoader can receive more
+    // data.
+    if (pp::Module::Get()->core()->IsMainThread()) {
+      NestedEvent event(instance_->pp_instance());
+      event.PostSignal(10);
+      event.Wait();
     }
   }
 
diff --git a/ppapi/tests/test_utils.cc b/ppapi/tests/test_utils.cc
index c708c44..06a4571 100644
--- a/ppapi/tests/test_utils.cc
+++ b/ppapi/tests/test_utils.cc
@@ -72,6 +72,7 @@ bool GetLocalHostPort(PP_Instance instance, std::string* host, uint16_t* port) {
 }
 
 void NestedEvent::Wait() {
+  PP_DCHECK(pp::Module::Get()->core()->IsMainThread());
   // Don't allow nesting more than once; it doesn't work with the code as-is,
   // and probably is a bad idea most of the time anyway.
   PP_DCHECK(!waiting_);
@@ -84,19 +85,37 @@ void NestedEvent::Wait() {
 }
 
 void NestedEvent::Signal() {
-  signalled_ = true;
-  if (waiting_)
-    GetTestingInterface()->QuitMessageLoop(instance_);
+  if (pp::Module::Get()->core()->IsMainThread())
+    SignalOnMainThread();
+  else
+    PostSignal(0);
+}
+
+void NestedEvent::PostSignal(int32_t wait_ms) {
+  pp::Module::Get()->core()->CallOnMainThread(
+      wait_ms,
+      pp::CompletionCallback(&SignalThunk, this),
+      0);
 }
 
 void NestedEvent::Reset() {
+  PP_DCHECK(pp::Module::Get()->core()->IsMainThread());
   // It doesn't make sense to reset when we're still waiting.
   PP_DCHECK(!waiting_);
-  // We must have already been Signalled().
-  PP_DCHECK(signalled_);
   signalled_ = false;
 }
 
+void NestedEvent::SignalOnMainThread() {
+  PP_DCHECK(pp::Module::Get()->core()->IsMainThread());
+  signalled_ = true;
+  if (waiting_)
+    GetTestingInterface()->QuitMessageLoop(instance_);
+}
+
+void NestedEvent::SignalThunk(void* event, int32_t /* result */) {
+  static_cast<NestedEvent*>(event)->SignalOnMainThread();
+}
+
 TestCompletionCallback::TestCompletionCallback(PP_Instance instance)
     : wait_for_result_called_(false),
       have_result_(false),
diff --git a/ppapi/tests/test_utils.h b/ppapi/tests/test_utils.h
index 71dffcc..d18b2845 100644
--- a/ppapi/tests/test_utils.h
+++ b/ppapi/tests/test_utils.h
@@ -41,6 +41,10 @@ bool GetLocalHostPort(PP_Instance instance, std::string* host, uint16_t* port);
 //  void TestFullscreen::DidChangeView(const pp::View& view) {
 //    nested_event_.Signal();
 //  }
+//
+// All methods except Signal and PostSignal must be invoked on the main thread.
+// It's OK to signal from a background thread, so you can (for example) Signal()
+// from the Audio thread.
 class NestedEvent {
  public:
   explicit NestedEvent(PP_Instance instance)
@@ -50,10 +54,18 @@ class NestedEvent {
   // has already been called, return immediately without running a nested loop.
   void Wait();
   // Signal the NestedEvent. If Wait() has been called, quit the message loop.
+  // This can be called from any thread.
   void Signal();
+  // Signal the NestedEvent in |wait_ms| milliseconds. This can be called from
+  // any thread.
+  void PostSignal(int32_t wait_ms);
+
   // Reset the NestedEvent so it can be used again.
   void Reset();
  private:
+  void SignalOnMainThread();
+  static void SignalThunk(void* async_event, int32_t result);
+
   PP_Instance instance_;
   bool waiting_;
   bool signalled_;