[Cronet] Test the libcronet that's shipped, not libcronet_test

components/cronet/android/test/cronet_test_util.cc

Index: components/cronet/android/test/cronet_test_util.cc
diff --git a/components/cronet/android/test/cronet_test_util.cc b/components/cronet/android/test/cronet_test_util.cc
index a44e484fab27ca7aae259ac3f37eddc2360e8b90..aa164a3ebc4f24601aeb55ac2bae6eeb8b540d1a 100644
--- a/components/cronet/android/test/cronet_test_util.cc
+++ b/components/cronet/android/test/cronet_test_util.cc
@@ -6,7 +6,8 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
-#include "components/cronet/android/cronet_url_request_adapter.h"
+#include "base/message_loop/message_loop.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "components/cronet/android/test/native_test_server.h"
 #include "jni/CronetTestUtil_jni.h"
 #include "net/url_request/url_request.h"
@@ -15,12 +16,87 @@ using base::android::JavaParamRef;
 
 namespace cronet {
 
+namespace {
+
+// A SingleThreadTaskRunner that works when a thread's SingleThreadTaskRunner
+// isn't accessible to native code, instead this implementation works by calling
+// out to Java which can access libcronet's network threads by issuing
+// UrlRequests with direct-executors.
+class JavaTaskRunner : public base::SingleThreadTaskRunner {

[kapishnikov, 2016/10/20 17:25:46]

Would it be better if we had a non-static CronetTestContext that would keep a
reference to all libcronet objects that it needs, e.g. net::URLRequestContext or
CronetURLRequestContextAdapter? Would it eliminate the need for jumping from C
to Java and back? I think this solution is good but it looks a little hackish
and introduces even more static object/methods, e.g. the static
postClosureToNetworkThread method in CronetTestUtils.java. What if in the future
we need to test multiple engines simultaneously?

[pauljensen, 2016/10/21 02:27:33]

It would not eliminate the jumping back and forth between C++ and Java.  To
implement SingleThreadTaskRunner without access to the actual
SingleThreadTaskRunner managing this thread, we have to jump from C++ to Java
because we have the ability to post to the network thread in Java but not in
C++.  Likewise we have to jump from Java to C++ to execute the Closure because
Closure is a C++ concept.
I'm not a fan of the static stuff either, but I think it's somewhat simpler than
the alternative of creating matched C++ and Java objects.  The C++ and Java
objects each need to hold a pointer to the other, and we'd need to add some JNI
to create the objects.  I don't feel strongly though.  Do you think it's worth
it to change to non-static paired Java and C++ objects?

+ public:
+  JavaTaskRunner(base::MessageLoop* message_loop)
+      : message_loop_(message_loop) {}
+  virtual ~JavaTaskRunner() {}
+
+  bool PostDelayedTask(const tracked_objects::Location& from_here,
+                       const base::Closure& task,
+                       base::TimeDelta delay) override {
+    // Use Java to post back to the network thread.  There is no native access
+    // to the libcronet.so MessageLoop or SingleThreadTaskRunner.
+    Java_CronetTestUtil_postClosureToNetworkThread(
+        base::android::AttachCurrentThread(),
+        reinterpret_cast<jlong>(new base::Closure(task)));
+    return true;
+  }
+
+  bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here,
+                                  const base::Closure& task,
+                                  base::TimeDelta delay) override {
+    return PostDelayedTask(from_here, task, delay);
+  }
+
+  bool RunsTasksOnCurrentThread() const override {
+    return message_loop_ == base::MessageLoop::current();
+  }
+
+ private:
+  // MessageLoop that owns this instance.
+  base::MessageLoop* message_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(JavaTaskRunner);
+};
+
+// MessageLoop for libcronet.so's network threads, to be accessed by
+// libcronet_tests.so methods.
+static std::unique_ptr<base::MessageLoop> message_loop;
+
+}  // namespace
+
 jint GetLoadFlags(JNIEnv* env,
                   const JavaParamRef<jclass>& jcaller,
                   const jlong urlRequest) {
-  return reinterpret_cast<CronetURLRequestAdapter*>(urlRequest)
-      ->GetURLRequestForTesting()
-      ->load_flags();
+  return reinterpret_cast<net::URLRequest*>(urlRequest)->load_flags();
+}
+
+// Tests need to call into libcronet.so code on libcronet.so threads.
+// libcronet.so's threads are registered with static tables for MessageLoops
+// and SingleThreadTaskRunners in libcronet.so, so libcronet_test.so
+// functions that try and access these tables will find missing entries in
+// the corresponding static tables in libcronet_test.so.  Fix this by
+// initializing a MessageLoop and SingleThreadTaskRunner in libcronet_test.so
+// for these threads.
+void PrepareNetworkThreadForTesting(JNIEnv* env,
+                                    const JavaParamRef<jclass>& jcaller) {
+  message_loop.reset(new base::MessageLoopForIO());
+  message_loop->SetTaskRunner(new JavaTaskRunner(message_loop.get()));
+}
+
+jlong GetNativeSingleThreadTaskRunner(JNIEnv* env,
+                                      const JavaParamRef<jclass>& jcaller) {
+  return reinterpret_cast<jlong>(message_loop->task_runner().get());
+}
+
+void CleanupNetorkThreadForTesting(JNIEnv* env,
+                                   const JavaParamRef<jclass>& jcaller) {
+  message_loop.reset();
+}
+
+void RunClosure(JNIEnv* env,
+                const JavaParamRef<jclass>& jcaller,
+                jlong jclosure) {
+  base::Closure* closure = reinterpret_cast<base::Closure*>(jclosure);
+  closure->Run();
+  delete closure;
 }
 
 bool RegisterCronetTestUtil(JNIEnv* env) {