Commit 110e2dc7 authored by Lukasz Suder's avatar Lukasz Suder Committed by Commit Bot

[Autofill Assistant] Checks the mId of details.

Comparing just title is fragile (e.g. different languages),
therefore we allow specifing the mid inside parameters.

Bug: 806868
Change-Id: Ie5790beb6cb06bc4c124ae7a969c2263a77fc2cf
Reviewed-on: https://chromium-review.googlesource.com/c/1349335
Commit-Queue: Lukasz Suder <lsuder@chromium.org>
Reviewed-by: default avatarStephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610598}
parent ae471b4b
...@@ -325,8 +325,8 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -325,8 +325,8 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
} }
@CalledByNative @CalledByNative
private void onShowDetails(String title, String url, String description, int year, int month, private void onShowDetails(String title, String url, String description, String mId, int year,
int day, int hour, int minute, int second) { int month, int day, int hour, int minute, int second) {
Date date; Date date;
if (year > 0 && month > 0 && day > 0 && hour >= 0 && minute >= 0 && second >= 0) { if (year > 0 && month > 0 && day > 0 && hour >= 0 && minute >= 0 && second >= 0) {
Calendar calendar = Calendar.getInstance(); Calendar calendar = Calendar.getInstance();
...@@ -339,7 +339,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -339,7 +339,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
} }
maybeUpdateDetails(new Details( maybeUpdateDetails(new Details(
title, url, date, description, /* isFinal= */ true, Collections.emptySet())); title, url, date, description, mId, /* isFinal= */ true, Collections.emptySet()));
} }
@CalledByNative @CalledByNative
......
...@@ -22,29 +22,31 @@ import java.util.Set; ...@@ -22,29 +22,31 @@ import java.util.Set;
* Java side equivalent of autofill_assistant::DetailsProto. * Java side equivalent of autofill_assistant::DetailsProto.
*/ */
class Details { class Details {
enum DetailsField { TITLE, URL, DATE, DESCRIPTION, IS_FINAL } enum DetailsField { TITLE, URL, DATE, DESCRIPTION, MID, IS_FINAL }
private final String mTitle; private final String mTitle;
private final String mUrl; private final String mUrl;
@Nullable @Nullable
private final Date mDate; private final Date mDate;
private final String mDescription; private final String mDescription;
private final String mMid;
private final boolean mIsFinal; private final boolean mIsFinal;
/** Contains the fields that have changed when merging with other Details object. */ /** Contains the fields that have changed when merging with other Details object. */
private final Set<DetailsField> mFieldsChanged; private final Set<DetailsField> mFieldsChanged;
// NOTE: When adding a new field, update the isEmpty method. // NOTE: When adding a new field, update the isEmpty and toJSONObject methods.
static final Details EMPTY_DETAILS = static final Details EMPTY_DETAILS =
new Details("", "", null, "", false, Collections.emptySet()); new Details("", "", null, "", "", false, Collections.emptySet());
private static final String RFC_3339_FORMAT_WITHOUT_TIMEZONE = "yyyy'-'MM'-'dd'T'HH':'mm':'ss"; private static final String RFC_3339_FORMAT_WITHOUT_TIMEZONE = "yyyy'-'MM'-'dd'T'HH':'mm':'ss";
Details(String title, String url, @Nullable Date date, String description, boolean isFinal, Details(String title, String url, @Nullable Date date, String description, String mId,
Set<DetailsField> fieldsChanged) { boolean isFinal, Set<DetailsField> fieldsChanged) {
this.mTitle = title; this.mTitle = title;
this.mUrl = url; this.mUrl = url;
this.mDate = date; this.mDate = date;
this.mDescription = description; this.mDescription = description;
this.mMid = mId;
this.mIsFinal = isFinal; this.mIsFinal = isFinal;
this.mFieldsChanged = fieldsChanged; this.mFieldsChanged = fieldsChanged;
} }
...@@ -66,11 +68,16 @@ class Details { ...@@ -66,11 +68,16 @@ class Details {
return mDescription; return mDescription;
} }
private String getMid() {
return mMid;
}
JSONObject toJSONObject() { JSONObject toJSONObject() {
// Details are part of the feedback form, hence they need a JSON representation. // Details are part of the feedback form, hence they need a JSON representation.
Map<String, String> movieDetails = new HashMap<>(); Map<String, String> movieDetails = new HashMap<>();
movieDetails.put("title", mTitle); movieDetails.put("title", mTitle);
movieDetails.put("url", mUrl); movieDetails.put("url", mUrl);
movieDetails.put("mId", mMid);
if (mDate != null) movieDetails.put("date", mDate.toString()); if (mDate != null) movieDetails.put("date", mDate.toString());
movieDetails.put("description", mDescription); movieDetails.put("description", mDescription);
return new JSONObject(movieDetails); return new JSONObject(movieDetails);
...@@ -89,7 +96,8 @@ class Details { ...@@ -89,7 +96,8 @@ class Details {
} }
boolean isEmpty() { boolean isEmpty() {
return mTitle.isEmpty() && mUrl.isEmpty() && mDescription.isEmpty() && mDate == null; return mTitle.isEmpty() && mUrl.isEmpty() && mDescription.isEmpty() && mDate == null
&& mMid.isEmpty();
} }
// TODO(crbug.com/806868): Create a fallback when there are no parameters for details. // TODO(crbug.com/806868): Create a fallback when there are no parameters for details.
...@@ -97,17 +105,23 @@ class Details { ...@@ -97,17 +105,23 @@ class Details {
String title = ""; String title = "";
String description = ""; String description = "";
Date date = null; Date date = null;
String mId = "";
for (String key : parameters.keySet()) { for (String key : parameters.keySet()) {
if (key.contains("E_NAME")) { if (key.endsWith("E_NAME")) {
title = parameters.get(key); title = parameters.get(key);
continue; continue;
} }
if (key.contains("R_NAME")) { if (key.endsWith("R_NAME")) {
description = parameters.get(key); description = parameters.get(key);
continue; continue;
} }
if (key.endsWith("MID")) {
mId = parameters.get(key);
continue;
}
if (key.contains("DATETIME")) { if (key.contains("DATETIME")) {
try { try {
// The parameter contains the timezone shift from the current location, that we // The parameter contains the timezone shift from the current location, that we
...@@ -120,7 +134,7 @@ class Details { ...@@ -120,7 +134,7 @@ class Details {
} }
} }
return new Details(title, /* url= */ "", date, description, /* isFinal= */ false, return new Details(title, /* url= */ "", date, description, mId, /* isFinal= */ false,
/* fieldsChanged= */ Collections.emptySet()); /* fieldsChanged= */ Collections.emptySet());
} }
...@@ -136,27 +150,40 @@ class Details { ...@@ -136,27 +150,40 @@ class Details {
static Details merge(Details oldDetails, Details newDetails) { static Details merge(Details oldDetails, Details newDetails) {
if (oldDetails.isEmpty()) { if (oldDetails.isEmpty()) {
return new Details(newDetails.getTitle(), newDetails.getUrl(), newDetails.getDate(), return new Details(newDetails.getTitle(), newDetails.getUrl(), newDetails.getDate(),
newDetails.getDescription(), newDetails.isFinal(), Collections.emptySet()); newDetails.getDescription(), newDetails.getMid(), newDetails.isFinal(),
Collections.emptySet());
} }
Set<DetailsField> changedFields = new HashSet<>(); Set<DetailsField> changedFields = new HashSet<>();
// TODO(crbug.com/806868): Change title if mid doesn't match. String title = oldDetails.getTitle();
String title = String mId = oldDetails.getMid();
oldDetails.getTitle().isEmpty() ? newDetails.getTitle() : oldDetails.getTitle(); // Title and mId are tightly connected. If mId is different then title should also be
// different, but it's not true the other way (titles might slightly differ).
if (!oldDetails.getMid().isEmpty() && !newDetails.getMid().isEmpty()
&& !oldDetails.getMid().equals(newDetails.getMid())
&& !oldDetails.getTitle().equals(newDetails.getTitle())) {
changedFields.add(DetailsField.TITLE);
title = newDetails.getTitle();
mId = newDetails.getMid();
}
String url = oldDetails.getUrl().isEmpty() ? newDetails.getUrl() : oldDetails.getUrl(); String url = oldDetails.getUrl().isEmpty() ? newDetails.getUrl() : oldDetails.getUrl();
Date date = oldDetails.getDate(); Date date = oldDetails.getDate();
if (newDetails.getDate() != null && oldDetails.getDate() != null if (oldDetails.getDate() == null) {
&& !newDetails.getDate().equals(oldDetails.getDate())) { // There was no date. Use the new one unconditionally.
changedFields.add(DetailsField.DATE);
date = newDetails.getDate(); date = newDetails.getDate();
} else if (newDetails.getDate() != null
&& !oldDetails.getDate().equals(newDetails.getDate())) {
// Only if new date is different than old (which wasn't null) mark it as changedField.
date = newDetails.getDate();
changedFields.add(DetailsField.DATE);
} }
String description = oldDetails.getDescription().isEmpty() ? newDetails.getDescription() String description = oldDetails.getDescription().isEmpty() ? newDetails.getDescription()
: oldDetails.getDescription(); : oldDetails.getDescription();
boolean isFinal = newDetails.isFinal(); boolean isFinal = newDetails.isFinal();
return new Details(title, url, date, description, isFinal, changedFields); return new Details(title, url, date, description, mId, isFinal, changedFields);
} }
} }
\ No newline at end of file
...@@ -401,8 +401,9 @@ void UiControllerAndroid::ShowDetails(const DetailsProto& details, ...@@ -401,8 +401,9 @@ void UiControllerAndroid::ShowDetails(const DetailsProto& details,
env, java_autofill_assistant_ui_controller_, env, java_autofill_assistant_ui_controller_,
base::android::ConvertUTF8ToJavaString(env, details.title()), base::android::ConvertUTF8ToJavaString(env, details.title()),
base::android::ConvertUTF8ToJavaString(env, details.url()), base::android::ConvertUTF8ToJavaString(env, details.url()),
base::android::ConvertUTF8ToJavaString(env, details.description()), year, base::android::ConvertUTF8ToJavaString(env, details.description()),
month, day, hour, minute, second); base::android::ConvertUTF8ToJavaString(env, details.m_id()), year, month,
day, hour, minute, second);
} }
void UiControllerAndroid::ShowProgressBar(int progress, void UiControllerAndroid::ShowProgressBar(int progress,
......
...@@ -535,6 +535,10 @@ message DetailsProto { ...@@ -535,6 +535,10 @@ message DetailsProto {
optional DateTimeProto datetime = 3; optional DateTimeProto datetime = 3;
optional string description = 4; optional string description = 4;
// Mid that comes from Knowledge Graph. Uniquely identify the object that this
// proto describes.
optional string m_id = 5;
} }
// Show contextual information. // Show contextual information.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment