Merge github.com:grpc/grpc into write_completion

12 files changed, 267 insertions, 42 deletions

diff --git a/doc/PROTOCOL-HTTP2.md b/doc/PROTOCOL-HTTP2.md
index 68af1f2ca1..29d3cc2e76 100644
--- a/doc/PROTOCOL-HTTP2.md
+++ b/doc/PROTOCOL-HTTP2.md
@@ -24,7 +24,8 @@ Request-Headers are delivered as HTTP2 headers in HEADERS + CONTINUATION frames.
 * **Call-Definition** → Method Scheme Path TE [Authority] [Timeout] Content-Type [Message-Type] [Message-Encoding] [Message-Accept-Encoding] [User-Agent]
 * **Method** →  ":method POST"
 * **Scheme** → ":scheme "  ("http" / "https")
-* **Path** → ":path"  {_path identifying method within exposed API_}
+* **Path** → ":path" "/" Service-Name "/" {_method name_}
+* **Service-Name** → {_IDL-specific service name_}
 * **Authority** → ":authority" {_virtual host name of authority_}
 * **TE** → "te" "trailers"  # Used to detect incompatible proxies
 * **Timeout** → "grpc-timeout" TimeoutValue TimeoutUnit
@@ -51,6 +52,13 @@ Request-Headers are delivered as HTTP2 headers in HEADERS + CONTINUATION frames.
 
 HTTP2 requires that reserved headers, ones starting with ":" appear before all other headers. Additionally implementations should send **Timeout** immediately after the reserved headers and they should send the **Call-Definition** headers before sending **Custom-Metadata**.
 
+Some gRPC implementations may allow the **Path** format shown above
+to be overridden, but this functionality is strongly discouraged.
+gRPC does not go out of its way to break users that are using this kind
+of override, but we do not actively support it, and some functionality
+(e.g., service config support) will not work when the path is not of
+the form shown above.
+
 If **Timeout** is omitted a server should assume an infinite timeout. Client implementations are free to send a default minimum timeout based on their deployment requirements.
 
 **Custom-Metadata** is an arbitrary set of key-value pairs defined by the application layer. Header names starting with "grpc-" but not listed here are reserved for future GRPC use and should not be used by applications as **Custom-Metadata**.
@@ -238,10 +246,10 @@ If a detectable connection failure occurs on the client all calls will be closed
 
 ### Appendix A - GRPC for Protobuf
 
-The service interfaces declared by protobuf are easily mapped onto GRPC by code generation extensions to protoc. The following defines the mapping to be used
-
+The service interfaces declared by protobuf are easily mapped onto GRPC by
+code generation extensions to protoc. The following defines the mapping
+to be used.
 
-* **Path** → / Service-Name / {_method name_}
 * **Service-Name** → ?( {_proto package name_} "." ) {_service name_}
 * **Message-Type** → {_fully qualified proto message name_}
 * **Content-Type** → "application/grpc+proto"
diff --git a/doc/PROTOCOL-WEB.md b/doc/PROTOCOL-WEB.md
index 6bb280894a..226871d7ae 100644
--- a/doc/PROTOCOL-WEB.md
+++ b/doc/PROTOCOL-WEB.md
@@ -1,16 +1,16 @@
-# Overview
+# gRPC Web
 
 gRPC-Web provides a JS client library that supports the same API
 as gRPC-Node to access a gRPC service. Due to browser limitation,
 the Web client library implements a different protocol than the
