[Autofill Assistant] Uses generic detail parameters.

Generic details parameters takes precedence, when they are provided.
Otherwise old logic of creating Details based on vertical specific
parameters is used.

Bug: 806868
Change-Id: I0bc0f77af3f86c9e7482c0893dc10be65b797883
Reviewed-on: https://chromium-review.googlesource.com/c/1488882
Commit-Queue: Lukasz Suder <lsuder@chromium.org>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635948}

chrome/browser/android/autofill_assistant/ui_controller_android.cc

@@ -515,8 +515,8 @@ void UiControllerAndroid::OnDetailsChanged(const Details* details) {
       proto.datetime().date().year(), proto.datetime().date().month(),
       proto.datetime().date().day(), proto.datetime().time().hour(),
       proto.datetime().time().minute(), proto.datetime().time().second(),
-      base::android::ConvertUTF8ToJavaString(env, proto.description_line1()),
-      base::android::ConvertUTF8ToJavaString(env, proto.description_line2()),
+      base::android::ConvertUTF8ToJavaString(env, proto.description_line_1()),
+      base::android::ConvertUTF8ToJavaString(env, proto.description_line_2()),
       changes.user_approval_required(), changes.highlight_title(),
       changes.highlight_line1(), changes.highlight_line2());
   Java_AssistantDetailsModel_setDetails(env, jmodel, jdetails);

components/autofill_assistant/browser/details.cc

