Reland "Added network stats to GetActions RPC."

This is a reland of commit e31ba27

Fixed type conversion warnings.

Original change's description:
> Added network stats to GetActions RPC.
>
> NextScriptActions requests will now contain some basic stats on the
> network cost incurred by the previously received action response. This
> is added to allow tracking the impact of client-side-execution on
> overall response sizes, both per-response and aggregated over the
> entire flow.
>
> Also cleaned up MockService and ProtocolUtilsTest.
>
> Bug: b/201970915
> Change-Id: I6af7dea7f94dd40bc7de0fb6e726ba997a777949
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3545250
> Reviewed-by: Luca Hunkeler <hluca@google.com>
> Reviewed-by: Florian Gauger <fga@google.com>
> Commit-Queue: Clemens Arbesser <arbesser@google.com>
> Cr-Commit-Position: refs/heads/main@{#987899}

Bug: b/201970915
Change-Id: I967b42b2f7cb132866e13324058f4027da6ddad4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3568162
Reviewed-by: Luca Hunkeler <hluca@google.com>
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/main@{#988498}

components/autofill_assistant/browser/protocol_utils.cc

@@ -201,6 +201,7 @@ std::string ProtocolUtils::CreateNextScriptActionsRequest(
     const std::string& script_payload,
     const std::vector<ProcessedActionProto>& processed_actions,
     const RoundtripTimingStats& timing_stats,
+    const RoundtripNetworkStats& network_stats,
     const ClientContextProto& client_context) {
   ScriptActionRequestProto request_proto;
   request_proto.set_global_payload(global_payload);
@@ -211,6 +212,7 @@ std::string ProtocolUtils::CreateNextScriptActionsRequest(
     next_request->add_processed_actions()->MergeFrom(processed_action);
   }
   *next_request->mutable_timing_stats() = timing_stats;
+  *next_request->mutable_network_stats() = network_stats;
   *request_proto.mutable_client_context() = client_context;
   std::string serialized_request_proto;
   bool success = request_proto.SerializeToString(&serialized_request_proto);
@@ -966,4 +968,26 @@ std::string ProtocolUtils::CreateGetUserDataRequest(
   return serialized_request_proto;
 }
 
+// static
+RoundtripNetworkStats ProtocolUtils::ComputeNetworkStats(
+    const std::string& response,
+    const ServiceRequestSender::ResponseInfo& response_info,
+    const std::vector<std::unique_ptr<Action>>& actions) {
+  RoundtripNetworkStats stats;
+  stats.set_roundtrip_decoded_body_size_bytes(response.size());
+  stats.set_roundtrip_encoded_body_size_bytes(
+      response_info.encoded_body_length);
+  for (const auto& action : actions) {
+    RoundtripNetworkStats::ActionNetworkStats* action_stats =
+        stats.add_action_stats();
+    action_stats->set_action_info_case(
+        static_cast<int>(action->proto().action_info_case()));
+
+    std::string serialized_action;
+    action->proto().SerializeToString(&serialized_action);
+    action_stats->set_decoded_size_bytes(serialized_action.size());
+  }
+  return stats;
+}
+
 }  // namespace autofill_assistant

components/autofill_assistant/browser/protocol_utils.h

@@ -15,6 +15,7 @@
 #include "components/autofill_assistant/browser/script.h"
 #include "components/autofill_assistant/browser/script_parameters.h"
 #include "components/autofill_assistant/browser/service.pb.h"
+#include "components/autofill_assistant/browser/service/service_request_sender.h"
 #include "components/autofill_assistant/browser/trigger_scripts/trigger_script.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
@@ -64,6 +65,7 @@ class ProtocolUtils {
       const std::string& script_payload,
       const std::vector<ProcessedActionProto>& processed_actions,
       const RoundtripTimingStats& timing_stats,
+      const RoundtripNetworkStats& network_stats,
       const ClientContextProto& client_context);
 
   // Create request to get the available trigger scripts for |url|.
@@ -131,6 +133,13 @@ class ProtocolUtils {
       absl::optional<int>* trigger_condition_timeout_ms,
       absl::optional<std::unique_ptr<ScriptParameters>>* script_parameters);
 
+  // Computes network stats for a roundtrip that returned |response| and
+  // |response_info|, which were successfully parsed into |actions|.
+  static RoundtripNetworkStats ComputeNetworkStats(
+      const std::string& response,
+      const ServiceRequestSender::ResponseInfo& response_info,
+      const std::vector<std::unique_ptr<Action>>& actions);
+
  private:
   // Checks that the |trigger_condition| is well-formed (e.g. does not contain
   // regexes that cannot be compiled).

components/autofill_assistant/browser/protocol_utils_unittest.cc

@@ -48,19 +48,6 @@ class ProtocolUtilsTest : public testing::Test {
   ClientContextProto client_context_proto_;
 };
 
-void AssertClientContext(const ClientContextProto& context) {
-  EXPECT_EQ("1,2,3", context.experiment_ids());
-  EXPECT_TRUE(context.is_cct());
-  EXPECT_EQ("v", context.chrome().chrome_version());
-  EXPECT_EQ(1, context.device_context().version().sdk_int());
-  EXPECT_EQ("ma", context.device_context().manufacturer());
-  EXPECT_EQ("mo", context.device_context().model());
-  EXPECT_FALSE(context.is_onboarding_shown());
-  EXPECT_FALSE(context.is_direct_action());
-  EXPECT_THAT(context.accounts_matching_status(),
-              Eq(ClientContextProto::UNKNOWN));
-}
-
 TEST_F(ProtocolUtilsTest, ScriptMissingPath) {
   SupportedScriptProto script;
   std::vector<std::unique_ptr<Script>> scripts;
@@ -128,7 +115,7 @@ TEST_F(ProtocolUtilsTest, CreateInitialScriptActionsRequest) {
   EXPECT_THAT(initial.script_parameters(),
               UnorderedElementsAreArray(parameters.ToProto()));
 
-  AssertClientContext(request.client_context());
+  EXPECT_EQ(request.client_context(), client_context_proto_);
   EXPECT_EQ("global_payload", request.global_payload());
   EXPECT_EQ("script_payload", request.script_payload());
   EXPECT_EQ("bundle/path", initial.script_store_config().bundle_path());
@@ -139,12 +126,24 @@ TEST_F(ProtocolUtilsTest, CreateNextScriptActionsRequest) {
   ScriptActionRequestProto request;
   std::vector<ProcessedActionProto> processed_actions;
   processed_actions.emplace_back(ProcessedActionProto());
+
+  RoundtripNetworkStats network_stats;
+  network_stats.set_roundtrip_encoded_body_size_bytes(12345);
+  network_stats.set_roundtrip_decoded_body_size_bytes(23456);
+  auto* action_stats = network_stats.add_action_stats();
+  action_stats->set_action_info_case(5);
+  action_stats->set_decoded_size_bytes(35);
+  action_stats = network_stats.add_action_stats();
+  action_stats->set_action_info_case(7);
+  action_stats->set_decoded_size_bytes(15);
+
   EXPECT_TRUE(
       request.ParseFromString(ProtocolUtils::CreateNextScriptActionsRequest(
           "global_payload", "script_payload", processed_actions,
-          RoundtripTimingStats(), client_context_proto_)));
+          RoundtripTimingStats(), network_stats, client_context_proto_)));
 
-  AssertClientContext(request.client_context());
+  EXPECT_EQ(request.client_context(), client_context_proto_);
+  EXPECT_EQ(request.next_request().network_stats(), network_stats);
   EXPECT_EQ(1, request.next_request().processed_actions().size());
 }
 
@@ -154,7 +153,7 @@ TEST_F(ProtocolUtilsTest, CreateGetScriptsRequest) {
   EXPECT_TRUE(request.ParseFromString(ProtocolUtils::CreateGetScriptsRequest(
       GURL("http://example.com/"), client_context_proto_, parameters)));
 
-  AssertClientContext(request.client_context());
+  EXPECT_EQ(request.client_context(), client_context_proto_);
   EXPECT_THAT(request.script_parameters(),
               UnorderedElementsAreArray(parameters.ToProto()));
   EXPECT_EQ("http://example.com/", request.url());
@@ -344,7 +343,7 @@ TEST_F(ProtocolUtilsTest, CreateGetTriggerScriptsRequest) {
       request.ParseFromString(ProtocolUtils::CreateGetTriggerScriptsRequest(
           GURL("http://example.com/"), client_context_proto_, parameters)));
 
-  AssertClientContext(request.client_context());
+  EXPECT_EQ(request.client_context(), client_context_proto_);
   EXPECT_THAT(request.script_parameters(),
               UnorderedElementsAreArray(
                   ScriptParameters(base::flat_map<std::string, std::string>{
@@ -629,4 +628,40 @@ TEST_F(ProtocolUtilsTest, CreateGetUserDataRequest) {
               ElementsAre("VISA", "MASTERCARD"));
 }
 
+TEST_F(ProtocolUtilsTest, ComputeNetworkStats) {
+  ActionProto tell_action;
+  tell_action.mutable_tell()->set_message("Hello world!");
+  std::string serialized_tell_action;
+  tell_action.SerializeToString(&serialized_tell_action);
+
+  ActionProto stop_action;
+  stop_action.mutable_stop()->set_close_cct(false);
+  std::string serialized_stop_action;
+  stop_action.SerializeToString(&serialized_stop_action);
+
+  std::vector<std::unique_ptr<Action>> actions;
+  actions.push_back(ProtocolUtils::ParseAction(/* delegate = */ nullptr,
+                                               serialized_tell_action));
+  actions.push_back(ProtocolUtils::ParseAction(/* delegate = */ nullptr,
+                                               serialized_stop_action));
+
+  ServiceRequestSender::ResponseInfo response_info;
+  response_info.encoded_body_length = 20;
+
+  RoundtripNetworkStats expected_stats;
+  expected_stats.set_roundtrip_encoded_body_size_bytes(20);
+  expected_stats.set_roundtrip_decoded_body_size_bytes(28);
+  auto* action_stats = expected_stats.add_action_stats();
+  action_stats->set_action_info_case(11);  // == tell
+  action_stats->set_decoded_size_bytes(serialized_tell_action.size());
+  action_stats = expected_stats.add_action_stats();
+  action_stats->set_action_info_case(35);  // == stop
+  action_stats->set_decoded_size_bytes(serialized_stop_action.size());
+
+  EXPECT_EQ(ProtocolUtils::ComputeNetworkStats(
+                /* response = */ "This string is 28 bytes long", response_info,
+                actions),
+            expected_stats);
+}
+
 }  // namespace autofill_assistant

components/autofill_assistant/browser/script_executor.cc

@@ -822,8 +822,8 @@ void ScriptExecutor::OnGetActions(
   // Doesn't trigger when the script is completed.
   roundtrip_timing_stats_.set_roundtrip_time_ms(
       roundtrip_duration.InMilliseconds());
-  bool success =
-      http_status == net::HTTP_OK && ProcessNextActionResponse(response);
+  bool success = http_status == net::HTTP_OK &&
+                 ProcessNextActionResponse(response, response_info);
   if (should_stop_script_) {
     // The last action forced the script to stop. Sending the result of the
     // action is considered best effort in this situation. Report a successful
@@ -858,7 +858,9 @@ void ScriptExecutor::OnGetActions(
   RunCallback(true);
 }
 
-bool ScriptExecutor::ProcessNextActionResponse(const std::string& response) {
+bool ScriptExecutor::ProcessNextActionResponse(
+    const std::string& response,
+    const ServiceRequestSender::ResponseInfo& response_info) {
   processed_actions_.clear();
   actions_.clear();
 
@@ -871,6 +873,8 @@ bool ScriptExecutor::ProcessNextActionResponse(const std::string& response) {
     return false;
   }
 
+  roundtrip_network_stats_ =
+      ProtocolUtils::ComputeNetworkStats(response, response_info, actions_);
   ReportPayloadsToListener();
   if (should_update_scripts) {
     ReportScriptsUpdateToListener(std::move(scripts));
@@ -965,7 +969,7 @@ void ScriptExecutor::GetNextActions() {
       TriggerContext(
           {delegate_->GetTriggerContext(), additional_context_.get()}),
       last_global_payload_, last_script_payload_, processed_actions_,
-      roundtrip_timing_stats_,
+      roundtrip_timing_stats_, roundtrip_network_stats_,
       base::BindOnce(&ScriptExecutor::OnGetActions,
                      weak_ptr_factory_.GetWeakPtr(), get_next_actions_start));
 }

components/autofill_assistant/browser/script_executor.h

@@ -273,7 +273,9 @@ class ScriptExecutor : public ActionDelegate,
                     int http_status,
                     const std::string& response,
                     const ServiceRequestSender::ResponseInfo& response_info);
-  bool ProcessNextActionResponse(const std::string& response);
+  bool ProcessNextActionResponse(
+      const std::string& response,
+      const ServiceRequestSender::ResponseInfo& response_info);
   void ReportPayloadsToListener();
   void ReportScriptsUpdateToListener(
       std::vector<std::unique_ptr<Script>> scripts);
@@ -392,6 +394,7 @@ class ScriptExecutor : public ActionDelegate,
 
   base::TimeTicks batch_start_time_;
   RoundtripTimingStats roundtrip_timing_stats_;
+  RoundtripNetworkStats roundtrip_network_stats_;
 
   bool connection_warning_already_shown_ = false;
   bool website_warning_already_shown_ = false;