[Autofill Assistant] Break the call from java to native when destroying native object

This CL also removed the unnecessary checks since destroy the Controller won't
change the raw pointer value in UiControllerAndroid.

Bug: 806868
Change-Id: Id23c0f73f2db236462e380e47b5836affefb5ef7
Reviewed-on: https://chromium-review.googlesource.com/c/1356121Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Commit-Queue: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613096}

chrome/android/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java

@@ -48,7 +48,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
     private String mStatusMessage;
 
     /** Native pointer to the UIController. */
-    private final long mUiControllerAndroid;
+    private long mUiControllerAndroid;
 
     private UiDelegateHolder mUiDelegateHolder;
 
@@ -89,10 +89,15 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
                 parameters.get(PARAMETER_USER_EMAIL), activity.getInitialIntent().getExtras());
     }
 
+    @CalledByNative
+    private void onNativeDestroy() {
+        mUiControllerAndroid = 0;
+    }
+
     public void init(UiDelegateHolder delegateHolder, Details details) {
         mUiDelegateHolder = delegateHolder;
         maybeUpdateDetails(details);
-        nativeStart(mUiControllerAndroid, mInitialUrl);
+        if (mUiControllerAndroid != 0) nativeStart(mUiControllerAndroid, mInitialUrl);
     }
 
     @Override
@@ -117,42 +122,45 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
 
     @Override
     public void scrollBy(float distanceXRatio, float distanceYRatio) {
+        if (mUiControllerAndroid != 0) {
             nativeScrollBy(mUiControllerAndroid, distanceXRatio, distanceYRatio);
         }
+    }
 
     @Override
     public void updateTouchableArea() {
-        nativeUpdateTouchableArea(mUiControllerAndroid);
+        if (mUiControllerAndroid != 0) nativeUpdateTouchableArea(mUiControllerAndroid);
     }
 
     @Override
     public void onScriptSelected(String scriptPath) {
-        nativeOnScriptSelected(mUiControllerAndroid, scriptPath);
+        if (mUiControllerAndroid != 0) nativeOnScriptSelected(mUiControllerAndroid, scriptPath);
     }
 
     @Override
     public void onChoice(byte[] serverPayload) {
-        nativeOnChoice(mUiControllerAndroid, serverPayload);
+        if (mUiControllerAndroid != 0) nativeOnChoice(mUiControllerAndroid, serverPayload);
     }
 
     @Override
     public void onAddressSelected(String guid) {
-        nativeOnAddressSelected(mUiControllerAndroid, guid);
+        if (mUiControllerAndroid != 0) nativeOnAddressSelected(mUiControllerAndroid, guid);
     }
 
     @Override
     public void onCardSelected(String guid) {
-        nativeOnCardSelected(mUiControllerAndroid, guid);
+        if (mUiControllerAndroid != 0) nativeOnCardSelected(mUiControllerAndroid, guid);
     }
 
     @Override
     public void onDetailsAcknowledged(Details displayedDetails, boolean canContinue) {
         mCurrentDetails = displayedDetails;
-        nativeOnShowDetails(mUiControllerAndroid, canContinue);
+        if (mUiControllerAndroid != 0) nativeOnShowDetails(mUiControllerAndroid, canContinue);
     }
 
     @Override
     public String getDebugContext() {
+        if (mUiControllerAndroid == 0) return "";
         return nativeOnRequestDebugContext(mUiControllerAndroid);
     }
 
@@ -167,7 +175,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
      * cause crashes.
      */
     void unsafeDestroy() {
-        nativeDestroy(mUiControllerAndroid);
+        if (mUiControllerAndroid != 0) nativeDestroy(mUiControllerAndroid);
     }
 
     @CalledByNative
