Don't access |response| for failed Heartbeat requests
Bug: 376065938, 376071938
Change-Id: Idbf7ca6eba236a7148e75dd35f4254bceb66638c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5972854
Reviewed-by: Gary Kacmarcik <garykac@chromium.org>
Commit-Queue: Gary Kacmarcik <garykac@chromium.org>
Auto-Submit: Joe Downing <joedow@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1374787}
diff --git a/remoting/host/cloud_heartbeat_service_client.cc b/remoting/host/cloud_heartbeat_service_client.cc
index 5f39f98..bf0740c3c 100644
--- a/remoting/host/cloud_heartbeat_service_client.cc
+++ b/remoting/host/cloud_heartbeat_service_client.cc
@@ -127,6 +127,11 @@
const ProtobufHttpStatus& status) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (!status.ok()) {
+ OnError(std::move(callback), status);
+ return;
+ }
+
// Cloud hosts always require session authorization and do not support
// changing the email address of the primary user. This service client always
// uses 'lite' heartbeats.
diff --git a/remoting/host/corp_heartbeat_service_client.cc b/remoting/host/corp_heartbeat_service_client.cc
index 3edbab2..8dd9481 100644
--- a/remoting/host/corp_heartbeat_service_client.cc
+++ b/remoting/host/corp_heartbeat_service_client.cc
@@ -109,6 +109,11 @@
const ProtobufHttpStatus& status) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (!status.ok()) {
+ OnError(std::move(callback), status);
+ return;
+ }
+
// TODO: joedow - Return wait interval from the service and pass it through.
// TODO: joedow - Return primary email from the service and pass it through.
std::move(callback).Run(status, /*wait_interval=*/std::nullopt,
diff --git a/remoting/host/heartbeat_service_client.cc b/remoting/host/heartbeat_service_client.cc
index eb084c61..bf84956 100644
--- a/remoting/host/heartbeat_service_client.cc
+++ b/remoting/host/heartbeat_service_client.cc
@@ -4,9 +4,23 @@
#include "remoting/host/heartbeat_service_client.h"
+#include <optional>
+
+#include "base/functional/callback.h"
+#include "remoting/base/protobuf_http_status.h"
+
namespace remoting {
HeartbeatServiceClient::HeartbeatServiceClient() = default;
HeartbeatServiceClient::~HeartbeatServiceClient() = default;
+void HeartbeatServiceClient::OnError(HeartbeatResponseCallback callback,
+ const ProtobufHttpStatus& status) {
+ CHECK(!status.ok());
+ std::move(callback).Run(status, /*wait_interval=*/std::nullopt,
+ /*primary_user_email=*/"",
+ /*require_session_authorization=*/std::nullopt,
+ /*use_lite_heartbeat=*/std::nullopt);
+}
+
} // namespace remoting
diff --git a/remoting/host/heartbeat_service_client.h b/remoting/host/heartbeat_service_client.h
index 74f0bb2f..11b89ef 100644
--- a/remoting/host/heartbeat_service_client.h
+++ b/remoting/host/heartbeat_service_client.h
@@ -54,6 +54,11 @@
// Cancels any pending service requests.
virtual void CancelPendingRequests() = 0;
+
+ // Common error handler for all HeartbeatServiceClients. Should only be called
+ // if the status response for a heartbeat is an error.
+ void OnError(HeartbeatResponseCallback callback,
+ const ProtobufHttpStatus& status);
};
} // namespace remoting
diff --git a/remoting/host/me2me_heartbeat_service_client.cc b/remoting/host/me2me_heartbeat_service_client.cc
index 4869661..eb1cd80b 100644
--- a/remoting/host/me2me_heartbeat_service_client.cc
+++ b/remoting/host/me2me_heartbeat_service_client.cc
@@ -63,6 +63,11 @@
std::unique_ptr<apis::v1::HeartbeatResponse> response) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (!status.ok()) {
+ OnError(std::move(callback), status);
+ return;
+ }
+
base::TimeDelta wait_interval =
base::Seconds(response->set_interval_seconds());
bool use_lite_heartbeat = false;
@@ -89,6 +94,11 @@
std::unique_ptr<apis::v1::SendHeartbeatResponse> response) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (!status.ok()) {
+ OnError(std::move(callback), status);
+ return;
+ }
+
base::TimeDelta wait_interval =
base::Seconds(response->wait_interval_seconds());
std::move(callback).Run(status, std::make_optional(wait_interval), "",