Commit 9aacd9fc authored by Lukasz Suder's avatar Lukasz Suder Committed by Commit Bot

[Autofill Assistant] Checks the details coming from Parameters.

The details and statusMessage were moved to controller.

If the details from parameters differ from details coming from Action, Autofill Assistant is closed.

Bug: 806868
Change-Id: Icb7da81fa7eca0bf4894632f4ca0c3a68b16ae1c
Reviewed-on: https://chromium-review.googlesource.com/c/1332192
Commit-Queue: Lukasz Suder <lsuder@chromium.org>
Reviewed-by: default avatarStephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607629}
parent 23bcf533
...@@ -60,12 +60,17 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -60,12 +60,17 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
/** Display the final message for that long before shutting everything down. */ /** Display the final message for that long before shutting everything down. */
private static final long GRACEFUL_SHUTDOWN_DELAY_MS = 5000; private static final long GRACEFUL_SHUTDOWN_DELAY_MS = 5000;
private static final String RFC_3339_FORMAT = "yyyy'-'MM'-'dd'T'HH':'mm':'ssZ"; private static final String RFC_3339_FORMAT_WITHOUT_TIMEZONE = "yyyy'-'MM'-'dd'T'HH':'mm':'ss";
private final WebContents mWebContents; private final WebContents mWebContents;
private final UiDelegateHolder mUiDelegateHolder; private final UiDelegateHolder mUiDelegateHolder;
private final String mInitialUrl; private final String mInitialUrl;
// TODO(crbug.com/806868): Move mCurrentDetails and mStatusMessage to a Model (refactor to MVC).
AutofillAssistantUiDelegate.Details mCurrentDetails =
AutofillAssistantUiDelegate.Details.getEmptyDetails();
private String mStatusMessage;
/** /**
* Native pointer to the UIController. * Native pointer to the UIController.
*/ */
...@@ -118,11 +123,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -118,11 +123,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
Map<String, String> parameters = extractParameters(activity.getInitialIntent().getExtras()); Map<String, String> parameters = extractParameters(activity.getInitialIntent().getExtras());
parameters.remove(PARAMETER_ENABLED); parameters.remove(PARAMETER_ENABLED);
AutofillAssistantUiDelegate.Details initialDetails = makeDetailsFromParameters(parameters); maybeUpdateDetails(makeDetailsFromParameters(parameters));
if (!initialDetails.isEmpty()) {
mUiDelegateHolder.performUiOperation(
uiDelegate -> uiDelegate.showDetails(initialDetails));
}
Tab activityTab = activity.getActivityTab(); Tab activityTab = activity.getActivityTab();
mWebContents = activityTab.getWebContents(); mWebContents = activityTab.getWebContents();
...@@ -162,6 +163,16 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -162,6 +163,16 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
mUiDelegateHolder.dismiss(R.string.autofill_assistant_stopped); mUiDelegateHolder.dismiss(R.string.autofill_assistant_stopped);
} }
@Override
public AutofillAssistantUiDelegate.Details getDetails() {
return mCurrentDetails;
}
@Override
public String getStatusMessage() {
return mStatusMessage;
}
@Override @Override
public void onUnexpectedTaps() { public void onUnexpectedTaps() {
mUiDelegateHolder.dismiss(R.string.autofill_assistant_maybe_give_up); mUiDelegateHolder.dismiss(R.string.autofill_assistant_maybe_give_up);
...@@ -222,7 +233,9 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -222,7 +233,9 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
if (key.contains("DATETIME")) { if (key.contains("DATETIME")) {
try { try {
date = new SimpleDateFormat(RFC_3339_FORMAT, Locale.getDefault()) // The parameter contains the timezone shift from the current location, that we
// don't care about.
date = new SimpleDateFormat(RFC_3339_FORMAT_WITHOUT_TIMEZONE, Locale.ROOT)
.parse(parameters.get(key)); .parse(parameters.get(key));
} catch (ParseException e) { } catch (ParseException e) {
// Ignore. // Ignore.
...@@ -241,6 +254,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -241,6 +254,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
@CalledByNative @CalledByNative
private void onShowStatusMessage(String message) { private void onShowStatusMessage(String message) {
mStatusMessage = message;
mUiDelegateHolder.performUiOperation(uiDelegate -> uiDelegate.showStatusMessage(message)); mUiDelegateHolder.performUiOperation(uiDelegate -> uiDelegate.showStatusMessage(message));
} }
...@@ -339,17 +353,38 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -339,17 +353,38 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
}); });
} }
/**
* Updates the currently shown details.
*
* @return false if details were rejected.
*/
private boolean maybeUpdateDetails(AutofillAssistantUiDelegate.Details newDetails) {
if (!mCurrentDetails.isSimilarTo(newDetails)) {
return false;
}
if (mCurrentDetails.isEmpty() && newDetails.isEmpty()) {
// No update on UI needed.
return true;
}
mCurrentDetails = AutofillAssistantUiDelegate.Details.merge(mCurrentDetails, newDetails);
mUiDelegateHolder.performUiOperation(uiDelegate -> uiDelegate.showDetails(mCurrentDetails));
return true;
}
@CalledByNative @CalledByNative
private void onHideDetails() { private void onHideDetails() {
mUiDelegateHolder.performUiOperation(AutofillAssistantUiDelegate::hideDetails); mUiDelegateHolder.performUiOperation(AutofillAssistantUiDelegate::hideDetails);
} }
@CalledByNative @CalledByNative
private void onShowDetails(String title, String url, String description, int year, int month, private boolean onShowDetails(String title, String url, String description, int year, int month,
int day, int hour, int minute, int second) { 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();
calendar.clear();
// Month in Java Date is 0-based, but the one we receive from the server is 1-based. // Month in Java Date is 0-based, but the one we receive from the server is 1-based.
calendar.set(year, month - 1, day, hour, minute, second); calendar.set(year, month - 1, day, hour, minute, second);
date = calendar.getTime(); date = calendar.getTime();
...@@ -357,9 +392,8 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -357,9 +392,8 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
date = null; date = null;
} }
mUiDelegateHolder.performUiOperation(uiDelegate return maybeUpdateDetails(new AutofillAssistantUiDelegate.Details(
-> uiDelegate.showDetails(new AutofillAssistantUiDelegate.Details( title, url, date, description, /* isFinal= */ true));
title, url, date, description, /* isFinal= */ true)));
} }
@CalledByNative @CalledByNative
......
...@@ -103,15 +103,10 @@ class AutofillAssistantUiDelegate { ...@@ -103,15 +103,10 @@ class AutofillAssistantUiDelegate {
private final boolean mIsRightToLeftLayout; private final boolean mIsRightToLeftLayout;
private ValueAnimator mDetailsPulseAnimation = null; private ValueAnimator mDetailsPulseAnimation = null;
@Nullable
private Details mDetails;
private String mStatusMessage;
private AutofillAssistantPaymentRequest mPaymentRequest; private AutofillAssistantPaymentRequest mPaymentRequest;
/** /**
* This is a client interface that relays interactions from the UI. * This is a client interface that relays interactions from the UI.
*
* Java version of the native autofill_assistant::UiDelegate.
*/ */
public interface Client extends TouchEventFilter.Client { public interface Client extends TouchEventFilter.Client {
/** /**
...@@ -145,6 +140,16 @@ class AutofillAssistantUiDelegate { ...@@ -145,6 +140,16 @@ class AutofillAssistantUiDelegate {
*/ */
void onCardSelected(String guid); void onCardSelected(String guid);
/**
* Returns current details.
*/
Details getDetails();
/**
* Returns current status message.
*/
String getStatusMessage();
/** /**
* Used to access information relevant for debugging purposes. * Used to access information relevant for debugging purposes.
* *
...@@ -203,6 +208,8 @@ class AutofillAssistantUiDelegate { ...@@ -203,6 +208,8 @@ class AutofillAssistantUiDelegate {
private final String mDescription; private final String mDescription;
private final boolean mIsFinal; private final boolean mIsFinal;
private static final Details EMPTY_DETAILS = new Details("", "", null, "", false);
public Details(String title, String url, @Nullable Date date, String description, public Details(String title, String url, @Nullable Date date, String description,
boolean isFinal) { boolean isFinal) {
this.mTitle = title; this.mTitle = title;
...@@ -250,6 +257,45 @@ class AutofillAssistantUiDelegate { ...@@ -250,6 +257,45 @@ class AutofillAssistantUiDelegate {
boolean isEmpty() { boolean isEmpty() {
return mTitle.isEmpty() && mUrl.isEmpty() && mDescription.isEmpty() && mDate == null; return mTitle.isEmpty() && mUrl.isEmpty() && mDescription.isEmpty() && mDate == null;
} }
/**
* Returns true {@code details} are similar to {@code this}. In order for details to be
* similar the conditions apply:
*
* <p>
* <ul>
* <li> Same date.
* <li> TODO(crbug.com/806868): 60% of characters match within title.
* </ul>
*/
boolean isSimilarTo(AutofillAssistantUiDelegate.Details details) {
if (this.isEmpty() || details.isEmpty()) {
return true;
}
return this.getDate().equals(details.getDate());
}
static Details getEmptyDetails() {
return EMPTY_DETAILS;
}
/**
* Merges {@code oldDetails} with the {@code newDetails} filling the missing fields. The
* distinction is important, as the fields from old version take precedence, with the
* exception of isFinal field.
*/
static Details merge(Details oldDetails, Details newDetails) {
String title =
oldDetails.getTitle().isEmpty() ? newDetails.getTitle() : oldDetails.getTitle();
String url = oldDetails.getUrl().isEmpty() ? newDetails.getUrl() : oldDetails.getUrl();
Date date = oldDetails.getDate() == null ? newDetails.getDate() : oldDetails.getDate();
String description = oldDetails.getDescription().isEmpty()
? newDetails.getDescription()
: oldDetails.getDescription();
boolean isFinal = newDetails.isFinal();
return new Details(title, url, date, description, isFinal);
}
} }
// Names borrowed from : // Names borrowed from :
...@@ -285,8 +331,8 @@ class AutofillAssistantUiDelegate { ...@@ -285,8 +331,8 @@ class AutofillAssistantUiDelegate {
-> HelpAndFeedback.getInstance(mActivity).showFeedback(mActivity, -> HelpAndFeedback.getInstance(mActivity).showFeedback(mActivity,
Profile.getLastUsedProfile(), mActivity.getActivityTab().getUrl(), Profile.getLastUsedProfile(), mActivity.getActivityTab().getUrl(),
FEEDBACK_CATEGORY_TAG, FEEDBACK_CATEGORY_TAG,
FeedbackContext.buildContextString( FeedbackContext.buildContextString(mActivity, mClient,
mActivity, mClient, mDetails, mStatusMessage, 4))); client.getDetails(), client.getStatusMessage(), 4)));
mCarouselScroll = mBottomBar.findViewById(R.id.carousel_scroll); mCarouselScroll = mBottomBar.findViewById(R.id.carousel_scroll);
mChipsViewContainer = mCarouselScroll.findViewById(R.id.carousel); mChipsViewContainer = mCarouselScroll.findViewById(R.id.carousel);
mStatusMessageView = mBottomBar.findViewById(R.id.status_message); mStatusMessageView = mBottomBar.findViewById(R.id.status_message);
...@@ -325,11 +371,7 @@ class AutofillAssistantUiDelegate { ...@@ -325,11 +371,7 @@ class AutofillAssistantUiDelegate {
* @param message Message to display. * @param message Message to display.
*/ */
public void showStatusMessage(@Nullable String message) { public void showStatusMessage(@Nullable String message) {
/// keep a copy of the most recent status message for feedback purposes
mStatusMessage = message;
show(); show();
mStatusMessageView.setText(message); mStatusMessageView.setText(message);
} }
...@@ -511,9 +553,6 @@ class AutofillAssistantUiDelegate { ...@@ -511,9 +553,6 @@ class AutofillAssistantUiDelegate {
/** Called to show contextual information. */ /** Called to show contextual information. */
public void showDetails(Details details) { public void showDetails(Details details) {
// keep a copy of most recent movie details for feedback purposes
mDetails = details;
Drawable defaultImage = AppCompatResources.getDrawable( Drawable defaultImage = AppCompatResources.getDrawable(
mActivity, R.drawable.autofill_assistant_default_details); mActivity, R.drawable.autofill_assistant_default_details);
......
...@@ -289,7 +289,7 @@ void UiControllerAndroid::HideDetails() { ...@@ -289,7 +289,7 @@ void UiControllerAndroid::HideDetails() {
AttachCurrentThread(), java_autofill_assistant_ui_controller_); AttachCurrentThread(), java_autofill_assistant_ui_controller_);
} }
void UiControllerAndroid::ShowDetails(const DetailsProto& details) { bool UiControllerAndroid::ShowDetails(const DetailsProto& details) {
int year = details.datetime().date().year(); int year = details.datetime().date().year();
int month = details.datetime().date().month(); int month = details.datetime().date().month();
int day = details.datetime().date().day(); int day = details.datetime().date().day();
...@@ -298,7 +298,7 @@ void UiControllerAndroid::ShowDetails(const DetailsProto& details) { ...@@ -298,7 +298,7 @@ void UiControllerAndroid::ShowDetails(const DetailsProto& details) {
int second = details.datetime().time().second(); int second = details.datetime().time().second();
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
Java_AutofillAssistantUiController_onShowDetails( return Java_AutofillAssistantUiController_onShowDetails(
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()),
......
...@@ -51,7 +51,7 @@ class UiControllerAndroid : public UiController, ...@@ -51,7 +51,7 @@ class UiControllerAndroid : public UiController,
const std::string& title, const std::string& title,
const std::vector<std::string>& supported_basic_card_networks) override; const std::vector<std::string>& supported_basic_card_networks) override;
void HideDetails() override; void HideDetails() override;
void ShowDetails(const DetailsProto& details) override; bool ShowDetails(const DetailsProto& details) override;
void ShowProgressBar(int progress, const std::string& message) override; void ShowProgressBar(int progress, const std::string& message) override;
void HideProgressBar() override; void HideProgressBar() override;
void UpdateTouchableArea(bool enabled, void UpdateTouchableArea(bool enabled,
......
...@@ -159,7 +159,7 @@ class ActionDelegate { ...@@ -159,7 +159,7 @@ class ActionDelegate {
virtual void HideDetails() = 0; virtual void HideDetails() = 0;
// Show contextual information. // Show contextual information.
virtual void ShowDetails(const DetailsProto& details) = 0; virtual bool ShowDetails(const DetailsProto& details) = 0;
// Show the progress bar with |message| and set it at |progress|%. // Show the progress bar with |message| and set it at |progress|%.
virtual void ShowProgressBar(int progress, const std::string& message) = 0; virtual void ShowProgressBar(int progress, const std::string& message) = 0;
......
...@@ -128,7 +128,7 @@ class MockActionDelegate : public ActionDelegate { ...@@ -128,7 +128,7 @@ class MockActionDelegate : public ActionDelegate {
MOCK_METHOD0(GetWebContents, content::WebContents*()); MOCK_METHOD0(GetWebContents, content::WebContents*());
MOCK_METHOD1(StopCurrentScriptAndShutdown, void(const std::string& message)); MOCK_METHOD1(StopCurrentScriptAndShutdown, void(const std::string& message));
MOCK_METHOD0(HideDetails, void()); MOCK_METHOD0(HideDetails, void());
MOCK_METHOD1(ShowDetails, void(const DetailsProto& details)); MOCK_METHOD1(ShowDetails, bool(const DetailsProto& details));
MOCK_METHOD2(ShowProgressBar, void(int progress, const std::string& message)); MOCK_METHOD2(ShowProgressBar, void(int progress, const std::string& message));
MOCK_METHOD0(HideProgressBar, void()); MOCK_METHOD0(HideProgressBar, void());
MOCK_METHOD0(ShowOverlay, void()); MOCK_METHOD0(ShowOverlay, void());
......
...@@ -21,12 +21,12 @@ void ShowDetailsAction::InternalProcessAction(ActionDelegate* delegate, ...@@ -21,12 +21,12 @@ void ShowDetailsAction::InternalProcessAction(ActionDelegate* delegate,
ProcessActionCallback callback) { ProcessActionCallback callback) {
if (!proto_.show_details().has_details()) { if (!proto_.show_details().has_details()) {
delegate->HideDetails(); delegate->HideDetails();
UpdateProcessedAction(ACTION_APPLIED);
std::move(callback).Run(std::move(processed_action_proto_));
} else { } else {
delegate->ShowDetails(proto_.show_details().details()); bool result = delegate->ShowDetails(proto_.show_details().details());
UpdateProcessedAction(result ? ACTION_APPLIED : OTHER_ACTION_STATUS);
std::move(callback).Run(std::move(processed_action_proto_));
} }
UpdateProcessedAction(ACTION_APPLIED);
std::move(callback).Run(std::move(processed_action_proto_));
} }
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -50,7 +50,7 @@ class MockUiController : public UiController { ...@@ -50,7 +50,7 @@ class MockUiController : public UiController {
const std::string& title, const std::string& title,
const std::vector<std::string>& supported_basic_card_networks)); const std::vector<std::string>& supported_basic_card_networks));
MOCK_METHOD0(HideDetails, void()); MOCK_METHOD0(HideDetails, void());
MOCK_METHOD1(ShowDetails, void(const DetailsProto& details)); MOCK_METHOD1(ShowDetails, bool(const DetailsProto& details));
MOCK_METHOD2(ShowProgressBar, void(int progress, const std::string& message)); MOCK_METHOD2(ShowProgressBar, void(int progress, const std::string& message));
MOCK_METHOD0(HideProgressBar, void()); MOCK_METHOD0(HideProgressBar, void());
MOCK_METHOD2(UpdateTouchableArea, MOCK_METHOD2(UpdateTouchableArea,
......
...@@ -232,8 +232,8 @@ void ScriptExecutor::HideDetails() { ...@@ -232,8 +232,8 @@ void ScriptExecutor::HideDetails() {
delegate_->GetUiController()->HideDetails(); delegate_->GetUiController()->HideDetails();
} }
void ScriptExecutor::ShowDetails(const DetailsProto& details) { bool ScriptExecutor::ShowDetails(const DetailsProto& details) {
delegate_->GetUiController()->ShowDetails(details); return delegate_->GetUiController()->ShowDetails(details);
} }
void ScriptExecutor::OnGetActions(bool result, const std::string& response) { void ScriptExecutor::OnGetActions(bool result, const std::string& response) {
......
...@@ -121,7 +121,7 @@ class ScriptExecutor : public ActionDelegate { ...@@ -121,7 +121,7 @@ class ScriptExecutor : public ActionDelegate {
content::WebContents* GetWebContents() override; content::WebContents* GetWebContents() override;
void StopCurrentScriptAndShutdown(const std::string& message) override; void StopCurrentScriptAndShutdown(const std::string& message) override;
void HideDetails() override; void HideDetails() override;
void ShowDetails(const DetailsProto& details) override; bool ShowDetails(const DetailsProto& details) override;
void ShowProgressBar(int progress, const std::string& message) override; void ShowProgressBar(int progress, const std::string& message) override;
void HideProgressBar() override; void HideProgressBar() override;
void ShowOverlay() override; void ShowOverlay() override;
......
...@@ -76,8 +76,11 @@ class UiController { ...@@ -76,8 +76,11 @@ class UiController {
// Hide contextual information. // Hide contextual information.
virtual void HideDetails() = 0; virtual void HideDetails() = 0;
// Show contextual information. // Show contextual information. Returns false if the contextual information is
virtual void ShowDetails(const DetailsProto& details) = 0; // not similar to the current one.
// TODO(806868): Pass details to the native side instead of comparing on the
// Java side.
virtual bool ShowDetails(const DetailsProto& details) = 0;
// Show the progress bar with |message| and set it at |progress|%. // Show the progress bar with |message| and set it at |progress|%.
virtual void ShowProgressBar(int progress, const std::string& message) = 0; virtual void ShowProgressBar(int progress, const std::string& message) = 0;
......
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