PPAPI: Make TestMessageHandler.Exceptions work

ppapi/tests/test_message_handler.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ppapi/tests/test_message_handler.h"
 
 #include <string.h>
 #include <algorithm>
 #include <map>
 #include <sstream>
@@ skipping 24 matching lines @@
 #undef PostMessage
 #endif
 
 REGISTER_TEST_CASE(MessageHandler);
 
 namespace {
 
 // Created and destroyed on the main thread. All public methods should be called
 // on the main thread. Most data members are only accessed on the main thread.
 // (Though it handles messages on the background thread).
-class EchoingMessageHandler : public pp::MessageHandler {
+class MockMessageHandler : public pp::MessageHandler {

[teravest, 2014/10/26 21:09:07]

I'm not sure Mock is the right prefix, maybe TestMessageHandler?

[dmichael (off chromium), 2014/10/27 16:30:39]

I almost used that name myself, but realized that's the name of the TestCase :).
I agree, it's not really a Mock or a Fake.

How about MyMessageHandler?

  public:
-  explicit EchoingMessageHandler(TestingInstance* instance,
-                                 const pp::MessageLoop& loop)
+  explicit MockMessageHandler(TestingInstance* instance,
+                              const pp::MessageLoop& loop)
       : testing_instance_(instance),
         message_handler_loop_(loop),
         is_registered_(false),
         test_finished_event_(instance->pp_instance()),
-        destroy_event_(instance->pp_instance()) {
+        destroy_event_(instance->pp_instance()),
+        async_message_received_(instance->pp_instance()) {
     AssertOnMainThread();
   }
   void Register() {
     AssertOnMainThread();
     assert(!is_registered_);
     int32_t result =
         testing_instance_->RegisterMessageHandler(this, message_handler_loop_);
     if (result == PP_OK) {
       is_registered_ = true;
     } else {
@@ skipping 24 matching lines @@
     assert(!is_registered_);
     destroy_event_.Wait();
     destroy_event_.Reset();
     // Now that we know Destroy() has been called, we know errors_ isn't being
     // written on the MessageHandler thread anymore. So we can safely read it
     // here on the main thread (since destroy_event_ gave us a memory barrier).
     std::string temp_errors;
     errors_.swap(temp_errors);
     return temp_errors;
   }
+  pp::Var WaitForAsyncMessage() {
+    async_message_received_.Wait();
+    pp::Var var_to_return = last_async_message_received_;
+    last_async_message_received_ = pp::Var();
+    async_message_received_.Reset();
+    return var_to_return;
+  }
  private:
   static void AssertOnMainThread() {
     assert(pp::MessageLoop::GetForMainThread() ==
            pp::MessageLoop::GetCurrent());
   }
   void AddError(const std::string& error) {
     if (!error.empty()) {
       if (!errors_.empty())
         errors_ += "<p>";
       errors_ += error;
     }
   }
   virtual void HandleMessage(pp::InstanceHandle instance, const pp::Var& var) {
     if (pp::MessageLoop::GetCurrent() != message_handler_loop_)
       AddError("HandleMessage was called on the wrong thread!");
     if (instance.pp_instance() != testing_instance_->pp_instance())
       AddError("HandleMessage was passed the wrong instance!");
     if (var.is_string() && var.AsString() == "FINISHED_TEST")
       test_finished_event_.Signal();
-    else
-      testing_instance_->PostMessage(var);

[dmichael (off chromium), 2014/10/24 21:06:21]

This "echo" was unused

+    else {

[teravest, 2014/10/26 21:09:07]

Please use braces on the if clause if you're going to use them on the else
clause.

[dmichael (off chromium), 2014/10/27 16:30:39]

Oops, done, thanks.

+      // Any client causing a message to arrive must wait for the message
+      // before continuing. See WaitForAsyncMessage().
+      assert(last_async_message_received_.is_undefined());
+      last_async_message_received_ = var;
+      async_message_received_.Signal();
+    }
   }
 
   virtual pp::Var HandleBlockingMessage(pp::InstanceHandle instance,
                                         const pp::Var& var) {
     if (pp::MessageLoop::GetCurrent() != message_handler_loop_)
       AddError("HandleBlockingMessage was called on the wrong thread!");
     if (instance.pp_instance() != testing_instance_->pp_instance())
       AddError("HandleBlockingMessage was passed the wrong instance!");
 
     // Attempt a blocking operation; make sure it's disallowed.
@@ skipping 29 matching lines @@
   // is_registered_ is only read/written on the main thread.
   bool is_registered_;
 
   // errors_ is written on the MessageHandler thread. When Destroy() is
   // called, we stop writing to errors_ and signal destroy_event_. This causes
   // a memory barrier, so it's safe to read errors_ after that.
   std::string errors_;
   NestedEvent test_finished_event_;
   NestedEvent destroy_event_;
 
+  pp::Var last_async_message_received_;
+  NestedEvent async_message_received_;
+
   // Undefined & private to disallow copy and assign.
-  EchoingMessageHandler(const EchoingMessageHandler&);
-  EchoingMessageHandler& operator=(const EchoingMessageHandler&);
+  MockMessageHandler(const MockMessageHandler&);
+  MockMessageHandler& operator=(const MockMessageHandler&);
 };
 
 void FakeHandleMessage(PP_Instance instance,
                        void* user_data,
                        const PP_Var* message_data) {}
 void FakeHandleBlockingMessage(PP_Instance instance,
                                void* user_data,
                                const PP_Var* message_data,
                                PP_Var* result) {}
 void FakeDestroy(PP_Instance instance, void* user_data) {}
 
 }  // namespace
 
 TestMessageHandler::TestMessageHandler(TestingInstance* instance)
     : TestCase(instance),
+      message_received_(instance->pp_instance()),
       ppb_messaging_if_(NULL),
-      handler_thread_(instance),
-      message_received_(instance->pp_instance()) {
+      handler_thread_(instance) {
 }
 
 TestMessageHandler::~TestMessageHandler() {
   handler_thread_.Join();
 }
 
 bool TestMessageHandler::Init() {
   ppb_messaging_if_ = static_cast<const PPB_Messaging_1_2*>(
       pp::Module::Get()->GetBrowserInterface(PPB_MESSAGING_INTERFACE_1_2));
   return ppb_messaging_if_ &&
          CheckTestingInterface() &&
          handler_thread_.Start();
 }
 
 void TestMessageHandler::RunTests(const std::string& filter) {
   RUN_TEST(RegisterErrorConditions, filter);
   RUN_TEST(PostMessageAndAwaitResponse, filter);
   RUN_TEST(Exceptions, filter);
 }
 
 void TestMessageHandler::HandleMessage(const pp::Var& message_data) {
   if (instance()->current_test_name() == "Exceptions") {
     // For TestPostMessageAndAwaitResponse(), all messages should go to the
     // background thread message handler.
     assert(false);
   } else {
-    if (message_data.is_string()) {
-      last_message_ = message_data.AsString();
-    } else {
-      last_message_ = "message_data was not a string!";
-    }
+    // Any subtest causing a message to arrive here must wait for it before
+    // continuing. See WaitForMessage().
+    assert(last_message_.is_undefined());
+    last_message_ = message_data;
     message_received_.Signal();
   }
 }
 
 std::string TestMessageHandler::TestRegisterErrorConditions() {
   {
     // Test registering with the main thread as the message loop.
     PPP_MessageHandler_0_2 fake_ppp_message_handler = {
       &FakeHandleMessage, &FakeHandleBlockingMessage, &FakeDestroy
     };
@@ skipping 17 matching lines @@
           reinterpret_cast<void*>(0xdeadbeef),
           &bad_ppp_ifs[i],
           handler_thread_.message_loop().pp_resource());
       ASSERT_EQ(PP_ERROR_BADARGUMENT, result);
     }
   }
   PASS();
 }
 
 std::string TestMessageHandler::TestPostMessageAndAwaitResponse() {
-  EchoingMessageHandler handler(instance(),
-                                handler_thread_.message_loop());
-  // Test doing a sync call before the handler is registered.
+  MockMessageHandler handler(instance(),
+                             handler_thread_.message_loop());
   handler.Register();
   std::string js_code("var plugin = document.getElementById('plugin');\n");
   js_code += "var result = undefined;\n";
   const char* const values_to_test[] = {
       "5",
       "undefined",
       "1.5",
       "'hello'",
       "{'key': 'value', 'array_key': [1, 2, 3, 4, 5]}",
       NULL
@@ skipping 12 matching lines @@
   instance_->EvalScript(js_code);
   instance_->EvalScript("plugin.postMessage('FINISHED_TEST');\n");
   handler.WaitForTestFinishedMessage();
   handler.Unregister();
   ASSERT_SUBTEST_SUCCESS(handler.WaitForDestroy());
 
   PASS();
 }
 
 std::string TestMessageHandler::TestExceptions() {
-  EchoingMessageHandler handler(instance(),
-                                handler_thread_.message_loop());
+  MockMessageHandler handler(instance(),
+                             handler_thread_.message_loop());
   {
     // First, try sending a blocking message when there is no handler
     // registered. It should throw an exception.
     std::string js_code(
         "var plugin = document.getElementById('plugin');\n"
         "var caught_exception = false;\n"
         "try {\n"
         "  plugin.postMessageAndAwaitResponse('Hello!');\n"
         "} catch (err) {\n"
         "  caught_exception = true;\n"
         "}\n"
         "plugin.postMessage(caught_exception ? 'SUCCESS' : 'FAIL');\n");
     instance_->EvalScript(js_code);
-    message_received_.Wait();
-    ASSERT_EQ("SUCCESS", last_message_);
+    // Note that we want to wait for the Instance to get the SUCCESS/FAIL
+    // message here. |message_handler| is not yet registered, so the message
+    // goes to the instance instead.
+    pp::Var msg = WaitForMessage();
+    ASSERT_TRUE(msg.is_string());
+    ASSERT_EQ("SUCCESS", msg.AsString());
   }
   handler.Register();
   {
     // Now that a handler is registered, try requesting and sending a
     // FileSystem. It should throw an exception. The file system is opened
     // asynchronously. What *should* happen is that it opens successfully, then
     // we try to send it via postMessageAndAwaitResponse, which fails with an
     // exception. The test could fail either because the filesystem doesn't
     // open or because postMessageAndAwaitResponse doesn't throw an exception.
     std::string js_code(
         "var plugin = document.getElementById('plugin');\n"
         "function gotFileSystem(fs) {\n"
         "  var caught_exception = false;\n"
         "  try {\n"
         "    plugin.postMessageAndAwaitResponse(fs);\n"
         "  } catch (err) {\n"
         "    caught_exception = true;\n"
         "  }\n"
         "  plugin.postMessage(caught_exception ? 'SUCCESS' : 'FAIL');\n"
         "}\n"
         "function fileSystemError() {\n"
         "  plugin.postMessage('Failed to open filesystem');\n"
         "}\n"
         "window.webkitRequestFileSystem(\n"
         "    window.Temporary, 1024, gotFileSystem, fileSystemError)\n");
     instance_->EvalScript(js_code);
-    message_received_.Wait();

[dmichael (off chromium), 2014/10/24 21:06:21]

I never called message_received_.Reset(), so this Wait() returned immediately,
and "last_message_" was still set to "SUCCESS" from 308.

-    ASSERT_EQ("SUCCESS", last_message_);
+    pp::Var msg = handler.WaitForAsyncMessage();
+    ASSERT_EQ(PP_VARTYPE_STRING, msg.pp_var().type);
+    ASSERT_EQ("SUCCESS", msg.AsString());
   }
   handler.Unregister();
   ASSERT_SUBTEST_SUCCESS(handler.WaitForDestroy());
 
   PASS();
 }
 
+pp::Var TestMessageHandler::WaitForMessage() {
+  message_received_.Wait();
+  pp::Var var_to_return = last_message_;
+  last_message_ = pp::Var();
+  message_received_.Reset();
+  return var_to_return;
+}
+