[remoting][gRPC] Set a default deadline

This CL sets a default 30s timeout for gRPC requests if the caller
didn't set it.

Bug: 956424
Change-Id: I27a90a84cfc81d6763d24722d40b3cb06b931f65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1590549
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#655509}

remoting/base/grpc_support/BUILD.gn

@@ -18,6 +18,8 @@ static_library("grpc_support") {
     "grpc_channel.cc",
     "grpc_channel.h",
     "grpc_executor.h",
+    "grpc_util.cc",
+    "grpc_util.h",
     "scoped_grpc_server_stream.cc",
     "scoped_grpc_server_stream.h",
     "using_grpc_channel_shared_ptr.inc",

remoting/base/grpc_support/grpc_async_executor.cc

@@ -12,13 +12,18 @@
 #include "base/no_destructor.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/threading/thread.h"
+#include "base/time/time.h"
 #include "remoting/base/grpc_support/grpc_async_request.h"
+#include "remoting/base/grpc_support/grpc_util.h"
 #include "third_party/grpc/src/include/grpcpp/completion_queue.h"
 
 namespace remoting {
 
 namespace {
 
+constexpr base::TimeDelta kDefaultRequestTimeout =
+    base::TimeDelta::FromSeconds(30);
+
 using DequeueCallback = base::OnceCallback<void(bool operation_succeeded)>;
 
 struct DispatchTask {
@@ -100,6 +105,10 @@ void GrpcAsyncExecutor::ExecuteRpc(std::unique_ptr<GrpcAsyncRequest> request) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto* unowned_request = request.get();
   DCHECK(FindRequest(unowned_request) == pending_requests_.end());
+  if (GetDeadline(*request->context()).is_max()) {
+    VLOG(1) << "Deadline is not set. Using the default request timeout.";
+    SetDeadline(request->context(), base::Time::Now() + kDefaultRequestTimeout);
+  }
   auto task = std::make_unique<DispatchTask>();
   task->caller_sequence_task_runner = base::SequencedTaskRunnerHandle::Get();
   task->callback =

remoting/base/grpc_support/grpc_async_executor_unittest.cc

@@ -20,6 +20,7 @@
 #include "remoting/base/grpc_support/grpc_async_unary_request.h"
 #include "remoting/base/grpc_support/grpc_support_test_services.grpc.pb.h"
 #include "remoting/base/grpc_support/grpc_test_util.h"
+#include "remoting/base/grpc_support/grpc_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/grpc/src/include/grpcpp/grpcpp.h"
@@ -544,4 +545,41 @@ TEST_F(GrpcAsyncExecutorTest, StreamWithTwoExecutors_VerifyNoInterference) {
   run_loop.Run();
 }
 
+TEST_F(GrpcAsyncExecutorTest, ExecuteWithoutDeadline_DefaultDeadlineSet) {
+  EchoRequest request;
+  auto grpc_request = CreateGrpcAsyncUnaryRequest(
+      base::BindOnce(&GrpcAsyncExecutorTestService::Stub::AsyncEcho,
+                     base::Unretained(stub_.get())),
+      std::make_unique<grpc::ClientContext>(), request,
+      base::BindOnce(
+          [](const grpc::Status&, const EchoResponse&) { NOTREACHED(); }));
+  auto* context = grpc_request->context();
+  ASSERT_TRUE(GetDeadline(*context).is_max());
+  base::Time min_deadline = base::Time::Now();
+  base::Time max_deadline = min_deadline + base::TimeDelta::FromHours(1);
+  executor_->ExecuteRpc(std::move(grpc_request));
+  base::Time deadline = GetDeadline(*context);
+  ASSERT_LT(min_deadline, deadline);
+  ASSERT_GT(max_deadline, deadline);
+}
+
+TEST_F(GrpcAsyncExecutorTest, ExecuteWithDeadline_DeadlineNotChanged) {
+  constexpr base::TimeDelta kDeadlineEpsilon = base::TimeDelta::FromSeconds(1);
+  EchoRequest request;
+  auto context = std::make_unique<grpc::ClientContext>();
+  base::Time deadline = base::Time::Now() + base::TimeDelta::FromSeconds(10);
+  SetDeadline(context.get(), deadline);
+  auto grpc_request = CreateGrpcAsyncUnaryRequest(
+      base::BindOnce(&GrpcAsyncExecutorTestService::Stub::AsyncEcho,
+                     base::Unretained(stub_.get())),
+      std::move(context), request,
+      base::BindOnce(
+          [](const grpc::Status&, const EchoResponse&) { NOTREACHED(); }));
+  auto* unowned_context = grpc_request->context();
+  executor_->ExecuteRpc(std::move(grpc_request));
+  base::Time new_deadline = GetDeadline(*unowned_context);
+  ASSERT_LT(deadline - kDeadlineEpsilon, new_deadline);
+  ASSERT_GT(deadline + kDeadlineEpsilon, new_deadline);
+}
+
 }  // namespace remoting

remoting/base/grpc_support/grpc_util.cc

@@ -0,0 +1,27 @@
+// Copyright 2019 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 "remoting/base/grpc_support/grpc_util.h"
+
+#include <chrono>
+
+#include "third_party/grpc/src/include/grpcpp/client_context.h"
+
+namespace remoting {
+
+void SetDeadline(grpc::ClientContext* context, base::Time deadline) {
+  context->set_deadline(
+      std::chrono::system_clock::from_time_t(deadline.ToTimeT()));
+}
+
+base::Time GetDeadline(const grpc::ClientContext& context) {
+  auto deadline_tp = context.deadline();
+  if (deadline_tp == std::chrono::system_clock::time_point::max()) {
+    return base::Time::Max();
+  }
+  return base::Time::FromTimeT(
+      std::chrono::system_clock::to_time_t(deadline_tp));
+}
+
+}  // namespace remoting

remoting/base/grpc_support/grpc_util.h

@@ -0,0 +1,25 @@
+// Copyright 2019 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.
+
+#ifndef REMOTING_BASE_GRPC_SUPPORT_GRPC_UTIL_H_
+#define REMOTING_BASE_GRPC_SUPPORT_GRPC_UTIL_H_
+
+#include "base/time/time.h"
+
+namespace grpc {
+class ClientContext;
+}  // namespace grpc
+
+namespace remoting {
+
+// Sets the deadline on |context|.
+void SetDeadline(grpc::ClientContext* context, base::Time deadline);
+
+// Gets the deadline in base::Time. Returns base::Time::Max if the deadline is
+// not set.
+base::Time GetDeadline(const grpc::ClientContext& context);
+
+}  // namespace remoting
+
+#endif  // REMOTING_BASE_GRPC_SUPPORT_GRPC_UTIL_H_