Merge remote-tracking branch 'upstream/master' into spiffe1

src/core/ext/filters/client_channel/client_channel.cc

@@ -3881,6 +3881,7 @@ void CallData::StartPickLocked(void* arg, grpc_error* error) {
         GRPC_ERROR_UNREF(result.error);
         GRPC_CLOSURE_SCHED(&calld->pick_closure_,
                            GRPC_ERROR_REF(disconnect_error));
+        if (calld->pick_queued_) calld->RemoveCallFromQueuedPicksLocked(elem);
         break;
       }
       // If wait_for_ready is false, then the error indicates the RPC

src/core/ext/transport/chttp2/server/chttp2_server.cc

@@ -266,7 +266,7 @@ static void tcp_server_shutdown_complete(void* arg, grpc_error* error) {
   // may do a synchronous unref.
   grpc_core::ExecCtx::Get()->Flush();
   if (destroy_done != nullptr) {
-    destroy_done->cb(destroy_done->cb_arg, GRPC_ERROR_REF(error));
+    GRPC_CLOSURE_SCHED(destroy_done, GRPC_ERROR_REF(error));
     grpc_core::ExecCtx::Get()->Flush();
   }
   grpc_channel_args_destroy(state->args);

src/core/lib/iomgr/tcp_posix.cc

@@ -1020,7 +1020,7 @@ static void tcp_handle_write(void* arg /* grpc_tcp */, grpc_error* error) {
   if (error != GRPC_ERROR_NONE) {
     cb = tcp->write_cb;
     tcp->write_cb = nullptr;
-    cb->cb(cb->cb_arg, error);
+    GRPC_CLOSURE_SCHED(cb, GRPC_ERROR_REF(error));
     TCP_UNREF(tcp, "write");
     return;
   }
@@ -1030,6 +1030,8 @@ static void tcp_handle_write(void* arg /* grpc_tcp */, grpc_error* error) {
       gpr_log(GPR_INFO, "write: delayed");
     }
     notify_on_write(tcp);
+    // tcp_flush does not populate error if it has returned false.
+    GPR_DEBUG_ASSERT(error == GRPC_ERROR_NONE);
   } else {
     cb = tcp->write_cb;
     tcp->write_cb = nullptr;
@@ -1037,6 +1039,7 @@ static void tcp_handle_write(void* arg /* grpc_tcp */, grpc_error* error) {
       const char* str = grpc_error_string(error);
       gpr_log(GPR_INFO, "write: %s", str);
     }
+    // No need to take a ref on error since tcp_flush provides a ref.
     GRPC_CLOSURE_SCHED(cb, error);
     TCP_UNREF(tcp, "write");
   }

src/core/lib/slice/slice_intern.cc

@@ -208,7 +208,11 @@ static InternedSliceRefcount* InternNewStringLocked(slice_shard* shard,
   InternedSliceRefcount* s =
       static_cast<InternedSliceRefcount*>(gpr_malloc(sizeof(*s) + len));
   new (s) grpc_core::InternedSliceRefcount(len, hash, shard->strs[shard_idx]);
-  memcpy(reinterpret_cast<char*>(s + 1), buffer, len);
+  // TODO(arjunroy): Investigate why hpack tried to intern the nullptr string.
+  // https://github.com/grpc/grpc/pull/20110#issuecomment-526729282
+  if (len > 0) {
+    memcpy(reinterpret_cast<char*>(s + 1), buffer, len);
+  }
   shard->strs[shard_idx] = s;
   shard->count++;
   if (shard->count > shard->capacity * 2) {

src/core/lib/transport/static_metadata.cc

@@ -1217,12 +1217,7 @@ static const uint16_t elem_keys[] = {
     9076,  9185,  9294, 9403,  9512,  9621, 6242,  9730, 9839, 9948, 10057,
     10166, 1189,  538,  10275, 10384, 212,  10493, 1195, 1196, 1197, 1198,
     1080,  10602, 1843, 11365, 0,     0,    0,     1734, 0,    1850, 0,
-    0,     0,     356,  1627,  0,     0,    0,     0,    0,    0,    0,
-    0,     0,     0,    0,     0,     0,    0,     0,    0,    0,    0,
-    0,     0,     0,    0,     0,     0,    0,     0,    0,    0,    0,
-    0,     0,     0,    0,     0,     0,    0,     0,    0,    0,    0,
-    0,     0,     0,    0,     0,     0,    0,     0,    0,    0,    0,
-    0,     0,     0,    0,     0,     0,    0,     0,    0};
+    0,     0,     356,  1627};
 static const uint8_t elem_idxs[] = {
     7,  8,   9,   10,  11, 12,  13, 76,  78,  71,  1,  2,  5,  6,  25, 3,
     4,  66,  65,  30,  83, 62,  63, 67,  61,  73,  57, 37, 14, 19, 21, 22,

src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc

@@ -251,7 +251,14 @@ static void on_handshaker_service_resp_recv(void* arg, grpc_error* error) {
     gpr_log(GPR_ERROR, "ALTS handshaker client is nullptr");
     return;
   }
-  alts_handshaker_client_handle_response(client, true);
+  bool success = true;
+  if (error != GRPC_ERROR_NONE) {
+    gpr_log(GPR_ERROR,
+            "ALTS handshaker on_handshaker_service_resp_recv error: %s",
+            grpc_error_string(error));
+    success = false;
+  }
+  alts_handshaker_client_handle_response(client, success);
 }
 
 /* gRPC provided callback used when dedicatd CQ and thread are used.

test/core/transport/BUILD

@@ -82,6 +82,20 @@ grpc_cc_test(
     ],
 )
 
+grpc_cc_test(
+    name = "static_metadata_test",
+    srcs = ["static_metadata_test.cc"],
+    external_deps = [
+        "gtest",
+    ],
+    language = "C++",
+    deps = [
+        "//:gpr",
+        "//:grpc",
+        "//test/core/util:grpc_test_util",
+    ],
+)
+
 grpc_cc_test(
     name = "status_conversion_test",
     srcs = ["status_conversion_test.cc"],

test/core/transport/static_metadata_test.cc

@@ -0,0 +1,51 @@
+/*
+ *
+ * Copyright 2019 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include <gtest/gtest.h>
+
+#include <grpc/grpc.h>
+
+#include "src/core/lib/transport/metadata.h"
+#include "src/core/lib/transport/static_metadata.h"
+#include "test/core/util/test_config.h"
+
+namespace grpc_core {
+namespace {
+
+TEST(StaticMetadataTest, ReadAllStaticElements) {
+  // This makes sure that all static elements are returned when
+  // grpc_mdelem_from_slices is called with key pairs pregenerated.
+  for (int i = 0; i < GRPC_STATIC_MDELEM_COUNT; i++) {
+    const grpc_mdelem mdelem = grpc_static_mdelem_manifested()[i];
+    const grpc_mdelem mdelem2 =
+        grpc_mdelem_from_slices(GRPC_MDKEY(mdelem), GRPC_MDVALUE(mdelem));
+    EXPECT_EQ(mdelem.payload, mdelem2.payload);
+  }
+}
+
+}  // namespace
+}  // namespace grpc_core
+
+int main(int argc, char** argv) {
+  grpc_init();
+  grpc::testing::TestEnvironment env(argc, argv);
+  ::testing::InitGoogleTest(&argc, argv);
+  int retval = RUN_ALL_TESTS();
+  grpc_shutdown();
+  return retval;
+}

test/cpp/README-iOS.md

@@ -0,0 +1,52 @@
+## C++ tests on iOS
+
+[GTMGoogleTestRunner](https://github.com/google/google-toolbox-for-mac/blob/master/UnitTesting/GTMGoogleTestRunner.mm) is used to convert googletest cases to XCTest that can be run on iOS. GTMGoogleTestRunner doesn't execute the `main` function, so we can't have any test logic in `main`.
+However, it's ok to call `::testing::InitGoogleTest` in `main`, as `GTMGoogleTestRunner` [calls InitGoogleTest](https://github.com/google/google-toolbox-for-mac/blob/master/UnitTesting/GTMGoogleTestRunner.mm#L151).
+`grpc::testing::TestEnvironment` can also be called from `main`, as it does some test initialization (install crash handler, seed RNG) that's not strictly required to run testcases on iOS.
+
+
+## Porting exising C++ tests to run on iOS
+
+Please follow these guidelines when porting tests to run on iOS:
+
+- Tests need to use the googletest framework
+- Any setup/teardown code in `main` needs to be moved to `SetUpTestCase`/`TearDownTestCase`, and `TEST` needs to be changed to `TEST_F`.
+- [Death tests](https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#death-tests) are not supported on iOS, so use the `*_IF_SUPPORTED()` macros to ensure that your code compiles on iOS.
+
+For example, the following test
+```c++
+TEST(MyTest, TestOne) {
+  ASSERT_DEATH(ThisShouldDie(), "");
+}
+
+int main(int argc, char** argv) {
+  grpc::testing::TestEnvironment env(argc, argv);
+  ::testing::InitGoogleTest(&argc, argv);
+  grpc_init();
+  return RUN_ALL_TESTS();
+  grpc_shutdown();
+}
+```
+
+should be changed to
+```c++
+class MyTest : public ::testing::Test {
+ protected:
+  static void SetUpTestCase() { grpc_init(); }
+  static void TearDownTestCase() { grpc_shutdown(); }
+};
+
+TEST_F(MyTest, TestOne) {
+  ASSERT_DEATH_IF_SUPPORTED(ThisShouldDie(), "");
+}
+
+int main(int argc, char** argv) {
+  grpc::testing::TestEnvironment env(argc, argv);
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
+```
+
+## Limitations
+
+Due to a [limitation](https://github.com/google/google-toolbox-for-mac/blob/master/UnitTesting/GTMGoogleTestRunner.mm#L48-L56) in GTMGoogleTestRunner, `SetUpTestCase`/`TeardownTestCase` will be called before/after *every* individual test case, similar to `SetUp`/`TearDown`.

test/cpp/microbenchmarks/BUILD

@@ -27,14 +27,6 @@ grpc_cc_test(
     deps = ["//test/core/util:grpc_test_util"],
 )
 
-grpc_cc_binary(
-    name = "bm_chttp2_transport",
-    testonly = 1,
-    srcs = ["bm_chttp2_transport.cc"],
-    tags = ["no_windows"],
-    deps = [":helpers"],
-)
-
 grpc_cc_library(
     name = "helpers",
     testonly = 1,

tools/codegen/core/gen_static_metadata.py

@@ -627,7 +627,6 @@ def perfect_hash(keys, name):
         return x + p.r[y]
 
     return {
-        'PHASHRANGE': p.t - 1 + max(p.r),
         'PHASHNKEYS': len(p.slots),
         'pyfunc': f,
         'code': """
@@ -659,7 +658,7 @@ elem_keys = [
 elem_hash = perfect_hash(elem_keys, 'elems')
 print >> C, elem_hash['code']
 
-keys = [0] * int(elem_hash['PHASHRANGE'])
+keys = [0] * int(elem_hash['PHASHNKEYS'])
 idxs = [255] * int(elem_hash['PHASHNKEYS'])
 for i, k in enumerate(elem_keys):
     h = elem_hash['pyfunc'](k)