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

components/cronet/android/test/mock_url_request_job_factory.cc

Index: components/cronet/android/test/mock_url_request_job_factory.cc
diff --git a/components/cronet/android/test/mock_url_request_job_factory.cc b/components/cronet/android/test/mock_url_request_job_factory.cc
index 092df0ae1fd64c8633cf355bab57a6f7ed81d1c0..e698c1274d0d64e49b4c4d0a37c8bfc3b63c8351 100644
--- a/components/cronet/android/test/mock_url_request_job_factory.cc
+++ b/components/cronet/android/test/mock_url_request_job_factory.cc
@@ -6,11 +6,16 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
+#include "base/memory/ptr_util.h"
+#include "components/cronet/android/test/cronet_test_util.h"
 #include "jni/MockUrlRequestJobFactory_jni.h"
 #include "net/test/url_request/ssl_certificate_error_job.h"
 #include "net/test/url_request/url_request_failed_job.h"
 #include "net/test/url_request/url_request_hanging_read_job.h"
 #include "net/test/url_request/url_request_mock_data_job.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_intercepting_job_factory.h"
 #include "url/gurl.h"
 
 using base::android::JavaParamRef;
@@ -18,6 +23,49 @@ using base::android::ScopedJavaLocalRef;
 
 namespace cronet {
 
+// Intercept URLRequestJob creation using URLRequestFilter from
+// libcronet_tests.so
+class UrlInterceptorJobFactoryHandle {
+ public:
+  // |jcontext_adapter| points to a URLRequestContextAdapater.
+  UrlInterceptorJobFactoryHandle(jlong jcontext_adapter)
+      : jcontext_adapter_(jcontext_adapter) {
+    TestUtil::RunAfterContextInit(
+        jcontext_adapter,
+        base::Bind(&UrlInterceptorJobFactoryHandle::InitOnNetworkThread,
+                   base::Unretained(this)));
+  }
+  // Should only be called on network thread; other threads should use
+  // shutdown().
+  ~UrlInterceptorJobFactoryHandle() {
+    TestUtil::GetURLRequestContext(jcontext_adapter_)
+        ->set_job_factory(old_job_factory_);
+  }
+
+  void Shutdown() {

[xunjieli, 2016/10/27 14:49:43]

nit: Matt told me that Shutdown() is a noun, and ShutDown() is a verb phrase, so
s/Shutdown/ShutDown.

[pauljensen, 2016/10/27 18:43:26]

Done.  I should mention that Shutdown() appears 25x more than ShutDown() in
Chromium, but Matt's correct that shutdown is a noun.

+    TestUtil::GetTaskRunner(jcontext_adapter_)->DeleteSoon(FROM_HERE, this);
+  }
+
+ private:
+  void InitOnNetworkThread() {
+    net::URLRequestContext* request_context =
+        TestUtil::GetURLRequestContext(jcontext_adapter_);
+    old_job_factory_ = request_context->job_factory();
+    new_job_factory_.reset(new net::URLRequestInterceptingJobFactory(
+        const_cast<net::URLRequestJobFactory*>(old_job_factory_),
+        net::URLRequestFilter::GetInstance()));
+    request_context->set_job_factory(new_job_factory_.get());
+  }
+
+  // The URLRequestContextAdapater this object intercepts from.
+  const jlong jcontext_adapter_;
+  // URLRequestJobFactory previously used in URLRequestContext.
+  const net::URLRequestJobFactory* old_job_factory_;
+  // URLRequestJobFactory inserted during tests to intercept URLRequests with
+  // libcronet's URLRequestFilter.
+  std::unique_ptr<net::URLRequestInterceptingJobFactory> new_job_factory_;
+};
+
 void AddUrlInterceptors(JNIEnv* env, const JavaParamRef<jclass>& jcaller) {
   net::URLRequestMockDataJob::AddUrlHandler();
   net::URLRequestFailedJob::AddUrlHandler();
@@ -25,6 +73,26 @@ void AddUrlInterceptors(JNIEnv* env, const JavaParamRef<jclass>& jcaller) {
   net::SSLCertificateErrorJob::AddUrlHandler();
 }
 
+// URL interceptors are registered with the URLRequestFilter in
+// libcronet_tests.so.  However tests are run on libcronet.so.  Use the
+// URLRequestFilter in libcronet_tests.so with the URLRequestContext in
+// libcronet.so by installing a URLRequestInterceptingJobFactory
+// that calls into libcronet_tests.so's URLRequestFilter.
+jlong AddUrlInterceptorJobFactory(JNIEnv* env,

[xunjieli, 2016/10/27 14:49:43]

cleanup idea: maybe combine AddUrlInterceptorJobFactory and AddUrlInterceptors
into one function. It took me some time to differentiate those two. Since they
are almost doing the same thing, maybe we can use one JNI function.

[pauljensen, 2016/10/27 18:43:26]

Done.

+                                  const JavaParamRef<jclass>& jcaller,
+                                  jlong jcontext_adapter) {
+  return reinterpret_cast<jlong>(
+      new UrlInterceptorJobFactoryHandle(jcontext_adapter));
+}
+
+// Put back the old URLRequestJobFactory into the URLRequestContext.
+void RemoveUrlInterceptorJobFactory(JNIEnv* env,

[xunjieli, 2016/10/27 14:49:43]

Optional nit:

Use @NativeClassQualifiedName() and directly invoke Shutdown() on the handle.
That way, we can have one fewer static functions.

[pauljensen, 2016/10/27 18:43:25]

I tried to get this to work for a while, but it appears
@NativeClassQualifiedName doesn't really work with classes defined in .cc files
:(

+                                    const JavaParamRef<jclass>& jcaller,
+                                    jlong jinterceptor_handle) {
+  reinterpret_cast<UrlInterceptorJobFactoryHandle*>(jinterceptor_handle)
+      ->Shutdown();
+}
+
 ScopedJavaLocalRef<jstring> GetMockUrlWithFailure(
     JNIEnv* jenv,
     const JavaParamRef<jclass>& jcaller,