Move CreateFIFO() and RedirectStream() from testing/android/ to base/android/.

testing/android/native_test_launcher.cc

Index: testing/android/native_test_launcher.cc
diff --git a/testing/android/native_test_launcher.cc b/testing/android/native_test_launcher.cc
index 2fde735112c473b5dd35b55ff0f7164a398abd90..6a78ea77c71641a6c0093939d75adad4c9befd47 100644
--- a/testing/android/native_test_launcher.cc
+++ b/testing/android/native_test_launcher.cc
@@ -13,23 +13,21 @@
 #include <signal.h>
 
 #include "base/android/base_jni_registrar.h"
+#include "base/android/fifo_utils.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/at_exit.h"
 #include "base/base_switches.h"
 #include "base/command_line.h"
+#include "base/file_util.h"
 #include "base/files/file_path.h"
-#include "base/logging.h"
-#include "base/stringprintf.h"
 #include "gtest/gtest.h"
 #include "testing/android/native_test_util.h"
 #include "testing/jni/ChromeNativeTestActivity_jni.h"
 
 using testing::native_test_util::ArgsToArgv;
-using testing::native_test_util::CreateFIFO;
 using testing::native_test_util::ParseArgsFromCommandLineFile;
-using testing::native_test_util::RedirectStream;
 using testing::native_test_util::ScopedMainEntryLogger;
 
 // The main function of the program to be wrapped as a test apk.
@@ -78,6 +76,41 @@ void InstallHandlers() {
   }
 }
 
+// Writes printf() style string to Android's logger where |priority| is one of
+// the levels defined in <android/log.h>.
+void AndroidLog(int priority, const char* format, ...) {

[scherkus (not reviewing), 2013/06/07 21:30:57]

nileshagrawal: any reason why we can't use LOG()? these seems somewhat
duplicated from base/logging.cc

FWIW, I tested it out and it works.

[nilesh, 2013/06/08 02:11:03]

This was because logging::InitLogging in not called yet.
It is called later in TestSuite::Initialize

I have to look more closely to figure out if it is safe to use LOG here. bulach
may know better. 

[bulach, 2013/06/10 13:18:40]

that's right... the FIFO need(ed) to be setup as soon as possible so that test
harness in the host can capture it.. InitLogging and all other initializations
were done later on...

to be clear, this is not a hard-depedency, it can certainly be changed.. :) but
it may not be trivial.

[scherkus (not reviewing), 2013/06/14 23:19:33]

Gotcha -- I didn't notice before that the RunTests() code actually runs before
main()

+  va_list args;
+  va_start(args, format);
+  __android_log_vprint(priority, kLogTag, format, args);
+  va_end(args);
+}
+
+// Ensures that the fifo at |path| is created by deleting whatever is at |path|
+// prior to (re)creating the fifo, otherwise logs the error and terminates the
+// program.
+void EnsureCreateFIFO(const base::FilePath& path) {
+  file_util::Delete(path, false);

[bulach, 2013/06/10 13:18:40]

afaict, this has a ThreadRestrictions::AssertIOAllowed, which may kill the test
suite.. the previous unlink wouldn't have this issue..

again, not a hard-depedency, but it may fail on some test suites.

[scherkus (not reviewing), 2013/06/14 23:19:33]

If my understanding of when/how RunTests() is executed, there shouldn't have
been any ThreadRestrictions set on the thread by time we execute this code.

In any case, I'll get it a whirl on the try bots

+  if (base::android::CreateFIFO(path, 0666))
+    return;
+
+  AndroidLog(ANDROID_LOG_ERROR, "Failed to create fifo %s: %s\n",
+             path.value().c_str(), strerror(errno));
+  exit(EXIT_FAILURE);

[scherkus (not reviewing), 2013/06/07 21:30:57]

nileshagrawal: ditto here -- any reason we can't use CHECK() for these types of
helper functions?

[bulach, 2013/06/10 13:18:40]

as above, it's a chicken-and-egg problem :) we wanted to setup the environment
and start communicating with the harness as soon as possible... this requires
logging, command line, etc.., to be fully initialized before the we can start
communicating with the harness..

+}
+
+// Ensures that |stream| is redirected to |path|, otherwise logs the error and
+// terminates the program.
+void EnsureRedirectStream(FILE* stream,
+                          const base::FilePath& path,
+                          const char* mode) {
+  if (base::android::RedirectStream(stream, path, mode))
+    return;
+
+  AndroidLog(ANDROID_LOG_ERROR, "Failed to redirect stream to file: %s: %s\n",
+             path.value().c_str(), strerror(errno));
+  exit(EXIT_FAILURE);
+}
+
 }  // namespace
 
 // This method is called on a separate java thread so that we won't trigger
@@ -115,7 +148,7 @@ static void RunTests(JNIEnv* env,
   // A few options, such "--gtest_list_tests", will just use printf directly
   // Always redirect stdout to a known file.
   base::FilePath fifo_path(files_dir.Append(base::FilePath("test.fifo")));
-  CreateFIFO(fifo_path.value().c_str());
+  EnsureCreateFIFO(fifo_path);
 
   base::FilePath stderr_fifo_path, stdin_fifo_path;
 
@@ -123,29 +156,28 @@ static void RunTests(JNIEnv* env,
   // other tests, insert stderr content to the same fifo we use for stdout.
   if (command_line.HasSwitch(kSeparateStderrFifo)) {
     stderr_fifo_path = files_dir.Append(base::FilePath("stderr.fifo"));
-    CreateFIFO(stderr_fifo_path.value().c_str());
+    EnsureCreateFIFO(stderr_fifo_path);
   }
 
   // DumpRenderTree uses stdin to receive input about which test to run.
   if (command_line.HasSwitch(kCreateStdinFifo)) {
     stdin_fifo_path = files_dir.Append(base::FilePath("stdin.fifo"));
-    CreateFIFO(stdin_fifo_path.value().c_str());
+    EnsureCreateFIFO(stdin_fifo_path);
   }
 
   // Only redirect the streams after all fifos have been created.
-  RedirectStream(stdout, fifo_path.value().c_str(), "w");
+  EnsureRedirectStream(stdout, fifo_path, "w");
   if (!stdin_fifo_path.empty())
-    RedirectStream(stdin, stdin_fifo_path.value().c_str(), "r");
+    EnsureRedirectStream(stdin, stdin_fifo_path, "r");
   if (!stderr_fifo_path.empty())
-    RedirectStream(stderr, stderr_fifo_path.value().c_str(), "w");
+    EnsureRedirectStream(stderr, stderr_fifo_path, "w");
   else
     dup2(STDOUT_FILENO, STDERR_FILENO);
 
   if (command_line.HasSwitch(switches::kWaitForDebugger)) {
-    std::string msg = base::StringPrintf("Native test waiting for GDB because "
-                                         "flag %s was supplied",
-                                         switches::kWaitForDebugger);
-    __android_log_write(ANDROID_LOG_VERBOSE, kLogTag, msg.c_str());
+    AndroidLog(ANDROID_LOG_VERBOSE,
+               "Native test waiting for GDB because flag %s was supplied",
+               switches::kWaitForDebugger);
     base::debug::WaitForDebugger(24 * 60 * 60, false);
   }