Fix static initializers in remoting::ProtobufHttpStatus.

- Change ProtobufHttpStatus::OK, a static instance of
  ProtobufHttpStatus initialized at runtime, to OK(). Internally, OK()
  will create the instance on first use.
- Remove ProtobufHttpStatus::CANCELLED, which is only used in test code.

Bug: 537099
Change-Id: Icc7d2865e9721157cff3c5f477f4a418ab1a1b20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437734
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812039}

remoting/base/protobuf_http_request.cc

@@ -78,7 +78,7 @@ ProtobufHttpStatus ProtobufHttpRequest::ParseResponse(
     LOG(ERROR) << "Failed to parse response body";
     return ProtobufHttpStatus(net::ERR_INVALID_RESPONSE);
   }
-  return ProtobufHttpStatus::OK;
+  return ProtobufHttpStatus::OK();
 }
 
 }  // namespace remoting

remoting/base/protobuf_http_status.cc

@@ -4,6 +4,7 @@
 
 #include "remoting/base/protobuf_http_status.h"
 
+#include "base/no_destructor.h"
 #include "net/http/http_status_code.h"
 #include "remoting/base/protobuf_http_client_messages.pb.h"
 
@@ -11,7 +12,7 @@ namespace remoting {
 
 namespace {
 
-ProtobufHttpStatus::Code HttpStatusCodeToClientCode(
+constexpr ProtobufHttpStatus::Code HttpStatusCodeToClientCode(
     net::HttpStatusCode http_status_code) {
   DCHECK_LT(0, http_status_code);
   switch (http_status_code) {
@@ -45,7 +46,7 @@ ProtobufHttpStatus::Code HttpStatusCodeToClientCode(
   }
 }
 
-ProtobufHttpStatus::Code NetErrorToClientCode(net::Error net_error) {
+constexpr ProtobufHttpStatus::Code NetErrorToClientCode(net::Error net_error) {
   DCHECK_GT(0, net_error);
   DCHECK_NE(net::Error::ERR_HTTP_RESPONSE_CODE_FAILURE, net_error)
       << "Use the HttpStatusCode overload";
@@ -72,11 +73,10 @@ ProtobufHttpStatus::Code NetErrorToClientCode(net::Error net_error) {
 
 }  // namespace
 
-const ProtobufHttpStatus& ProtobufHttpStatus::OK =
-    ProtobufHttpStatus(Code::OK, "OK");
-
-const ProtobufHttpStatus& ProtobufHttpStatus::CANCELLED =
-    ProtobufHttpStatus(Code::CANCELLED, "Cancelled");
+const ProtobufHttpStatus& ProtobufHttpStatus::OK() {
+  static const base::NoDestructor<ProtobufHttpStatus> kOK(Code::OK, "OK");
+  return *kOK;
+}
 
 ProtobufHttpStatus::ProtobufHttpStatus(net::HttpStatusCode http_status_code)
     : error_code_(HttpStatusCodeToClientCode(http_status_code)),

remoting/base/protobuf_http_status.h

@@ -40,9 +40,7 @@ class ProtobufHttpStatus {
   };
 
   // An OK pre-defined instance.
-  static const ProtobufHttpStatus& OK;
-  // A CANCELLED pre-defined instance.
-  static const ProtobufHttpStatus& CANCELLED;
+  static const ProtobufHttpStatus& OK();
 
   explicit ProtobufHttpStatus(net::HttpStatusCode http_status_code);
   explicit ProtobufHttpStatus(net::Error net_error);

remoting/host/heartbeat_sender_unittest.cc

@@ -93,7 +93,7 @@ decltype(auto) DoValidateHeartbeatAndRespondOk(
                       expected_host_offline_reason, is_googler);
     auto response = std::make_unique<apis::v1::HeartbeatResponse>();
     response->set_set_interval_seconds(kGoodIntervalSeconds);
-    std::move(callback).Run(ProtobufHttpStatus::OK, std::move(response));
+    std::move(callback).Run(ProtobufHttpStatus::OK(), std::move(response));
   };
 }
 
@@ -315,7 +315,7 @@ TEST_F(HeartbeatSenderTest, RemoteCommand) {
         auto response = std::make_unique<apis::v1::HeartbeatResponse>();
         response->set_set_interval_seconds(kGoodIntervalSeconds);
         response->set_remote_command(apis::v1::HeartbeatResponse::RESTART_HOST);
-        std::move(callback).Run(ProtobufHttpStatus::OK, std::move(response));
+        std::move(callback).Run(ProtobufHttpStatus::OK(), std::move(response));
       });
   EXPECT_CALL(*mock_observer_, OnHeartbeatSent()).Times(1);
   EXPECT_CALL(mock_delegate_, OnFirstHeartbeatSuccessful()).Times(1);

remoting/host/remoting_register_support_host_request_unittest.cc

@@ -45,7 +45,7 @@ void RespondOk(RegisterSupportHostResponseCallback callback) {
   auto response = std::make_unique<apis::v1::RegisterSupportHostResponse>();
   response->set_support_id(kSupportId);
   response->set_support_id_lifetime_seconds(kSupportIdLifetime.InSeconds());
-  std::move(callback).Run(ProtobufHttpStatus::OK, std::move(response));
+  std::move(callback).Run(ProtobufHttpStatus::OK(), std::move(response));
 }
 
 decltype(auto) DoValidateRegisterHostAndRespondOk() {

remoting/ios/facade/host_list_service_unittest.mm

@@ -157,7 +157,7 @@ TEST_F(HostListServiceTest, SuccessfullyFetchedOneHost) {
   EXPECT_HOST_LIST_STATE(HostListService::State::FETCHING);
   EXPECT_NO_FETCH_FAILURE();
 
-  BlockAndRespondGetHostList(ProtobufHttpStatus::OK,
+  BlockAndRespondGetHostList(ProtobufHttpStatus::OK(),
                              CreateFakeHostListResponse("fake_host_1"));
   EXPECT_HOST_LIST_STATE(HostListService::State::FETCHED);
   EXPECT_NO_FETCH_FAILURE();
@@ -180,7 +180,7 @@ TEST_F(HostListServiceTest, SuccessfullyFetchedTwoHosts_HostListSorted) {
   apis::v1::GetHostListResponse response;
   response.add_hosts()->set_host_name("Host 2");
   response.add_hosts()->set_host_name("Host 1");
-  BlockAndRespondGetHostList(ProtobufHttpStatus::OK, response);
+  BlockAndRespondGetHostList(ProtobufHttpStatus::OK(), response);
   EXPECT_HOST_LIST_STATE(HostListService::State::FETCHED);
   EXPECT_NO_FETCH_FAILURE();
 
@@ -276,7 +276,7 @@ TEST_F(HostListServiceTest, UserSwitchAccount_cancelThenRequestNewFetch) {
   ASSERT_TRUE(test_responder_.GetPendingRequest(1).client.is_connected());
   EXPECT_HOST_LIST_STATE(HostListService::State::FETCHING);
 
-  BlockAndRespondGetHostList(ProtobufHttpStatus::OK,
+  BlockAndRespondGetHostList(ProtobufHttpStatus::OK(),
                              CreateFakeHostListResponse("fake_host_id"));
   EXPECT_HOST_LIST_STATE(HostListService::State::FETCHED);
 

remoting/signaling/ftl_message_reception_channel_unittest.cc

@@ -323,7 +323,9 @@ TEST_F(FtlMessageReceptionChannelTest, StreamsTwoMessages) {
             *response->mutable_inbox_message() = message_2;
             on_incoming_msg.Run(std::move(response));
 
-            std::move(on_channel_closed).Run(ProtobufHttpStatus::CANCELLED);
+            const ProtobufHttpStatus kCancel(
+                ProtobufHttpStatus::Code::CANCELLED, "Cancelled");
+            std::move(on_channel_closed).Run(kCancel);
           }));
 
   channel_->StartReceivingMessages(
@@ -415,7 +417,7 @@ TEST_F(FtlMessageReceptionChannelTest, ServerClosesStream_ResetsStream) {
             std::move(on_channel_ready).Run();
 
             // Close the stream with OK.
-            std::move(on_channel_closed).Run(ProtobufHttpStatus::OK);
+            std::move(on_channel_closed).Run(ProtobufHttpStatus::OK());
           },
           &old_stream))
       .WillOnce(StartStream(

remoting/signaling/ftl_messaging_client_unittest.cc

@@ -404,7 +404,7 @@ TEST_F(FtlMessagingClientTest,
           FROM_HERE, ProtobufHttpStatus::Code::OK, &run_loop));
 
   test_responder_.AddStreamResponseToMostRecentRequestUrl(
-      {&response_1, &response_2}, ProtobufHttpStatus::OK);
+      {&response_1, &response_2}, ProtobufHttpStatus::OK());
 
   run_loop.Run();
 }

remoting/signaling/ftl_registration_manager_unittest.cc

@@ -53,7 +53,7 @@ decltype(auto) RespondOkToSignInGaia(const std::string& registration_id) {
     response->mutable_auth_token()->set_payload(kAuthToken);
     response->mutable_auth_token()->set_expires_in(
         kAuthTokenExpiresInMicroseconds);
-    std::move(on_done).Run(ProtobufHttpStatus::OK, std::move(response));
+    std::move(on_done).Run(ProtobufHttpStatus::OK(), std::move(response));
   };
 }
 

remoting/signaling/ftl_signal_strategy_unittest.cc

@@ -168,7 +168,7 @@ class FakeRegistrationManager : public RegistrationManager {
   void ExpectSignInGaiaSucceeds() {
     EXPECT_CALL(*this, SignInGaia(_)).WillOnce([&](DoneCallback callback) {
       is_signed_in_ = true;
-      std::move(callback).Run(ProtobufHttpStatus::OK);
+      std::move(callback).Run(ProtobufHttpStatus::OK());
     });
   }
 
@@ -380,7 +380,7 @@ TEST_F(FtlSignalStrategyTest, SendStanza_Success) {
                                 const ftl::ChromotingMessage& message,
                                 MessagingClient::DoneCallback on_done) {
         ASSERT_EQ(stanza_string, message.xmpp().stanza());
-        std::move(on_done).Run(ProtobufHttpStatus::OK);
+        std::move(on_done).Run(ProtobufHttpStatus::OK());
       });
   signal_strategy_->SendStanza(std::move(stanza));
 }
@@ -530,7 +530,7 @@ TEST_F(FtlSignalStrategyTest, SendMessage_Success) {
                                   const ftl::ChromotingMessage& message,
                                   MessagingClient::DoneCallback on_done) {
         ASSERT_EQ(message_payload, message.xmpp().stanza());
-        std::move(on_done).Run(ProtobufHttpStatus::OK);
+        std::move(on_done).Run(ProtobufHttpStatus::OK());
       });
 
   signal_strategy_->SendMessage(

remoting/signaling/remoting_log_to_server_unittest.cc

@@ -66,7 +66,7 @@ TEST_F(RemotingLogToServerTest, SuccessfullySendOneLog) {
         ASSERT_EQ("test-key", request.payload().entry().field(0).key());
         ASSERT_EQ("test-value", request.payload().entry().field(0).value());
         std::move(callback).Run(
-            ProtobufHttpStatus::OK,
+            ProtobufHttpStatus::OK(),
             std::make_unique<apis::v1::CreateLogEntryResponse>());
       });
 
@@ -161,10 +161,10 @@ TEST_F(RemotingLogToServerTest, FailedToSendTwoLogs_RetryThenSucceeds) {
   ASSERT_EQ(2, GetBackoffEntry().failure_count());
 
   std::move(response_callback_1)
-      .Run(ProtobufHttpStatus::OK,
+      .Run(ProtobufHttpStatus::OK(),
            std::make_unique<apis::v1::CreateLogEntryResponse>());
   std::move(response_callback_2)
-      .Run(ProtobufHttpStatus::OK,
+      .Run(ProtobufHttpStatus::OK(),
            std::make_unique<apis::v1::CreateLogEntryResponse>());
   task_environment_.FastForwardUntilNoTasksRemain();
   ASSERT_EQ(0, GetBackoffEntry().failure_count());

testing/scripts/check_static_initializers.py

@@ -37,7 +37,6 @@ _CROS_SI_FILE_ALLOWLIST = {
         'iostream.cpp',  # TODO(crbug.com/973554): Remove.
         'spinlock.cc',  # TODO(crbug.com/973556): Remove.
         'int256.cc',  # TODO(crbug.com/537099): Remove.
-        'protobuf_http_status.cc',  # TODO(crbug.com/537099): Remove.
         'rpc.pb.cc',  # TODO(crbug.com/537099): Remove.
     ],
     'nacl_helper_bootstrap': [],