@@ -13,9 +13,9 @@ namespace autofill_assistant {
 
 Details::Details(const ShowDetailsProto& proto) : proto_(proto) {
   // Legacy treatment for old proto fields. Can be removed once the backend
-  // is updated to set the description_line1/line2 fields.
-  if (details().has_description() && !details().has_description_line2()) {
-    proto_.mutable_details()->set_description_line2(details().description());
+  // is updated to set the description_line_1/line_2 fields.
+  if (details().has_description() && !details().has_description_line_2()) {
+    proto_.mutable_details()->set_description_line_2(details().description());
   }
 }
 
@@ -30,13 +30,13 @@ base::Value Details::GetDebugContext() const {
   if (!details().total_price().empty())
     dict.SetKey("total_price", base::Value(details().total_price()));
 
-  if (!details().description_line1().empty())
-    dict.SetKey("description_line1",
-                base::Value(details().description_line1()));
+  if (!details().description_line_1().empty())
+    dict.SetKey("description_line_1",
+                base::Value(details().description_line_1()));
 
-  if (!details().description_line2().empty())
-    dict.SetKey("description_line2",
-                base::Value(details().description_line2()));
+  if (!details().description_line_2().empty())
+    dict.SetKey("description_line_2",
+                base::Value(details().description_line_2()));
 
   if (details().has_datetime()) {
     dict.SetKey("datetime",
@@ -63,39 +63,30 @@ base::Value Details::GetDebugContext() const {
 
 bool Details::UpdateFromParameters(
     const std::map<std::string, std::string>& parameters) {
-  bool is_updated = false;
-
-  const std::unordered_set<std::string> title_parameters = {"MOVIES_MOVIE_NAME",
-                                                            "TITLE"};
-  const std::unordered_set<std::string> description_line1_parameters = {
-      "DESCRIPTION_LINE_1"};
-  const std::unordered_set<std::string> description_line2_parameters = {
-      "MOVIES_THEATER_NAME", "DESCRIPTION_LINE_2"};
+  if (MaybeUpdateFromDetailsParameters(parameters)) {
+    return true;
+  }
 
+  // NOTE: The logic below is only needed for backward compatibility.
+  // Remove once we always pass detail parameters.
+  bool is_updated = false;
   for (const auto& iter : parameters) {
-    if (title_parameters.find(iter.first) != title_parameters.end()) {
+    std::string key = iter.first;
+    if (key == "MOVIES_MOVIE_NAME") {
       proto_.mutable_details()->set_title(iter.second);
       is_updated = true;
       continue;
     }
 
-    if (description_line1_parameters.find(iter.first) !=
-        description_line1_parameters.end()) {
-      proto_.mutable_details()->set_description_line1(iter.second);
-      is_updated = true;
-      continue;
-    }
-
-    if (description_line2_parameters.find(iter.first) !=
-        description_line2_parameters.end()) {
-      proto_.mutable_details()->set_description_line2(iter.second);
+    if (key == "MOVIES_THEATER_NAME") {
+      proto_.mutable_details()->set_description_line_2(iter.second);
       is_updated = true;
       continue;
     }
 
     if (iter.first.compare("MOVIES_SCREENING_DATETIME") == 0) {
       // TODO(crbug.com/806868): Parse the string here and fill
-      // proto.description_line1, then get rid of datetime_ in Details.
+      // proto.description_line_1, then get rid of datetime_ in Details.
       datetime_ = iter.second;
       is_updated = true;
       continue;
@@ -104,6 +95,51 @@ bool Details::UpdateFromParameters(
   return is_updated;
 }
 
+bool Details::MaybeUpdateFromDetailsParameters(
+    const std::map<std::string, std::string>& parameters) {
+  bool details_updated = false;
+  for (const auto& iter : parameters) {
+    std::string key = iter.first;
+    if (key == "DETAILS_TITLE") {
+      proto_.mutable_details()->set_title(iter.second);
+      details_updated = true;
+      continue;
+    }
+
+    if (key == "DETAILS_DESCRIPTION_LINE_1") {
+      proto_.mutable_details()->set_description_line_1(iter.second);
+      details_updated = true;
+      continue;
+    }
+
+    if (key == "DETAILS_DESCRIPTION_LINE_2") {
+      proto_.mutable_details()->set_description_line_2(iter.second);
+      details_updated = true;
+      continue;
+    }
+
+    if (key == "DETAILS_IMAGE_URL") {
+      proto_.mutable_details()->set_image_url(iter.second);
+      details_updated = true;
+      continue;
+    }
+
+    if (key == "DETAILS_TOTAL_PRICE_LABEL") {
+      proto_.mutable_details()->set_total_price_label(iter.second);
+      details_updated = true;
+      continue;
+    }
+
+    if (key == "DETAILS_TOTAL_PRICE") {
+      proto_.mutable_details()->set_total_price(iter.second);
+      details_updated = true;
+      continue;
+    }
+  }
+
+  return details_updated;
+}
+
 void Details::ClearChanges() {
   proto_.clear_change_flags();
 }

components/autofill_assistant/browser/details.h

@@ -28,6 +28,8 @@ class Details {
 
   // Update details from the given parameters. Returns true if changes were
   // made.
+  // If one of the generic detail parameter is present then vertical specific
+  // parameters are not used for Details creation.
   bool UpdateFromParameters(
       const std::map<std::string, std::string>& parameters);
 
@@ -38,11 +40,16 @@ class Details {
   const DetailsProto& details() const { return proto_.details(); }
   const DetailsChangesProto& changes() const { return proto_.change_flags(); }
 
+  // Tries updating the details using generic detail parameters. Returns true
+  // if at least one generic detail parameter was found and used.
+  bool MaybeUpdateFromDetailsParameters(
+      const std::map<std::string, std::string>& parameters);
+
   ShowDetailsProto proto_;
-  // RFC 3339 date-time. Ignore if proto.description_line1 is set.
+  // RFC 3339 date-time. Ignore if proto.description_line_1 is set.
   //
   // TODO(crbug.com/806868): parse RFC 3339 date-time on the C++ side and fill
-  // proto.description_line1 with the result instead of carrying a string
+  // proto.description_line_1 with the result instead of carrying a string
   // representation of the datetime.
   std::string datetime_;
 };

components/autofill_assistant/browser/service.proto

@@ -656,11 +656,11 @@ message DetailsProto {
   // in the client's locale (e.g., $123.00).
   optional string total_price = 6;
 
-  optional string description_line1 = 7;
-  optional string description_line2 = 8;
+  optional string description_line_1 = 7;
+  optional string description_line_2 = 8;
 
   // Deprecated, but currently still necessary and supported. We can get rid of
-  // these fields when the backend starts setting description_line1 and 2.
+  // these fields when the backend starts setting description_line_1 and 2.
   optional DateTimeProto datetime = 3;
   optional string description = 4;