Commit df516d54 authored by Boris Sazonov's avatar Boris Sazonov Committed by Commit Bot

[Signin][Android] Fix race condition and NPE in AccountSigninView

This CL fixes an unlikely race condition and a NullPointerException
that were happening in AccountSigninView.recordConsent because of
simultaneous unsynchronized accesses to mSelectedAccountName field.
This CL also adds conditions to ignore taps on confirmation button and
Settings link if there's no account selected. This is necessary
because UI events are delivered asynchronously.

Bug: 891508
Change-Id: I366122a927e2c0b4683cfc85eb461cbfd6485b30
Reviewed-on: https://chromium-review.googlesource.com/c/1356802Reviewed-by: default avatarMartin Šrámek <msramek@chromium.org>
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612671}
parent f6e075c5
......@@ -342,12 +342,13 @@ public class AccountSigninView extends FrameLayout {
// The clickable "Settings" link.
NoUnderlineClickableSpan settingsSpan = new NoUnderlineClickableSpan((widget) -> {
if (mSelectedAccountName == null) return;
mListener.onAccountSelected(mSelectedAccountName, mIsDefaultAccountSelected, true);
RecordUserAction.record("Signin_Signin_WithAdvancedSyncSettings");
// Record the fact that the user consented to the consent text by clicking
// on |mSigninSettingsControl|.
recordConsent((TextView) widget);
recordConsent((TextView) widget, mSelectedAccountName);
});
mConsentTextTracker.setText(mSigninSettingsControl,
getSettingsControlDescription(mChildAccountStatus), input -> {
......@@ -749,12 +750,14 @@ public class AccountSigninView extends FrameLayout {
private void setUpConfirmButton() {
mConsentTextTracker.setText(mPositiveButton, R.string.signin_accept);
mPositiveButton.setOnClickListener(view -> {
if (mSelectedAccountName == null) return;
mListener.onAccountSelected(mSelectedAccountName, mIsDefaultAccountSelected, false);
RecordUserAction.record("Signin_Signin_WithDefaultSyncSettings");
// Record the fact that the user consented to the consent text by clicking
// on |mPositiveButton|.
recordConsent((TextView) view);
recordConsent((TextView) view, mSelectedAccountName);
});
setUpMoreButtonVisible(true);
}
......@@ -802,13 +805,14 @@ public class AccountSigninView extends FrameLayout {
* Records the Sync consent.
* @param confirmationView The view that the user clicked when consenting.
*/
private void recordConsent(TextView confirmationView) {
private void recordConsent(TextView confirmationView, String selectedAccountName) {
// TODO(crbug.com/831257): Provide the account id synchronously from AccountManagerFacade.
assert selectedAccountName != null;
final AccountIdProvider accountIdProvider = AccountIdProvider.getInstance();
new AsyncTask<String>() {
@Override
public String doInBackground() {
return accountIdProvider.getAccountId(mSelectedAccountName);
return accountIdProvider.getAccountId(selectedAccountName);
}
@Override
......
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