[Cronet] Provide bidirectional stream support from native CronetEngine.

Bug: 874542
Change-Id: I8414e75049ee219f1c7491caf91f6282c42f23dd
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/1178666
Commit-Queue: Misha Efimov <mef@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588541}

components/cronet/cronet_url_request_context.cc

@@ -456,6 +456,40 @@ CronetURLRequestContext::NetworkTasks::GetURLRequestContext() {
   return context_.get();
 }
 
+// Request context getter for CronetURLRequestContext.
+class CronetURLRequestContext::ContextGetter
+    : public net::URLRequestContextGetter {
+ public:
+  explicit ContextGetter(CronetURLRequestContext* cronet_context)
+      : cronet_context_(cronet_context) {
+    DCHECK(cronet_context_);
+  }
+
+  net::URLRequestContext* GetURLRequestContext() override {
+    return cronet_context_->GetURLRequestContext();
+  }
+
+  scoped_refptr<base::SingleThreadTaskRunner> GetNetworkTaskRunner()
+      const override {
+    return cronet_context_->GetNetworkTaskRunner();
+  }
+
+ private:
+  // Must be called on the network thread.
+  ~ContextGetter() override { DCHECK(cronet_context_->IsOnNetworkThread()); }
+
+  // CronetURLRequestContext associated with this ContextGetter.
+  CronetURLRequestContext* const cronet_context_;
+
+  DISALLOW_COPY_AND_ASSIGN(ContextGetter);
+};
+
+net::URLRequestContextGetter*
+CronetURLRequestContext::CreateURLRequestContextGetter() {
+  DCHECK(IsOnNetworkThread());
+  return new ContextGetter(this);
+}
+
 net::URLRequestContext* CronetURLRequestContext::GetURLRequestContext() {
   DCHECK(IsOnNetworkThread());
   return network_tasks_->GetURLRequestContext();

components/cronet/cronet_url_request_context.h

@@ -34,6 +34,7 @@ enum EffectiveConnectionType;
 class NetLog;
 class ProxyConfigService;
 class URLRequestContext;
+class URLRequestContextGetter;
 class FileNetLogObserver;
 }  // namespace net
 
