Add TestConnectFailure, TestGetHandleFailure and TestConnectAndPipe to PPAPI Broker UI test.

ppapi/tests/test_broker.cc

Index: ppapi/tests/test_broker.cc
diff --git a/ppapi/tests/test_broker.cc b/ppapi/tests/test_broker.cc
index b1d5b028c58cf2adedbc4f45237335461277dfb4..bd75d634feaf71508ca3d94c1333fd0705f2948d 100644
--- a/ppapi/tests/test_broker.cc
+++ b/ppapi/tests/test_broker.cc
@@ -4,13 +4,118 @@
 
 #include "ppapi/tests/test_broker.h"
 
+#include <cstring>
+#include <fstream>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/platform_file.h"
+#include "base/scoped_temp_dir.h"
+#include "base/sync_socket.h"
 #include "ppapi/c/pp_errors.h"
+#include "ppapi/c/trusted/ppp_broker.h"
 #include "ppapi/c/trusted/ppb_broker_trusted.h"
-#include "ppapi/cpp/module.h"
+#include "ppapi/tests/test_utils.h"
 #include "ppapi/tests/testing_instance.h"
 
 REGISTER_TEST_CASE(Broker);
 
+namespace {
+
+const char kHelloMessage[]        = "Hello Plugin! This is Broker!";
+const char kBrokerIsUnsandboxed[] = "Broker is Unsandboxed!";
+
+// TODO(xhwang): merge PlatformFileToInt definitions and move to a common place.
+int32_t PlatformFileToInt(base::PlatformFile handle) {
+#if defined(OS_WIN)
+  return static_cast<int32_t>(reinterpret_cast<intptr_t>(handle));
+#elif defined(OS_POSIX)
+  return handle;
+#else
+  #error Not implemented.
+#endif
+}
+
+base::PlatformFile IntToPlatformFile(int32_t handle) {
+#if defined(OS_WIN)
+  return reinterpret_cast<HANDLE>(static_cast<intptr_t>(handle));
+#elif defined(OS_POSIX)
+  return handle;
+#else
+  #error Not implemented.
+#endif
+}
+
+bool VerifyIsUnsandboxed() {
+  std::fstream fs;
+
+  // Temporary directory will be cleaned when this object goes out of scope.
+  ScopedTempDir temp_dir_;
+  if (!temp_dir_.CreateUniqueTempDir())
+    return false;
+
+  FilePath test_file_name =
+      temp_dir_.path().Append(FILE_PATH_LITERAL("test_pepper_broker.txt"));
+
+  fs.open(test_file_name.value().c_str(), std::fstream::out);
+  if (!fs)
+    return false;
+  fs << "Verify that the broker is unsandboxed.";
+  if (!fs)
+    return false;
+  fs.close();
+
+  fs.open(test_file_name.value().c_str(), std::fstream::in);
+  if (!fs)
+    return false;
+  char buffer[16] = {0};
+  fs.read(buffer, 8);
+  if (!fs)
+    return false;
+  fs.close();
+
+  return true;
+}
+
+bool VerifyMessage(base::SyncSocket& sync_socket, const char* message_sent,
+                   size_t length) {
+  scoped_array<char> message_received(new char[length]);
+  sync_socket.Receive(message_received.get(), length);
+  return !::strcmp(message_received.get(), message_sent);
+}
+
+// Callback in the broker when a new broker connection occurs.
+int32_t OnInstanceConnected(PP_Instance instance, int32_t handle) {
+  base::PlatformFile platform_file = IntToPlatformFile(handle);
+  if (platform_file == base::kInvalidPlatformFileValue)
+    return PP_ERROR_FAILED;
+
+  base::SyncSocket sync_socket(platform_file);
+
+  // Send hello message.
+  size_t ret = sync_socket.Send(kHelloMessage, sizeof(kHelloMessage));
+  if (ret != sizeof(kHelloMessage))
+    return PP_ERROR_FAILED;
+
+  // Verify broker is not sandboxed and send result to plugin over the pipe.
+  if (VerifyIsUnsandboxed()) {
+    ret = sync_socket.Send(kBrokerIsUnsandboxed, sizeof(kBrokerIsUnsandboxed));
+    if (ret != sizeof(kBrokerIsUnsandboxed))
+      return PP_ERROR_FAILED;
+  }
+
+  return PP_OK;
+}
+
+} // namespace
+
+PP_EXPORT int32_t PPP_InitializeBroker(
+    PP_ConnectInstance_Func* connect_instance_func) {
+  *connect_instance_func = &OnInstanceConnected;
+  return PP_OK;
+}
+
+PP_EXPORT void PPP_ShutdownBroker() {}
+
 TestBroker::TestBroker(TestingInstance* instance)
     : TestCase(instance),
       broker_interface_(NULL) {
@@ -24,6 +129,7 @@ bool TestBroker::Init() {
 
 void TestBroker::RunTest() {
   RUN_TEST(Create);
+  RUN_TEST(ConnectAndGetHandle);
 }
 
 std::string TestBroker::TestCreate() {
@@ -33,14 +139,55 @@ std::string TestBroker::TestCreate() {
   ASSERT_TRUE(broker);
 
   ASSERT_FALSE(broker_interface_->IsBrokerTrusted(0));
-  ASSERT_TRUE(broker_interface_->IsBrokerTrusted(broker));
+  ASSERT_TRUE (broker_interface_->IsBrokerTrusted(broker));

[ddorwin, 2011/10/28 17:11:29]

Remove space.

[xhwang, 2011/10/28 18:18:28]

Done.

 
-  // Test getting the handle for an invalid resource.
-  int32_t handle;
-  ASSERT_TRUE(broker_interface_->GetHandle(0, &handle) == PP_ERROR_BADRESOURCE);
+  PASS();
+}
+
+std::string TestBroker::TestConnectAndGetHandle() {
+  PP_Resource broker = broker_interface_->CreateTrusted(
+      instance_->pp_instance());
+  ASSERT_TRUE(broker);
+
+  const int32_t kInvalidHandle =
+      PlatformFileToInt(base::kInvalidPlatformFileValue);
+  int32_t handle = kInvalidHandle;
 
+  // Test getting the handle for an invalid resource.
+  ASSERT_EQ(PP_ERROR_BADRESOURCE, broker_interface_->GetHandle(0, &handle));
   // Connect hasn't been called so this should fail.
-  ASSERT_TRUE(broker_interface_->GetHandle(broker, &handle) == PP_ERROR_FAILED);
+  ASSERT_EQ(PP_ERROR_FAILED, broker_interface_->GetHandle(broker, &handle));
+
+  // Connect should fail on invalid resource and since not force asynced, the
+  // callback will not be posted.
+  TestCompletionCallback cb_1(instance_->pp_instance());

[ddorwin, 2011/10/28 17:11:29]

I'd be explicit about the parameter since you refer to it:
TestCompletionCallback cb_1(instance_->pp_instance(), false);  // Not forced.
Note also the explanation of the bool literal. You can use this at 170 to avoid
an extra constant too.

[xhwang, 2011/10/28 18:18:28]

Done.

+  ASSERT_EQ(PP_ERROR_BADRESOURCE, broker_interface_->Connect(

[ddorwin, 2011/10/28 17:11:29]

This is a bit hard to read. Maybe this:
ASSERT_EQ(PP_ERROR_BADRESOURCE,
          broker_interface_->Connect(
              0, pp::CompletionCallback(cb_1).pp_completion_callback()));

[xhwang, 2011/10/28 18:18:28]

Done.

+        0, pp::CompletionCallback(cb_1).pp_completion_callback()));
+
+  // Since force asynced, Connect will return PP_OK_COMPLETIONPENDING and the
+  // callback will be posted.  However, the callback should fail.
+  const bool kForceAsync = true;
+  TestCompletionCallback cb_2(instance_->pp_instance(), kForceAsync);
+  ASSERT_EQ(PP_OK_COMPLETIONPENDING, broker_interface_->Connect(
+        0, pp::CompletionCallback(cb_2).pp_completion_callback()));
+  ASSERT_EQ(PP_ERROR_BADRESOURCE, cb_2.WaitForResult());
+
+  // Test normal case.

[ddorwin, 2011/10/28 17:11:29]

It's usually best to do one thing in a test, and if you have to add comments
like this, it's an indication you might want to make a separate the test.

In this case, you might name the above TestConnectAndGetHandle_Failures and the
below TestConnectAndGetHandle[_Success].

Thinking along the same lines (and the strange naming "TestConnectAndGetHandle",
it might make sense to separate the GetHandle tests and have three tests:
TestConnectFails
TestGetHandleFails
TestConnectAndPipe

[xhwang, 2011/10/28 18:18:28]

Done.

+  TestCompletionCallback cb_3(instance_->pp_instance());
+  ASSERT_EQ(PP_OK_COMPLETIONPENDING, broker_interface_->Connect(
+      broker, pp::CompletionCallback(cb_3).pp_completion_callback()));
+  ASSERT_EQ(PP_OK, cb_3.WaitForResult());
+
+  handle = kInvalidHandle;
+  ASSERT_EQ(PP_OK, broker_interface_->GetHandle(broker, &handle));
+  base::PlatformFile platform_file = IntToPlatformFile(handle);
+  ASSERT_NE(platform_file, base::kInvalidPlatformFileValue);
+
+  base::SyncSocket sync_socket(platform_file);
+
+  ASSERT_TRUE(VerifyMessage(sync_socket, kHelloMessage, sizeof(kHelloMessage)));
+  ASSERT_TRUE(VerifyMessage(
+      sync_socket, kBrokerIsUnsandboxed, sizeof(kBrokerIsUnsandboxed)));

[ddorwin, 2011/10/28 17:11:29]

Maybe:
ASSERT_TRUE(VerifyMessage(sync_socket, kBrokerIsUnsandboxed,
                          sizeof(kBrokerIsUnsandboxed)));

[xhwang, 2011/10/28 18:18:28]

Done.

 
   PASS();
 }