Commit d3ceb0a7 authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[PwdCheckAndroid] Fix empty screen bug

In canary builds (and infrequent on Emulators or very new Android
versions), the LifeCycleObserver does not trigger even if the observer
is added properly.

Although the failure reason is unclear, the fix is simple: forward the
corresponding Fragment events through the already existing delegate
interface.

Bug: 1115124, 1092444
Change-Id: I2fa45d31eba7830fc2a73d6999a2864b6408eeed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352892
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarNatalie Chouinard <chouinard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797593}
parent 089bee64
......@@ -12,9 +12,7 @@ import android.view.MenuItem;
import androidx.annotation.VisibleForTesting;
import androidx.browser.customtabs.CustomTabsIntent;
import androidx.lifecycle.Lifecycle;
import androidx.lifecycle.LifecycleObserver;
import androidx.lifecycle.OnLifecycleEvent;
import org.chromium.base.IntentUtils;
import org.chromium.chrome.browser.IntentHandler;
......@@ -77,11 +75,14 @@ class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObs
mFragmentView = fragmentView;
// TODO(crbug.com/1101256): If help is part of the view, make mediator the delegate.
mFragmentView.setComponentDelegate(this);
mFragmentView.getLifecycle().addObserver(this);
// TODO(crbug.com/1092444): Ideally, the following replaces the lifecycle event forwarding.
// Figure out why it isn't working and use the following lifecycle observer once it does:
// mFragmentView.getLifecycle().addObserver(this);
}
@OnLifecycleEvent(Lifecycle.Event.ON_START)
public void connectToModelWhenViewIsReady() {
@Override
public void onStartFragment() {
// In the rare case of a restarted activity, don't recreate the model and mediator.
if (mModel == null) {
mModel = PasswordCheckProperties.createDefaultModel();
......@@ -91,10 +92,14 @@ class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObs
}
}
@OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
public void stopCheck() {
@Override
public void onDestroyFragment() {
PasswordCheck check = PasswordCheckFactory.getPasswordCheckInstance();
if (check != null) check.stopCheck();
if (mFragmentView.getActivity() == null || mFragmentView.getActivity().isFinishing()) {
mMediator.destroy();
mModel = null;
}
}
// TODO(crbug.com/1101256): Move to view code.
......@@ -112,7 +117,6 @@ class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObs
@Override
public void destroy() {
mMediator.destroy();
PasswordCheckFactory.destroy();
}
......
......@@ -27,6 +27,16 @@ interface PasswordCheckComponentUi {
*/
boolean handleHelp(MenuItem item);
/**
* Forwards the signal that the fragment was started.
*/
void onStartFragment();
/**
* Forwards the signal that the fragment is being destroyed.
*/
void onDestroyFragment();
/**
* Tears down the component when it's no longer needed.
*/
......
......@@ -38,6 +38,12 @@ public class PasswordCheckFragmentView extends PreferenceFragmentCompat {
mPasswordCheckReferrer = getReferrerFromInstanceStateOrLaunchBundle(savedInstanceState);
}
@Override
public void onStart() {
super.onStart();
mComponentDelegate.onStartFragment();
}
private @PasswordCheckReferrer int getReferrerFromInstanceStateOrLaunchBundle(
Bundle savedInstanceState) {
if (savedInstanceState != null && savedInstanceState.containsKey(PASSWORD_CHECK_REFERRER)) {
......@@ -56,6 +62,7 @@ public class PasswordCheckFragmentView extends PreferenceFragmentCompat {
// The component should only be destroyed when the activity has been closed by the user
// (e.g. by pressing on the back button) and not when the activity is temporarily destroyed
// by the system.
mComponentDelegate.onDestroyFragment();
if (getActivity().isFinishing()
&& mPasswordCheckReferrer == PasswordCheckReferrer.LEAK_DIALOG) {
mComponentDelegate.destroy();
......
......@@ -15,9 +15,7 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.verify;
......@@ -43,6 +41,7 @@ import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.R
import static org.chromium.content_public.browser.test.util.CriteriaHelper.pollUiThread;
import static org.chromium.content_public.browser.test.util.TestThreadUtils.runOnUiThreadBlocking;
import android.content.DialogInterface;
import android.content.res.Resources;
import android.graphics.Bitmap;
import android.graphics.Canvas;
......@@ -56,7 +55,6 @@ import android.widget.ProgressBar;
import android.widget.TextView;
import androidx.annotation.IdRes;
import androidx.appcompat.app.AlertDialog;
import androidx.appcompat.content.res.AppCompatResources;
import androidx.recyclerview.widget.RecyclerView;
import androidx.test.filters.MediumTest;
......@@ -85,6 +83,8 @@ import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.widget.ButtonCompat;
import org.chromium.url.GURL;
import java.util.concurrent.atomic.AtomicInteger;
/**
* View tests for the Password Check component ensure model changes are reflected in the check UI.
*/
......@@ -512,17 +512,25 @@ public class PasswordCheckViewTest {
@Test
@MediumTest
public void testConfirmingDeletionDialogTriggersHandler() {
PasswordCheckDeletionDialogFragment.Handler mockHandler =
mock(PasswordCheckDeletionDialogFragment.Handler.class);
final AtomicInteger recordedConfirmation = new AtomicInteger(0);
PasswordCheckDeletionDialogFragment.Handler fakeHandler =
new PasswordCheckDeletionDialogFragment.Handler() {
@Override
public void onDismiss() {}
@Override
public void onClick(DialogInterface dialogInterface, int i) {
recordedConfirmation.incrementAndGet();
}
};
mModel.set(DELETION_ORIGIN, ANA.getDisplayOrigin());
runOnUiThreadBlocking(() -> mModel.set(DELETION_CONFIRMATION_HANDLER, mockHandler));
runOnUiThreadBlocking(() -> mModel.set(DELETION_CONFIRMATION_HANDLER, fakeHandler));
onView(withText(R.string.password_check_delete_credential_dialog_confirm))
.inRoot(withDecorView(
not(is(mPasswordCheckView.getActivity().getWindow().getDecorView()))))
.perform(click());
verify(mockHandler).onClick(any(), eq(AlertDialog.BUTTON_POSITIVE));
assertThat(recordedConfirmation.get(), is(1));
}
private MVCListAdapter.ListItem buildHeader(@PasswordCheckUIStatus int status,
......
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