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..57399c21d6757222dc04297c1599a8889d66ccbf 100644
--- a/ppapi/tests/test_broker.cc
+++ b/ppapi/tests/test_broker.cc
@@ -4,13 +4,222 @@
 
 #include "ppapi/tests/test_broker.h"
 
+#include <cstdio>
+#include <cstring>
+#include <fstream>
+#include <limits>
+
+#include "base/memory/scoped_ptr.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"
 
+#if defined(OS_WIN)
+#include "Windows.h"

[piman, 2011/11/14 23:49:17]

Should be written <windows.h>

[xhwang, 2011/11/15 01:52:49]

Done.

+#elif !defined(OS_POSIX)
+#error Not implemented. Don't remove as it's only checked here in this file.
+#endif
+
 REGISTER_TEST_CASE(Broker);
 
+namespace {
+
+const char kHelloMessage[] = "Hello Plugin! This is Broker!";
+const char kBrokerUnsandboxed[] = "Broker is Unsandboxed!";
+
+#if defined(OS_WIN)
+typedef HANDLE PlatformFile;
+const PlatformFile kInvalidPlatformFileValue = INVALID_HANDLE_VALUE;
+const int32_t kInvalidHandle = static_cast<int32_t>(
+    reinterpret_cast<intptr_t>(INVALID_HANDLE_VALUE));
+#elif defined(OS_POSIX)
+typedef int PlatformFile;
+const PlatformFile kInvalidPlatformFileValue = -1;
+const int32_t kInvalidHandle = -1;
+#endif
+
+PlatformFile IntToPlatformFile(int32_t handle) {
+#if defined(OS_WIN)
+  return reinterpret_cast<HANDLE>(static_cast<intptr_t>(handle));
+#elif defined(OS_POSIX)
+  return handle;
+#endif
+}
+
+bool ReadFromPlatformFile(PlatformFile file, size_t bytes_to_read,
+                          char* buffer, size_t* bytes_read) {
+#if defined(OS_WIN)
+  assert(bytes_to_read < std::numeric_limits<DWORD>::max());
+  DWORD read = 0;
+  if (!::ReadFile(file, buffer, static_cast<DWORD>(bytes_to_read), &read, NULL))
+    return false;
+  *bytes_read = static_cast<size_t>(read);
+#elif defined(OS_POSIX)
+  assert(bytes_to_read <
+         static_cast<size_t>(std::numeric_limits<ssize_t>::max()));
+  ssize_t ret = ::read(file, buffer, bytes_to_read);

[piman, 2011/11/14 23:49:17]

FYI, ::read and ::write may fail to fully read/write bytes_to_read bytes if
interrupted by a signal, and may need to be restarted (see HANDLE_EINTR in
base/eintr_wrapper). I don't know if the windows equivalents have this issue
too.

[xhwang, 2011/11/15 01:52:49]

Done.

+  if (ret == -1)
+    return false;
+  *bytes_read = static_cast<size_t>(ret);
+#endif
+
+  return true;
+}
+
+bool WriteToPlatformFile(PlatformFile file, size_t bytes_to_write,
+                         const char* buffer, size_t* bytes_written) {
+#if defined(OS_WIN)
+  assert(bytes_to_write < std::numeric_limits<DWORD>::max());
+  DWORD written = 0;
+  if (!::WriteFile(file, buffer, static_cast<DWORD>(bytes_to_write), &written,
+                   NULL))
+    return false;
+  *bytes_written = static_cast<size_t>(written);
+#elif defined(OS_POSIX)
+  assert(bytes_to_write <
+         static_cast<size_t>(std::numeric_limits<ssize_t>::max()));
+  ssize_t ret = ::write(file, buffer, bytes_to_write);
+  if (ret == -1)
+    return false;
+  *bytes_written = static_cast<size_t>(ret);
+#endif
+
+  return true;
+}
+
+bool ClosePlatformFile(PlatformFile file) {
+#if defined(OS_WIN)
+  return !!::CloseHandle(file);
+#elif defined(OS_POSIX)
+  return !::close(file);
+#endif
+}
+
+bool WriteMessage(PlatformFile file, size_t message_len, const char* message) {
+  size_t bytes_written = 0;
+  return  WriteToPlatformFile(file, message_len, message, &bytes_written) &&
+          bytes_written == message_len;
+}
+
+bool ReadMessage(PlatformFile file, size_t message_len, char* message) {
+  size_t bytes_read = 0;
+  return ReadFromPlatformFile(file, message_len, message, &bytes_read) &&
+         bytes_read == message_len;
+}
+
+bool VerifyMessage(PlatformFile file, size_t message_len, const char* message) {
+  scoped_array<char> message_received(new char[message_len]);
+  return ReadMessage(file, message_len, message_received.get()) &&
+         !::strcmp(message_received.get(), message);
+}
+
+// May not delete file if fails. If these tests are ever converted to Google
+// Test, use EXPECT_* to ensure the file is deleted.
+bool VerifyIsUnsandboxed() {
+  FILE* file = NULL;
+
+#if defined(OS_WIN)
+  wchar_t temp_path[MAX_PATH] = {'\0'};
+  wchar_t file_name[MAX_PATH] = {'\0'};
+  if (!::GetTempPath(MAX_PATH, temp_path) ||
+      !::GetTempFileName(temp_path, L"test_pepper_broker", 0, file_name) ||
+      ::_wfopen_s(&file, file_name, L"w"))
+    return false;
+#elif defined(OS_POSIX)
+  char file_name[] = "/tmp/test_pepper_broker_XXXXXX";
+  int fd = ::mkstemp(file_name);
+  if (-1 == fd)
+    return false;
+
+  file = ::fdopen(fd, "w");
+  if (!file) {
+    ::remove(file_name);
+    ::close(fd);
+    return false;
+  }
+#endif

[piman, 2011/11/14 23:49:17]

I'd say, at this point, you should know whether you're sandboxed or not.
Creating the temp file should be disallowed on all platforms if the sandbox is
on. So I think you could stop here rather than adding more complexity (and
ifdefs) below.

[xhwang, 2011/11/15 01:52:49]

Done.

+
+  const char text[] = "Verify that the broker is unsandboxed.";
+  const size_t text_len = sizeof(text);
+  if (text_len != ::fwrite(text, 1, text_len, file)) {
+    ::fclose(file);
+    return false;
+  }
+
+  if (::fclose(file))
+    return false;
+
+  file = NULL;
+
+#if defined(OS_WIN)
+  if (::_wfopen_s(&file, file_name, L"r"))
+    return false;
+#elif defined(OS_POSIX)
+  file = ::fopen(file_name, "r");
+  if (!file)
+    return false;
+#endif
+
+  scoped_array<char> buffer(new char[text_len]);
+  if (text_len != ::fread(buffer.get(), 1, text_len, file)) {
+    ::fclose(file);
+    return false;
+  }
+
+  if (::fclose(file))  // Will also close |fd| in POSIX.
+    return false;
+
+#if defined(OS_WIN)
+  if (!::DeleteFile(file_name))
+    return false;
+#elif defined(OS_POSIX)
+  if (::remove(file_name))
+    return false;
+#endif
+
+  return true;
+}
+
+// Callback in the broker when a new broker connection occurs.
+int32_t OnInstanceConnected(PP_Instance instance, int32_t handle) {
+  PlatformFile file = IntToPlatformFile(handle);
+  if (file == kInvalidPlatformFileValue)
+    return PP_ERROR_FAILED;
+
+  // Send hello message.
+  if (!WriteMessage(file, sizeof(kHelloMessage), kHelloMessage)) {
+    ClosePlatformFile(file);
+    return PP_ERROR_FAILED;
+  }
+
+  // Verify broker is not sandboxed and send result to plugin over the pipe.
+  if (VerifyIsUnsandboxed()) {
+    if (!WriteMessage(file, sizeof(kBrokerUnsandboxed), kBrokerUnsandboxed)) {

[piman, 2011/11/14 23:49:17]

It would be better I think to write a response message whether the broker is
sandboxed or not. Right now I think you're relying on the fact that you close
the socket below to make the ReadMessage on the client side fail. If the
connection doesn't properly close for any reason (e.g. recent bug), then the
test will hang and timeout, making it flaky and hard to debug. It seems
preferable to send a "failure" message that's unambiguous.

[xhwang, 2011/11/15 01:52:49]

Done.

+      ClosePlatformFile(file);
+      return PP_ERROR_FAILED;
+    }
+  }
+  // If kBrokerUnsandboxed is not received by the plugin, the plugin knows that
+  // the broker is sandboxed. So no "else" needed here.
+
+  if (!ClosePlatformFile(file))
+    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 +233,11 @@ bool TestBroker::Init() {
 
 void TestBroker::RunTest() {
   RUN_TEST(Create);
+  RUN_TEST(GetHandleFailure);
+  RUN_TEST(ConnectFailure);
+#if !defined(OS_WIN) // This is broken on Windows. See http://crbug.com/103975.
+  RUN_TEST(ConnectAndPipe);
+#endif
 }
 
 std::string TestBroker::TestCreate() {
@@ -35,12 +249,65 @@ std::string TestBroker::TestCreate() {
   ASSERT_FALSE(broker_interface_->IsBrokerTrusted(0));
   ASSERT_TRUE(broker_interface_->IsBrokerTrusted(broker));
 
+  PASS();
+}
+
+// Test connection on invalid resource.
+std::string TestBroker::TestConnectFailure() {
+  // Callback NOT force async. Connect should fail.  The callback will not be
+  // posted so there's no need to wait for the callback to complete.
+  TestCompletionCallback cb_1(instance_->pp_instance(), false);
+  ASSERT_EQ(PP_ERROR_BADRESOURCE,
+            broker_interface_->Connect(
+                0, pp::CompletionCallback(cb_1).pp_completion_callback()));
+
+  // Callback force async. Connect will return PP_OK_COMPLETIONPENDING and the
+  // callback will be posted.  However, the callback should fail.
+  TestCompletionCallback cb_2(instance_->pp_instance(), true);
+  ASSERT_EQ(PP_OK_COMPLETIONPENDING,
+            broker_interface_->Connect(
+                0, pp::CompletionCallback(cb_2).pp_completion_callback()));
+  ASSERT_EQ(PP_ERROR_BADRESOURCE, cb_2.WaitForResult());
+
+  PASS();
+}
+
+std::string TestBroker::TestGetHandleFailure() {
+  int32_t handle = kInvalidHandle;
+
   // Test getting the handle for an invalid resource.
-  int32_t handle;
-  ASSERT_TRUE(broker_interface_->GetHandle(0, &handle) == PP_ERROR_BADRESOURCE);
+  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);
+  PP_Resource broker = broker_interface_->CreateTrusted(
+      instance_->pp_instance());
+  ASSERT_TRUE(broker);
+  ASSERT_EQ(PP_ERROR_FAILED, broker_interface_->GetHandle(broker, &handle));
+
+  PASS();
+}
+
+std::string TestBroker::TestConnectAndPipe() {
+  PP_Resource broker = broker_interface_->CreateTrusted(
+      instance_->pp_instance());
+  ASSERT_TRUE(broker);
+
+  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());
+
+  int32_t handle = kInvalidHandle;
+  ASSERT_EQ(PP_OK, broker_interface_->GetHandle(broker, &handle));
+  ASSERT_NE(kInvalidHandle, handle);
+
+  PlatformFile file = IntToPlatformFile(handle);
+  ASSERT_TRUE(VerifyMessage(file, sizeof(kHelloMessage), kHelloMessage));
+  ASSERT_TRUE(VerifyMessage(file, sizeof(kBrokerUnsandboxed),
+                            kBrokerUnsandboxed));
+
+  ASSERT_TRUE(ClosePlatformFile(file));
 
   PASS();
 }