-[native gRPC protocol](http://www.grpc.io/docs/guides/wire.html).
+[native gRPC protocol](https://grpc.io/docs/guides/wire.html).
 This protocol is designed to make it easy for a proxy to translate
 between the protocols as this is the most likely deployment model.
 
 This document lists the differences between the two protocols.
 To help tracking future revisions, this document describes a delta
 with the protocol details specified in the
-[native gRPC protocol](http://www.grpc.io/docs/guides/wire.html).
+[native gRPC protocol](https://grpc.io/docs/guides/wire.html).
 
 # Design goals
 
@@ -31,12 +31,14 @@ web-specific features such as CORS, XSRF
 * become optional (in 1-2 years) when browsers are able to speak the native
 gRPC protocol via the new [whatwg fetch/streams API](https://github.com/whatwg/fetch)
 
-# Protocol differences vs [gRPC over HTTP2](http://www.grpc.io/docs/guides/wire.html)
+# Protocol differences vs [gRPC over HTTP2](https://grpc.io/docs/guides/wire.html)
 
 Content-Type
 
 1. application/grpc-web
   * e.g. application/grpc-web+[proto, json, thrift]
+  * the sender should always specify the message format, e.g. +proto, +json
+  * the receiver should assume the default is "+proto" when the message format is missing in Content-Type (as "application/grpc-web")
 2. application/grpc-web-text
   * text-encoded streams of “application/grpc-web”
   * e.g. application/grpc-web-text+[proto, thrift]
@@ -51,14 +53,14 @@ HTTP wire protocols
 
 ---
 
-HTTP/2 related behavior (specified in [gRPC over HTTP2](http://www.grpc.io/docs/guides/wire.html))
+HTTP/2 related behavior (specified in [gRPC over HTTP2](https://grpc.io/docs/guides/wire.html))
 
 1. stream-id is not supported or used
 2. go-away is not supported or used
 
 ---
 
-Message framing (vs. [http2-transport-mapping](http://www.grpc.io/docs/guides/wire.html#http2-transport-mapping))
+Message framing (vs. [http2-transport-mapping](https://grpc.io/docs/guides/wire.html#http2-transport-mapping))
 
 1. Response status encoded as part of the response body
   * Key-value pairs encoded as a HTTP/1 headers block (without the terminating newline), per https://tools.ietf.org/html/rfc7230#section-3.2
@@ -84,7 +86,7 @@ in the body.
 User Agent
 
 * Do NOT use User-Agent header (which is to be set by browsers, by default)
-* Use X-User-Agent: grpc-web-javascript/0.1 (follow the same format as specified in [gRPC over HTTP2](http://www.grpc.io/docs/guides/wire.html))
+* Use X-User-Agent: grpc-web-javascript/0.1 (follow the same format as specified in [gRPC over HTTP2](https://grpc.io/docs/guides/wire.html))
 
 ---
 
@@ -101,10 +103,6 @@ to security policies with XHR
   * While the server runtime will always base64-encode and flush gRPC messages
   atomically the client library should not assume base64 padding always
   happens at the boundary of message frames. That is, the implementation may send base64-encoded "chunks" with potential padding whenever the runtime needs to flush a byte buffer.
-3. For binary trailers, when the content-type is set to
-application/grpc-web-text, the extra base64 encoding specified
-in [gRPC over HTTP2](http://www.grpc.io/docs/guides/wire.html)
-for binary custom metadata is skipped.
 
 # Other features
 
diff --git a/doc/compression.md b/doc/compression.md
index de245d90fe..ee22bc3f12 100644
--- a/doc/compression.md
+++ b/doc/compression.md
@@ -1,4 +1,4 @@
-## **gRPC Compression**
+## gRPC Compression
 
 The keywords "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD",
 "SHOULD NOT", "RECOMMENDED",  "MAY", and "OPTIONAL" in this document are to be
@@ -112,7 +112,7 @@ unsupported condition as well as the supported ones. The returned
 1. An ill-constructed message with its [Compressed-Flag
 bit](PROTOCOL-HTTP2.md#compressed-flag)
 set but lacking a
-"[grpc-encoding](PROTOCOL-HTTP2.md#message-encoding)"
+[grpc-encoding](PROTOCOL-HTTP2.md#message-encoding)
 entry different from _identity_ in its metadata MUST fail with `INTERNAL`
 status, its associated description indicating the invalid Compressed-Flag
 condition.
diff --git a/doc/core/grpc-error.md b/doc/core/grpc-error.md
index c05d1dd909..49a95b353c 100644
--- a/doc/core/grpc-error.md
+++ b/doc/core/grpc-error.md
@@ -83,12 +83,12 @@ c->cb(exec_ctx, c->cb_arg, err);
 The caller is still responsible for unref-ing the error.
 
 However, the above line is currently being phased out! It is safer to invoke
-callbacks with `grpc_closure_run` and `grpc_closure_sched`. These functions are
+callbacks with `GRPC_CLOSURE_RUN` and `GRPC_CLOSURE_SCHED`. These functions are
 not callbacks, so they will take ownership of the error passed to them.
 
 ```C
 grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Some error occured");
-grpc_closure_run(exec_ctx, cb, error);
+GRPC_CLOSURE_RUN(exec_ctx, cb, error);
 // current function no longer has ownership of the error
 ```
 
@@ -97,7 +97,7 @@ you must explicitly take a reference.
 
 ```C
 grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Some error occured");
-grpc_closure_run(exec_ctx, cb, GRPC_ERROR_REF(error));
+GRPC_CLOSURE_RUN(exec_ctx, cb, GRPC_ERROR_REF(error));
 // do some other things with the error
 GRPC_ERROR_UNREF(error);
 ```
diff --git a/doc/cpp-style-guide.md b/doc/cpp-style-guide.md
index a1f91353fe..8211703d02 100644
--- a/doc/cpp-style-guide.md
+++ b/doc/cpp-style-guide.md
@@ -5,5 +5,4 @@ The majority of gRPC's C++ requirements are drawn from the [Google C++ style
 guide] (https://google.github.io/styleguide/cppguide.html). Additionally,
 as in C, layout rules are defined by clang-format, and all code
 should be passed through clang-format. A (docker-based) script to do
-so is included in [tools/distrib/clang\_format\_code.sh]
-(../tools/distrib/clang_format_code.sh).
+so is included in [tools/distrib/clang_format_code.sh](../tools/distrib/clang_format_code.sh).
diff --git a/doc/environment_variables.md b/doc/environment_variables.md
index 47efb3a1d8..339b705252 100644
--- a/doc/environment_variables.md
+++ b/doc/environment_variables.md
@@ -4,6 +4,10 @@ gRPC environment variables
 gRPC C core based implementations (those contained in this repository) expose
 some configuration as environment variables that can be set.
 
+* http_proxy
+  The URI of the proxy to use for HTTP CONNECT support.  Does not currently
+  support username or password information in the URI.
+
 * GRPC_ABORT_ON_LEAKS
   A debugging aid to cause a call to abort() when gRPC objects are leaked past
   grpc_shutdown(). Set to 1 to cause the abort, if unset or 0 it does not
@@ -38,6 +42,8 @@ some configuration as environment variables that can be set.
   - bdp_estimator - traces behavior of bdp estimation logic
   - call_error - traces the possible errors contributing to final call status
   - channel - traces operations on the C core channel stack
+  - client_channel - traces client channel activity, including resolver
+    and load balancing policy interaction
   - combiner - traces combiner lock state
   - compression - traces compression operations
   - connectivity_state - traces connectivity state changes to channels
@@ -47,8 +53,6 @@ some configuration as environment variables that can be set.
   - flowctl - traces http2 flow control
   - op_failure - traces error information when failure is pushed onto a
     completion queue
-  - pending_tags - [debug builds only] traces still-in-progress tags on
-    completion queues
   - round_robin - traces the round_robin load balancing policy
   - glb - traces the grpclb load balancer
   - queue_pluck
@@ -59,6 +63,23 @@ some configuration as environment variables that can be set.
   - transport_security - traces metadata about secure channel establishment
   - tcp - traces bytes in and out of a channel
 
+  The following tracers will only run in binaries built in DEBUG mode. This is
+  accomplished by invoking `CONFIG=dbg make <target>`
+  - metadata - tracks creation and mutation of metadata
+  - closure - tracks closure creation, scheduling, and completion
+  - pending_tags - traces still-in-progress tags on completion queues
+  - polling - traces the selected polling engine
+  - queue_refcount
+  - error_refcount
+  - stream_refcount
+  - workqueue_refcount
+  - fd_refcount
+  - auth_context_refcount
+  - security_connector_refcount
+  - resolver_refcount
+  - lb_policy_refcount
+  - chttp2_refcount
+
   'all' can additionally be used to turn all traces on.
   Individual traces can be disabled by prefixing them with '-'.
 
diff --git a/doc/fail_fast.md b/doc/fail_fast.md
index 2dd5561fc6..ff3d235397 100644
--- a/doc/fail_fast.md
+++ b/doc/fail_fast.md
@@ -1 +1 @@
-Moved to wait-for-ready.md
+Moved to [wait-for-ready.md](wait-for-ready.md)
diff --git a/doc/interop-test-descriptions.md b/doc/interop-test-descriptions.md
index b040621f88..0ee2cae2bd 100644
--- a/doc/interop-test-descriptions.md
+++ b/doc/interop-test-descriptions.md
@@ -183,7 +183,8 @@ the `response_compressed` boolean.
 
 Whether compression was actually performed is determined by the compression bit
 in the response's message flags. *Note that some languages may not have access
-to the message flags*.
+to the message flags, in which case the client will be unable to verify that
+the `response_compressed` boolean is obeyed by the server*.
 
 
 Server features:
@@ -218,10 +219,10 @@ Procedure:
     ```
     Client asserts:
     * call was successful
-    * when `response_compressed` is true, the response MUST have the
-      compressed message flag set.
-    * when `response_compressed` is false, the response MUST NOT have
-      the compressed message flag set.
+    * if supported by the implementation, when `response_compressed` is true,
+      the response MUST have the compressed message flag set.
+    * if supported by the implementation, when `response_compressed` is false,
+      the response MUST NOT have the compressed message flag set.
     * response payload body is 314159 bytes in size in both cases.
     * clients are free to assert that the response payload body contents are
       zero and comparing the entire response message against a golden response
@@ -304,8 +305,8 @@ Procedure:
       }
     }
     ```
-    If the call fails with `INVALID_ARGUMENT`, the test fails. Otherwise, we
-    continue.
+    If the call does not fail with `INVALID_ARGUMENT`, the test fails.
+    Otherwise, we continue.
 
  1. Client calls `StreamingInputCall` again, sending the *compressed* message
 
@@ -377,7 +378,13 @@ Client asserts:
 ### server_compressed_streaming
 
 This test verifies that the server can compress streaming messages and disable
-compression on individual messages.
+compression on individual messages, expecting the server's response to be
+compressed or not according to the `response_compressed` boolean.
+
+Whether compression was actually performed is determined by the compression bit
+in the response's message flags. *Note that some languages may not have access
+to the message flags, in which case the client will be unable to verify that the
+`response_compressed` boolean is obeyed by the server*.
 
 Server features:
 * [StreamingOutputCall][]
@@ -407,15 +414,14 @@ Procedure:
     Client asserts:
     * call was successful
     * exactly two responses
-    * when `response_compressed` is false, the response's messages MUST
-      NOT have the compressed message flag set.
-    * when `response_compressed` is true, the response's messages MUST
-      have the compressed message flag set.
+    * if supported by the implementation, when `response_compressed` is false,
+      the response's messages MUST NOT have the compressed message flag set.
+    * if supported by the implementation, when `response_compressed` is true,
+      the response's messages MUST have the compressed message flag set.
     * response payload bodies are sized (in order): 31415, 92653
     * clients are free to assert that the response payload body contents are
       zero and comparing the entire response messages against golden responses
 
-
 ### ping_pong
 
 This test verifies that full duplex bidi is supported.
@@ -1095,4 +1101,3 @@ Discussion:
 Ideally, this would be communicated via metadata and not in the
 request/response, but we want to use this test in code paths that don't yet
 fully communicate metadata.
-
diff --git a/doc/service_config.md b/doc/service_config.md
index 8039fcad09..99d985f3bf 100644
--- a/doc/service_config.md
+++ b/doc/service_config.md
@@ -131,8 +131,8 @@ functionality is introduced.
 
 # Architecture
 
-A service config is associated with a server name.  The [name
-resolver](naming.md) plugin, when asked to resolve a particular server
+A service config is associated with a server name.  The [name resolver](naming.md)
+plugin, when asked to resolve a particular server
 name, will return both the resolved addresses and the service config.
 
 TODO(roth): Design how the service config will be encoded in DNS.
diff --git a/doc/stress_test_framework.md b/doc/stress_test_framework.md
index 18f545e090..2212d9842c 100644
--- a/doc/stress_test_framework.md
+++ b/doc/stress_test_framework.md
@@ -2,7 +2,7 @@
 
 (Sree Kuchibhotla - sreek@)
 
-> Status: This is implemented. More details at [README.md](https://github.com/grpc/grpc/blob/master/tools/run_tests/stress_test/README.md)
+Status: This is implemented. More details at [README.md](https://github.com/grpc/grpc/blob/master/tools/run_tests/stress_test/README.md)
 
 
 **I. GOALS**
diff --git a/doc/unit_testing.md b/doc/unit_testing.md
new file mode 100644
index 0000000000..0aa9be9b9d
--- /dev/null
+++ b/doc/unit_testing.md
@@ -0,0 +1,175 @@
+# How to write unit tests for gRPC C client.
+
+tl;dr: [Example code](https://github.com/grpc/grpc/blob/master/test/cpp/end2end/mock_test.cc).
+
+To unit-test client-side logic via the synchronous API, gRPC provides a mocked Stub based on googletest(googlemock) that can be programmed upon and easily incorporated in the test code.
+
+For instance, consider an EchoService like this:
+
+
+```proto
+service EchoTestService {
+        rpc Echo(EchoRequest) returns (EchoResponse);
+        rpc BidiStream(stream EchoRequest) returns (stream EchoResponse);
+}
+```
+
+The code generated would look something like this:
+
+```c
+class EchoTestService final {
+  public:
+  class StubInterface {
+    virtual ::grpc::Status Echo(::grpc::ClientContext* context, const ::grpc::testing::EchoRequest& request, ::grpc::testing::EchoResponse* response) = 0;
+  …
+    std::unique_ptr< ::grpc::ClientReaderWriterInterface< ::grpc::testing::EchoRequest, ::grpc::testing::EchoResponse>> BidiStream(::grpc::ClientContext* context) {
+      return std::unique_ptr< ::grpc::ClientReaderWriterInterface< ::grpc::testing::EchoRequest, ::grpc::testing::EchoResponse>>(BidiStreamRaw(context));
+    }
+  …
+    private:
+    virtual ::grpc::ClientReaderWriterInterface< ::grpc::testing::EchoRequest, ::grpc::testing::EchoResponse>* BidiStreamRaw(::grpc::ClientContext* context) = 0;
+  …
+  } // End StubInterface
+…
+} // End EchoTestService
+```
+
+
+If we mock the StubInterface and set expectations on the pure-virtual methods we can test client-side logic without having to make any rpcs.
+
+A mock for this StubInterface will look like this:
+
+
+```c
+class MockEchoTestServiceStub : public EchoTestService::StubInterface {
+ public:
+  MOCK_METHOD3(Echo, ::grpc::Status(::grpc::ClientContext* context, const ::grpc::testing::EchoRequest& request, ::grpc::testing::EchoResponse* response));
+  MOCK_METHOD1(BidiStreamRaw, ::grpc::ClientReaderWriterInterface< ::grpc::testing::EchoRequest, ::grpc::testing::EchoResponse>*(::grpc::ClientContext* context));
+};
+```
+
+
+**Generating mock code:**
+
+Such a mock can be auto-generated by:
+
+
+
+1.  Setting flag(generate_mock_code=true) on grpc plugin for protoc, or
+1.  Setting an attribute(generate_mock) in your bazel rule.
+
+Protoc plugin flag:
+
+```sh
+protoc -I . --grpc_out=generate_mock_code=true:. --plugin=protoc-gen-grpc=`which grpc_cpp_plugin` echo.proto
+```
+
+Bazel rule:
+
+```py
+grpc_proto_library(
+  name = "echo_proto",
+  srcs = ["echo.proto"],
+  generate_mock = True, 
+)
+```
+
+
+By adding such a flag now a header file `echo_mock.grpc.pb.h` containing the mocked stub will also be generated. 
+
+This header file can then be included in test files along with a gmock dependency.
+
+**Writing tests with mocked Stub.**
+
+Consider the following client a user might have:
+
+```c
+class FakeClient {
+ public:
+  explicit FakeClient(EchoTestService::StubInterface* stub) : stub_(stub) {}
+
+  void DoEcho() {
+    ClientContext context;
+    EchoRequest request;
+    EchoResponse response;
+    request.set_message("hello world");
+    Status s = stub_->Echo(&context, request, &response);
+    EXPECT_EQ(request.message(), response.message());
+    EXPECT_TRUE(s.ok());
+  }
+
+  void DoBidiStream() {
+    EchoRequest request;
+    EchoResponse response;
+    ClientContext context;
+    grpc::string msg("hello");
+
+    std::unique_ptr<ClientReaderWriterInterface<EchoRequest, EchoResponse>>
+        stream = stub_->BidiStream(&context);
+
+    request.set_message(msg  "0");
+    EXPECT_TRUE(stream->Write(request));
+    EXPECT_TRUE(stream->Read(&response));
+    EXPECT_EQ(response.message(), request.message());
+
+    request.set_message(msg  "1");
+    EXPECT_TRUE(stream->Write(request));
+    EXPECT_TRUE(stream->Read(&response));
+    EXPECT_EQ(response.message(), request.message());
+
+    request.set_message(msg  "2");
+    EXPECT_TRUE(stream->Write(request));
+    EXPECT_TRUE(stream->Read(&response));
+    EXPECT_EQ(response.message(), request.message());
+
+    stream->WritesDone();
+    EXPECT_FALSE(stream->Read(&response));
+
+    Status s = stream->Finish();
+    EXPECT_TRUE(s.ok());
+  }
+
+  void ResetStub(EchoTestService::StubInterface* stub) { stub_ = stub; }
+
+ private:
+  EchoTestService::StubInterface* stub_;
+};
+```
+
+A test could initialize this FakeClient with a mocked stub having set expectations on it:
+
+Unary RPC:
+
+```c
+MockEchoTestServiceStub stub;
+EchoResponse resp;
+resp.set_message("hello world");
+Expect_CALL(stub, Echo(_,_,_)).Times(Atleast(1)).WillOnce(DoAll(SetArgPointee<2>(resp), Return(Status::OK)));
+FakeClient client(stub);
+client.DoEcho();
+```
+
+Streaming RPC:
+
+```c
+ACTION_P(copy, msg) {
+  arg0->set_message(msg->message());
+}
+
+
+auto rw = new MockClientReaderWriter<EchoRequest, EchoResponse>();
+EchoRequest msg;
+EXPECT_CALL(*rw, Write(_, _)).Times(3).WillRepeatedly(DoAll(SaveArg<0>(&msg), Return(true)));
+EXPECT_CALL(*rw, Read(_)).
+      WillOnce(DoAll(WithArg<0>(copy(&msg)), Return(true))).
+      WillOnce(DoAll(WithArg<0>(copy(&msg)), Return(true))).
+      WillOnce(DoAll(WithArg<0>(copy(&msg)), Return(true))).
+      WillOnce(Return(false));
+
+MockEchoTestServiceStub  stub;
+EXPECT_CALL(stub, BidiStreamRaw(_)).Times(AtLeast(1)).WillOnce(Return(rw));
+
+FakeClient client(stub);
+client.DoBidiStream();
+```
+
diff --git a/doc/workarounds.md b/doc/workarounds.md
new file mode 100644
index 0000000000..bc511860f8
--- /dev/null
+++ b/doc/workarounds.md
@@ -0,0 +1,19 @@
+# gRPC Server Backward Compatibility Issues and Workarounds Manageent
+
+## Introduction
+This document lists the workarounds implemented on gRPC servers for record and reference when users need to enable a certain workaround.
+
+## Workaround List
+
+### Cronet Compression
+
+**Workaround ID:** WORKAROUND\_ID\_CRONET\_COMPRESSION
+
+**Date added:** May 06, 2017
+
+**Status:** Implemented in C core and C++
+
+**Issue:** Before version v1.3.0-dev, gRPC iOS client's Cronet transport did not implement compression. However the clients still claim to support compression. As a result, a client fails to parse received message when the message is compressed.
+The problem above was resolved in gRPC v1.3.0-dev. For backward compatibility, a server must forcingly disable compression for gRPC clients of version lower than or equal to v1.3.0-dev.
+
+**Workaround Description:** Implemented as a server channel filter in C core.  The filter identifies the version of peer client with incoming `user-agent` header of each call. If the client's gRPC version is lower that or equal to v1.3.x, a flag GRPC_WRITE_NO_COMPRESS is marked for all send_message ops which prevents compression of the messages to be sent out.