@@ -286,6 +294,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
                     supportedBasicCardNetworks, defaultEmail, (selectedPaymentInformation -> {
                         uiDelegate.closePaymentRequest();
                         if (selectedPaymentInformation.succeed) {
+                            if (mUiControllerAndroid != 0) {
                                 nativeOnGetPaymentInformation(mUiControllerAndroid,
                                         selectedPaymentInformation.succeed,
                                         selectedPaymentInformation.card,
@@ -294,6 +303,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
                                         selectedPaymentInformation.payerPhone,
                                         selectedPaymentInformation.payerEmail,
                                         selectedPaymentInformation.isTermsAndConditionsAccepted);
+                            }
                         } else {
                             // A failed payment request flow indicates that the UI was either
                             // dismissed or the back button was clicked. In that case we gracefully
@@ -312,7 +322,9 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
     void maybeUpdateDetails(Details newDetails) {
         if (mCurrentDetails.isEmpty() && newDetails.isEmpty()) {
             // No update on UI needed.
+            if (mUiControllerAndroid != 0) {
                 nativeOnShowDetails(mUiControllerAndroid, /* canContinue= */ true);
+            }
             return;
         }
 
@@ -374,7 +386,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
             return;
         }
         if (mAccount == null) {
-            nativeOnAccessToken(mUiControllerAndroid, true, "");
+            if (mUiControllerAndroid != 0) nativeOnAccessToken(mUiControllerAndroid, true, "");
             return;
         }
 
@@ -382,12 +394,14 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
                 mAccount, AUTH_TOKEN_TYPE, new AccountManagerFacade.GetAuthTokenCallback() {
                     @Override
                     public void tokenAvailable(String token) {
+                        if (mUiControllerAndroid != 0) {
                             nativeOnAccessToken(mUiControllerAndroid, true, token);
                         }
+                    }
 
                     @Override
                     public void tokenUnavailable(boolean isTransientError) {
-                        if (!isTransientError) {
+                        if (!isTransientError && mUiControllerAndroid != 0) {
                             nativeOnAccessToken(mUiControllerAndroid, false, "");
                         }
                     }
@@ -406,6 +420,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
     /** Choose an account to authenticate as for making RPCs to the backend. */
     private void chooseAccountAsync(@Nullable String accountFromParameter, Bundle extras) {
         AccountManagerFacade.get().tryGetGoogleAccounts(accounts -> {
+            if (mUiControllerAndroid == 0) return;
             if (accounts.size() == 1) {
                 // If there's only one account, there aren't any doubts.
                 onAccountChosen(accounts.get(0));

chrome/browser/android/autofill_assistant/ui_controller_android.cc

@@ -97,21 +97,24 @@ UiControllerAndroid::UiControllerAndroid(
   DCHECK(ui_delegate_);
 }
 
-UiControllerAndroid::~UiControllerAndroid() {}
+UiControllerAndroid::~UiControllerAndroid() {
+  Java_AutofillAssistantUiController_onNativeDestroy(
+      AttachCurrentThread(), java_autofill_assistant_ui_controller_);
+}
 
 void UiControllerAndroid::Start(JNIEnv* env,
                                 const JavaParamRef<jobject>& jcaller,
                                 const JavaParamRef<jstring>& initialUrlString) {
-  if (!ui_delegate_)
-    return;
-
   GURL initialUrl =
       GURL(base::android::ConvertJavaStringToUTF8(env, initialUrlString));
   ui_delegate_->Start(initialUrl);
 }
 
+// This interface must be called before everything else in this class except the
+// constructor.
 void UiControllerAndroid::SetUiDelegate(UiDelegate* ui_delegate) {
   ui_delegate_ = ui_delegate;
+  DCHECK(ui_delegate_);
 }
 
 void UiControllerAndroid::ShowStatusMessage(const std::string& message) {
@@ -182,18 +185,12 @@ void UiControllerAndroid::ScrollBy(
     const base::android::JavaParamRef<jobject>& obj,
     float distanceX,
     float distanceY) {
-  if (!ui_delegate_)
-    return;
-
   ui_delegate_->ScrollBy(distanceX, distanceY);
 }
 
 void UiControllerAndroid::UpdateTouchableArea(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj) {
-  if (!ui_delegate_)
-    return;
-
   ui_delegate_->UpdateTouchableArea();
 }
 
@@ -201,9 +198,6 @@ void UiControllerAndroid::OnScriptSelected(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller,
     const JavaParamRef<jstring>& jscript_path) {
-  if (!ui_delegate_)
-    return;
-
   std::string script_path;
   base::android::ConvertJavaStringToUTF8(env, jscript_path, &script_path);
   ui_delegate_->OnScriptSelected(script_path);
@@ -459,9 +453,6 @@ void UiControllerAndroid::UpdateTouchableArea(bool enabled,
 }
 
 std::string UiControllerAndroid::GetDebugContext() const {
-  if (!ui_delegate_)
-    return "";
-
   return ui_delegate_->GetDebugContext();
 }
 
@@ -518,9 +509,6 @@ void UiControllerAndroid::InvalidateAccessToken(
 
 void UiControllerAndroid::Destroy(JNIEnv* env,
                                   const JavaParamRef<jobject>& obj) {
-  if (ui_delegate_ == nullptr)
-    return;
-
   if (!ui_delegate_->Terminate()) {
     // This is a safety net and should be removed once all uses of
     // base::Unretained in the execution and script tracking has been removed.

chrome/browser/android/autofill_assistant/ui_controller_android.h

@@ -132,8 +132,7 @@ class UiControllerAndroid : public UiController,
   base::android::ScopedJavaGlobalRef<jobject>
       java_autofill_assistant_ui_controller_;
 
-  // UI delegate. It can be nullptr during initialization and after the delegate
-  // has been deleted. Always check for nullptr before using it.
+  // UI delegate can be nullptr before SetUiDelegate.
   UiDelegate* ui_delegate_;
   content::BrowserContext* browser_context_;