@@ -113,8 +114,14 @@ class CronetURLRequestContext {
   // Returns true if running on network thread.
   bool IsOnNetworkThread() const;
 
+  // Returns net::URLRequestContext owned by |this|.
   net::URLRequestContext* GetURLRequestContext();
 
+  // Returns a new instance of net::URLRequestContextGetter.
+  // The net::URLRequestContext and base::SingleThreadTaskRunner that
+  // net::URLRequestContextGetter returns are owned by |this|.
+  net::URLRequestContextGetter* CreateURLRequestContextGetter();
+
   // TODO(xunjieli): Keep only one version of StartNetLog().
 
   // Starts NetLog logging to file. This can be called on any thread.
@@ -150,6 +157,7 @@ class CronetURLRequestContext {
 
  private:
   friend class TestUtil;
+  class ContextGetter;
 
   // NetworkTasks performs tasks on the network thread and owns objects that
   // live on the network thread.

components/cronet/ios/BUILD.gn

@@ -113,7 +113,7 @@ ios_framework_bundle("cronet_framework") {
   libs = [ "UIKit.Framework" ]
 
   public_deps = [
-    "//components/grpc_support",
+    "//components/grpc_support:headers",
   ]
 
   public_headers = _cronet_public_headers

components/cronet/ios/test/BUILD.gn

@@ -19,7 +19,9 @@ test("cronet_test") {
     "cronet_quic_test.mm",
     "cronet_test_base.h",
     "cronet_test_base.mm",
-    "get_stream_engine.mm",
+
+    # Use native stream engine instead (https://crbug.com/874542)
+    # "get_stream_engine.mm",
     "start_cronet.h",
     "start_cronet.mm",
   ]
@@ -30,7 +32,7 @@ test("cronet_test") {
     "//components/cronet/ios:cronet_framework+link",
     "//components/cronet/native/test:cronet_native_tests",
     "//components/cronet/test:test_support",
-    "//components/grpc_support:bidirectional_stream_unittest",
+    "//components/grpc_support:bidirectional_stream_test",
     "//net",
     "//net:simple_quic_tools",
     "//net:test_support",

components/cronet/native/BUILD.gn

@@ -3,18 +3,21 @@
 # found in the LICENSE file.
 
 import("//components/cronet/native/include/headers.gni")
+import("//components/grpc_support/include/headers.gni")
 import("//testing/test.gni")
 
 config("cronet_native_include_config") {
   include_dirs = [
     "//components/cronet/native/generated",
     "//components/cronet/native/include",
+    "//components/grpc_support/include",
   ]
 }
 
 source_set("cronet_native_headers") {
   deps = [
     "//base",
+    "//components/grpc_support:headers",
   ]
 
   configs += [ ":cronet_native_include_config" ]
@@ -38,6 +41,7 @@ source_set("cronet_native_impl") {
     "//base",
     "//components/cronet:cronet_common",
     "//components/cronet:cronet_version_header",
+    "//components/grpc_support:grpc_support",
     "//net",
   ]
 

components/cronet/native/engine.cc

@@ -20,9 +20,11 @@
 #include "components/cronet/native/include/cronet_c.h"
 #include "components/cronet/url_request_context_config.h"
 #include "components/cronet/version.h"
+#include "components/grpc_support/include/bidirectional_stream_c.h"
 #include "net/base/hash_value.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_builder.h"
+#include "net/url_request/url_request_context_getter.h"
 
 namespace {
 
@@ -150,6 +152,8 @@ Cronet_RESULT Cronet_EngineImpl::StartWithParams(
         params->network_thread_priority;
   }
 
+  // MockCertVerifier to use for testing purposes.
+  context_config_builder.mock_cert_verifier = std::move(mock_cert_verifier_);
   std::unique_ptr<URLRequestContextConfig> config =
       context_config_builder.Build();
 
@@ -249,6 +253,7 @@ Cronet_RESULT Cronet_EngineImpl::Shutdown() {
         in_use_storage_path_);
   }
 
+  stream_engine_.reset();
   context_.reset();
   return CheckResult(Cronet_RESULT_SUCCESS);
 }
@@ -259,6 +264,25 @@ Cronet_RESULT Cronet_EngineImpl::CheckResult(Cronet_RESULT result) {
   return result;
 }
 
+// The struct stream_engine for grpc support.
+// Holds net::URLRequestContextGetter and app-specific annotation.
+class Cronet_EngineImpl::StreamEngineImpl : public stream_engine {
+ public:
+  explicit StreamEngineImpl(net::URLRequestContextGetter* context_getter) {
+    context_getter_ = context_getter;
+    obj = context_getter_.get();
+    annotation = nullptr;
+  }
+
+  ~StreamEngineImpl() {
+    obj = nullptr;
+    annotation = nullptr;
+  }
+
+ private:
+  scoped_refptr<net::URLRequestContextGetter> context_getter_;
+};
+
 // Callback is owned by CronetURLRequestContext. It is invoked and deleted
 // on the network thread.
 class Cronet_EngineImpl::Callback : public CronetURLRequestContext::Callback {
@@ -306,12 +330,16 @@ void Cronet_EngineImpl::Callback::OnInitNetworkThread() {
   // being intialized on network thread.
   base::AutoLock lock(engine_->lock_);
   if (engine_->context_) {
+    // Initialize bidirectional stream engine for grpc.
+    engine_->stream_engine_ = std::make_unique<StreamEngineImpl>(
+        engine_->context_->CreateURLRequestContextGetter());
     engine_->init_completed_.Signal();
   }
 }
 
 void Cronet_EngineImpl::Callback::OnDestroyNetworkThread() {
   DCHECK_CALLED_ON_VALID_THREAD(network_thread_checker_);
+  DCHECK(!engine_->stream_engine_);
 }
 
 void Cronet_EngineImpl::Callback::OnEffectiveConnectionTypeChanged(
@@ -349,8 +377,36 @@ void Cronet_EngineImpl::Callback::OnStopNetLogCompleted() {
   engine_->stop_netlog_completed_.Signal();
 }
 
+void Cronet_EngineImpl::SetMockCertVerifierForTesting(
+    std::unique_ptr<net::CertVerifier> mock_cert_verifier) {
+  CHECK(!context_);
+  mock_cert_verifier_ = std::move(mock_cert_verifier);
+}
+
+stream_engine* Cronet_EngineImpl::GetBidirectionalStreamEngine() {
+  init_completed_.Wait();
+  return stream_engine_.get();
+}
+
 };  // namespace cronet
 
 CRONET_EXPORT Cronet_EnginePtr Cronet_Engine_Create() {
   return new cronet::Cronet_EngineImpl();
 }
+
+CRONET_EXPORT void Cronet_Engine_SetMockCertVerifierForTesting(
+    Cronet_EnginePtr engine,
+    void* raw_mock_cert_verifier) {
+  cronet::Cronet_EngineImpl* engine_impl =
+      static_cast<cronet::Cronet_EngineImpl*>(engine);
+  std::unique_ptr<net::CertVerifier> cert_verifier;
+  cert_verifier.reset(static_cast<net::CertVerifier*>(raw_mock_cert_verifier));
+  engine_impl->SetMockCertVerifierForTesting(std::move(cert_verifier));
+}
+
+CRONET_EXPORT stream_engine* Cronet_Engine_GetStreamEngine(
+    Cronet_EnginePtr engine) {
+  cronet::Cronet_EngineImpl* engine_impl =
+      static_cast<cronet::Cronet_EngineImpl*>(engine);
+  return engine_impl->GetBidirectionalStreamEngine();
+}

components/cronet/native/engine.h

@@ -13,8 +13,13 @@
 #include "base/synchronization/waitable_event.h"
 #include "components/cronet/native/generated/cronet.idl_impl_interface.h"
 
-namespace cronet {
+extern "C" typedef struct stream_engine stream_engine;
+
+namespace net {
+class CertVerifier;
+}
 
+namespace cronet {
 class CronetURLRequestContext;
 
 // Implementation of Cronet_Engine that uses CronetURLRequestContext.
@@ -35,11 +40,20 @@ class Cronet_EngineImpl : public Cronet_Engine {
   // is true.
   Cronet_RESULT CheckResult(Cronet_RESULT result);
 
+  // Set Mock CertVerifier for testing. Must be called before StartWithParams.
+  void SetMockCertVerifierForTesting(
+      std::unique_ptr<net::CertVerifier> mock_cert_verifier);
+
+  // Get stream engine for GRPC Bidirectional Stream support. The returned
+  // stream_engine is owned by |this| and is only valid until |this| shutdown.
+  stream_engine* GetBidirectionalStreamEngine();
+
   CronetURLRequestContext* cronet_url_request_context() const {
     return context_.get();
   }
 
  private:
+  class StreamEngineImpl;
   class Callback;
 
   // Enable runtime CHECK of the result.
@@ -60,6 +74,12 @@ class Cronet_EngineImpl : public Cronet_Engine {
   // Storage path used by this engine.
   std::string in_use_storage_path_;
 
+  // Stream engine for GRPC Bidirectional Stream support.
+  std::unique_ptr<StreamEngineImpl> stream_engine_;
+
+  // Mock CertVerifier for testing. Only valid until StartWithParams.
+  std::unique_ptr<net::CertVerifier> mock_cert_verifier_;
+
   DISALLOW_COPY_AND_ASSIGN(Cronet_EngineImpl);
 };
 

components/cronet/native/include/cronet_c.h

@@ -7,14 +7,29 @@
 
 #include "cronet_export.h"
 
-// Cronet public C API is generated from cronet.idl and
+// Cronet public C API is generated from cronet.idl
 #include "cronet.idl_c.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-// TODO(mef): Remove this file if it is not needed.
+// Stream Engine used by Bidirectional Stream C API for GRPC.
+typedef struct stream_engine stream_engine;
+
+// Additional Cronet C API not generated from cronet.idl.
+
+// Sets net::CertVerifier* raw_mock_cert_verifier for testing of Cronet_Engine.
+// Must be called before Cronet_Engine_InitWithParams().
+CRONET_EXPORT void Cronet_Engine_SetMockCertVerifierForTesting(
+    Cronet_EnginePtr engine,
+    /* net::CertVerifier* */ void* raw_mock_cert_verifier);
+
+// Returns "stream_engine" interface for bidirectionsl stream support for GRPC.
+// Returned stream engine is owned by Cronet Engine and is only valid until
+// Cronet_Engine_Shutdown().
+CRONET_EXPORT stream_engine* Cronet_Engine_GetStreamEngine(
+    Cronet_EnginePtr engine);
 
 #ifdef __cplusplus
 }

components/cronet/native/test/BUILD.gn

@@ -34,6 +34,8 @@ source_set("cronet_native_tests") {
     "//base/test:test_support",
     "//components/cronet/native:cronet_native_headers",
     "//components/cronet/test:test_support",
+    "//components/grpc_support:bidirectional_stream_test",
+    "//components/grpc_support/test:get_stream_engine_header",
     "//net:test_support",
     "//testing/gtest",
   ]
@@ -44,6 +46,7 @@ source_set("cronet_native_tests") {
     "buffer_test.cc",
     "engine_test.cc",
     "executors_test.cc",
+    "test_stream_engine.cc",
     "test_upload_data_provider.cc",
     "test_upload_data_provider.h",
     "test_url_request_callback.cc",

components/cronet/native/test/engine_test.cc

@@ -11,6 +11,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "components/cronet/native/test/test_util.h"
+#include "net/cert/mock_cert_verifier.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -156,6 +157,18 @@ TEST_F(EngineTest, ValidPkpParams) {
   Cronet_PublicKeyPins_Destroy(public_key_pins);
 }
 
+// Verify that Cronet_Engine_SetMockCertVerifierForTesting() doesn't crash or
+// leak anything.
+TEST_F(EngineTest, SetMockCertVerifierForTesting) {
+  auto cert_verifier(std::make_unique<net::MockCertVerifier>());
+  Cronet_EnginePtr engine = Cronet_Engine_Create();
+  Cronet_Engine_SetMockCertVerifierForTesting(engine, cert_verifier.release());
+  Cronet_EngineParamsPtr engine_params = Cronet_EngineParams_Create();
+  Cronet_Engine_StartWithParams(engine, engine_params);
+  Cronet_Engine_Destroy(engine);
+  Cronet_EngineParams_Destroy(engine_params);
+}
+
 TEST_F(EngineTest, StartNetLogToFile) {
   base::ScopedTempDir temp_dir;
   EXPECT_TRUE(temp_dir.CreateUniqueTempDir());

components/cronet/native/test/test_stream_engine.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "cronet_c.h"
+
+#include "base/logging.h"
+#include "components/cronet/native/test/test_util.h"
+#include "components/grpc_support/test/get_stream_engine.h"
+
+namespace grpc_support {
+
+// Provides stream_engine support for testing of bidirectional_stream C API for
+// GRPC using native Cronet_Engine.
+
+Cronet_EnginePtr g_cronet_engine = nullptr;
+int quic_server_port = 0;
+
+// Returns a stream_engine* for testing with the QuicTestServer.
+// The engine returned resolve "test.example.com" as localhost:|port|,
+// and should have "test.example.com" configured as a QUIC server.
+stream_engine* GetTestStreamEngine(int port) {
+  CHECK(g_cronet_engine);
+  CHECK_EQ(port, quic_server_port);
+  return Cronet_Engine_GetStreamEngine(g_cronet_engine);
+}
+
+// Starts the stream_engine* for testing with the QuicTestServer.
+// Has the same properties as GetTestStreamEngine.  This function is
+// used when the stream_engine* needs to be shut down and restarted
+// between test cases (including between all of the bidirectional
+// stream test cases and all other tests for the engine; this is the
+// situation for Cronet).
+void StartTestStreamEngine(int port) {
+  CHECK(!g_cronet_engine);
+  quic_server_port = port;
+  g_cronet_engine = cronet::test::CreateTestEngine(port);
+}
+
+// Shuts a stream_engine* started with |StartTestStreamEngine| down.
+// See comment above.
+void ShutdownTestStreamEngine() {
+  CHECK(g_cronet_engine);
+  Cronet_Engine_Destroy(g_cronet_engine);
+  g_cronet_engine = nullptr;
+  quic_server_port = 0;
+}
+
+}  // namespace grpc_support

components/cronet/native/test/test_util.cc

@@ -53,6 +53,11 @@ Cronet_EnginePtr CreateTestEngine(int quic_server_port) {
   Cronet_QuicHint_Destroy(quic_hint);
   // Create Cronet Engine.
   Cronet_EnginePtr cronet_engine = Cronet_Engine_Create();
+  // Set Mock Cert Verifier.
+  auto cert_verifier = std::make_unique<net::MockCertVerifier>();
+  cert_verifier->set_default_result(net::OK);
+  Cronet_Engine_SetMockCertVerifierForTesting(cronet_engine,
+                                              cert_verifier.release());
   // Start Cronet Engine.
   Cronet_Engine_StartWithParams(cronet_engine, engine_params);
   Cronet_EngineParams_Destroy(engine_params);

components/grpc_support/BUILD.gn

+source_set("headers") {
+  public = [
+    "include/bidirectional_stream_c.h",
+  ]
+}
+
 source_set("grpc_support") {
   sources = [
     "bidirectional_stream.cc",
@@ -7,12 +13,14 @@ source_set("grpc_support") {
   ]
 
   deps = [
+    ":headers",
     "//base",
     "//net",
     "//url",
   ]
 }
 
+# Depends on ":grpc_support" implementation.
 source_set("bidirectional_stream_unittest") {
   testonly = true
   sources = [
@@ -30,3 +38,22 @@ source_set("bidirectional_stream_unittest") {
     "//components/grpc_support/test:get_stream_engine_header",
   ]
 }
+
+# Depends on ":headers" to avoid ":grpc_support" implementation.
+source_set("bidirectional_stream_test") {
+  testonly = true
+  sources = [
+    "bidirectional_stream_unittest.cc",
+  ]
+
+  deps = [
+    ":headers",
+    "//base",
+    "//net",
+    "//net:test_support",
+  ]
+
+  public_deps = [
+    "//components/grpc_support/test:get_stream_engine_header",
+  ]
+}

components/grpc_support/include/bidirectional_stream_c.h

@@ -6,7 +6,7 @@
 #define COMPONENTS_GRPC_SUPPORT_INCLUDE_BIDIRECTIONAL_STREAM_C_H_
 
 #if defined(WIN32)
-#define GRPC_SUPPORT_EXPORT
+#define GRPC_SUPPORT_EXPORT __declspec(dllexport)
 #else
 #define GRPC_SUPPORT_EXPORT __attribute__((visibility("default")))
 #endif
@@ -242,4 +242,4 @@ bool bidirectional_stream_is_done(bidirectional_stream* stream);
 }
 #endif
 
-#endif  // COMPONENTS_GRPC_SUPPORT_INCLUDE_BIDIRECTIONAL_STREAM_H_
+#endif  // COMPONENTS_GRPC_SUPPORT_INCLUDE_BIDIRECTIONAL_STREAM_C_H_