Commit 9a17da72 authored by Ioana Pandele's avatar Ioana Pandele Committed by Commit Bot

[PwdEditAndroid] Replace entry viewer fragment with editor

The editor should now be launched directly by tapping on the password
entry in Settings > Password.

This CL also separates the editing-specific tests from
PasswordSettingsTest.java. Only integration tests that actually rely on
PasswordSettings are left there.

Bug: 1141409, 1122310
Change-Id: Ie8d742d64e40b9881b35bf24a333d59c9b4cef02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2494922
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821413}
parent 172b7d28
......@@ -366,6 +366,7 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerHomepageUnitTest.java",
"javatests/src/org/chromium/chrome/browser/password_manager/PasswordGenerationDialogTest.java",
"javatests/src/org/chromium/chrome/browser/password_manager/PasswordManagerDialogTest.java",
"javatests/src/org/chromium/chrome/browser/password_manager/settings/PasswordEntryEditorTest.java",
"javatests/src/org/chromium/chrome/browser/password_manager/settings/PasswordSettingsTest.java",
"javatests/src/org/chromium/chrome/browser/password_manager/settings/PasswordViewingTypeTest.java",
"javatests/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinderTest.java",
......
......@@ -15,9 +15,7 @@
android:paddingLeft="@dimen/password_entry_editor_content_spacing"
android:paddingRight="@dimen/password_entry_editor_content_spacing">
<!-- Should not have autofillHints. See: https://crbug.com/1073966#c2 -->
<!-- Site -->
<!-- Site/App -->
<com.google.android.material.textfield.TextInputLayout
android:id="@+id/site_label"
android:labelFor="@+id/site_edit"
......@@ -27,11 +25,12 @@
android:layout_marginBottom="@dimen/password_entry_editor_field_bottom_margin">
<EditText
tools:ignore="Autofill,LabelFor"
tools:ignore="LabelFor"
android:id="@+id/site_edit"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:imeOptions="flagNoExtractUi"
android:importantForAutofill="noExcludeDescendants"
android:inputType="text"
android:hint="@string/password_entry_viewer_site_title"
android:enabled="false"/>
......@@ -47,11 +46,12 @@
android:layout_marginBottom="@dimen/password_entry_editor_field_bottom_margin">
<EditText
tools:ignore="Autofill,LabelFor"
tools:ignore="LabelFor"
android:id="@+id/username_edit"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:imeOptions="flagNoExtractUi"
android:importantForAutofill="noExcludeDescendants"
android:inputType="text"
android:hint="@string/password_entry_viewer_username_title"/>
</com.google.android.material.textfield.TextInputLayout>
......@@ -73,11 +73,12 @@
android:layout_marginBottom="@dimen/password_entry_editor_field_bottom_margin">
<EditText
tools:ignore="Autofill,LabelFor"
tools:ignore="LabelFor"
android:id="@+id/password_edit"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:imeOptions="flagNoExtractUi"
android:importantForAutofill="noExcludeDescendants"
android:inputType="textPassword"
android:hint="@string/password_entry_viewer_password"/>
</com.google.android.material.textfield.TextInputLayout>
......
......@@ -6,14 +6,6 @@
<menu xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto" >
<item
android:id="@+id/action_edit_saved_password"
android:icon="@drawable/ic_edit_24dp"
app:iconTint="@color/default_icon_color"
android:contentDescription="@string/password_entry_viewer_edit_stored_password"
android:title="@string/password_entry_viewer_edit_stored_password_action_title"
app:showAsAction="ifRoom"/>
<item
android:id="@+id/action_delete_saved_password"
android:icon="@drawable/ic_delete_white_24dp"
......
......@@ -27,7 +27,6 @@ import org.chromium.ui.widget.Toast;
/**
* Password entry editor that allows editing passwords stored in Chrome.
*/
public class PasswordEntryEditor extends Fragment {
private EditText mSiteText;
private EditText mUsernameText;
......
......@@ -36,7 +36,6 @@ import androidx.fragment.app.Fragment;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.sync.AndroidSyncSettings;
import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.components.browser_ui.settings.SettingsUtils;
......@@ -223,10 +222,6 @@ public class PasswordEntryViewer
@Override
public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
inflater.inflate(R.menu.password_entry_viewer_action_bar_menu, menu);
menu.findItem(R.id.action_edit_saved_password)
.setVisible(
ChromeFeatureList.isEnabled(ChromeFeatureList.EDIT_PASSWORDS_IN_SETTINGS)
&& !mException);
}
@Override
......@@ -236,12 +231,6 @@ public class PasswordEntryViewer
removeItem();
return true;
}
if (id == R.id.action_edit_saved_password) {
PasswordManagerHandlerProvider.getInstance()
.getPasswordManagerHandler()
.showPasswordEntryEditingView(getContext(), mID);
return true;
}
return super.onOptionsItemSelected(item);
}
......
......@@ -453,6 +453,12 @@ public class PasswordSettings
Intent.ACTION_VIEW, Uri.parse(PasswordUIView.getAccountDashboardURL()));
intent.setPackage(getActivity().getPackageName());
getActivity().startActivity(intent);
} else if (ChromeFeatureList.isEnabled(ChromeFeatureList.EDIT_PASSWORDS_IN_SETTINGS)) {
// Launch preference activity with a PasswordEntryEditor fragment.
PasswordManagerHandlerProvider.getInstance()
.getPasswordManagerHandler()
.showPasswordEntryEditingView(getContext(),
preference.getExtras().getInt(PasswordSettings.PASSWORD_LIST_ID));
} else {
// Launch preference activity with PasswordEntryViewer fragment with
// intent extras specifying the object.
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.password_manager.settings;
import static android.text.InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD;
import static androidx.test.espresso.Espresso.onView;
import static androidx.test.espresso.action.ViewActions.click;
import static androidx.test.espresso.action.ViewActions.typeText;
import static androidx.test.espresso.assertion.ViewAssertions.matches;
import static androidx.test.espresso.matcher.ViewMatchers.withId;
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.mockito.Mockito.verify;
import static org.chromium.chrome.browser.password_manager.settings.PasswordSettingsTest.withSaveMenuIdOrText;
import static org.chromium.content_public.browser.test.util.CriteriaHelper.pollUiThread;
import android.os.Bundle;
import android.view.View;
import android.widget.EditText;
import androidx.test.espresso.matcher.BoundedMatcher;
import androidx.test.filters.SmallTest;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.settings.SettingsActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
/**
* View tests for the password entry editor screen.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
public class PasswordEntryEditorTest {
private static final String URL = "https://example.com";
private static final String USERNAME = "test user";
private static final String PASSWORD = "passw0rd";
@Mock
private PasswordEditingDelegate mMockPasswordEditingDelegate;
private PasswordEntryEditor mPasswordEntryEditor;
@Rule
public SettingsActivityTestRule<PasswordEntryEditor> mTestRule =
new SettingsActivityTestRule<>(PasswordEntryEditor.class);
@Before
public void setUp() throws InterruptedException {
MockitoAnnotations.initMocks(this);
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
launchEditor();
pollUiThread(() -> mPasswordEntryEditor != null);
}
/**
* Check that the password editing activity displays the data received through arguments.
*/
@Test
@SmallTest
public void testPasswordDataDisplayedInEditingActivity() {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
onView(withId(R.id.site_edit)).check(matches(withText(URL)));
onView(withId(R.id.username_edit)).check(matches(withText(USERNAME)));
onView(withId(R.id.password_edit)).check(matches(withText(PASSWORD)));
}
/**
* Check that the password editing method from the PasswordEditingDelegate was called when the
* save button in the password editing activity was clicked.
*/
@Test
@SmallTest
public void testPasswordEditingMethodWasCalled() throws Exception {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
onView(withId(R.id.username_edit)).perform(typeText(" new"));
onView(withSaveMenuIdOrText()).perform(click());
verify(mMockPasswordEditingDelegate).editSavedPasswordEntry(USERNAME + " new", PASSWORD);
}
/**
* Check that the stored password is visible after clicking the unmasking icon and invisible
* after another click.
*/
@Test
@SmallTest
public void testStoredPasswordCanBeUnmaskedAndMaskedAgain() {
ReauthenticationManager.setApiOverride(ReauthenticationManager.OverrideState.AVAILABLE);
ReauthenticationManager.setScreenLockSetUpOverride(
ReauthenticationManager.OverrideState.AVAILABLE);
ReauthenticationManager.recordLastReauth(
System.currentTimeMillis(), ReauthenticationManager.ReauthScope.ONE_AT_A_TIME);
onView(withId(R.id.password_entry_editor_view_password)).perform(click());
onView(withId(R.id.password_edit)).check(matches(isVisiblePasswordInput(true)));
onView(withId(R.id.password_entry_editor_view_password)).perform(click());
onView(withId(R.id.password_edit)).check(matches(isVisiblePasswordInput(false)));
}
private void launchEditor() {
Bundle fragmentArgs = new Bundle();
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_URL, URL);
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_NAME, USERNAME);
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_PASSWORD, PASSWORD);
mTestRule.startSettingsActivity(fragmentArgs);
mPasswordEntryEditor = mTestRule.getFragment();
}
/**
* Matches any {@link EditText} which has the content visibility matching to |shouldBeVisible|.
* @return The matcher checking the input type.
*/
private static Matcher<View> isVisiblePasswordInput(final boolean shouldBeVisible) {
return new BoundedMatcher<View, EditText>(EditText.class) {
@Override
public boolean matchesSafely(EditText editText) {
return ((editText.getInputType() & TYPE_TEXT_VARIATION_VISIBLE_PASSWORD)
== TYPE_TEXT_VARIATION_VISIBLE_PASSWORD)
== shouldBeVisible;
}
@Override
public void describeTo(Description description) {
if (shouldBeVisible) {
description.appendText("The content should be visible.");
} else {
description.appendText("The content should not be visible.");
}
}
};
}
}
......@@ -4,8 +4,6 @@
package org.chromium.chrome.browser.password_manager.settings;
import static android.text.InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD;
import static androidx.test.espresso.Espresso.openActionBarOverflowOrOptionsMenu;
import static androidx.test.espresso.action.ViewActions.click;
import static androidx.test.espresso.action.ViewActions.closeSoftKeyboard;
......@@ -39,8 +37,6 @@ import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.verify;
import static org.chromium.chrome.test.util.ViewUtils.VIEW_GONE;
import static org.chromium.chrome.test.util.ViewUtils.VIEW_INVISIBLE;
......@@ -59,7 +55,6 @@ import android.os.Bundle;
import android.support.test.InstrumentationRegistry;
import android.view.MenuItem;
import android.view.View;
import android.widget.EditText;
import android.widget.LinearLayout;
import androidx.annotation.IdRes;
......@@ -93,8 +88,6 @@ import org.chromium.base.CollectionUtil;
import org.chromium.base.IntStringCallback;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.FlakyTest;
import org.chromium.base.test.util.ScalableTimeout;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.history.HistoryActivity;
......@@ -153,9 +146,6 @@ public class PasswordSettingsTest {
public SettingsActivityTestRule<PasswordSettings> mSettingsActivityTestRule =
new SettingsActivityTestRule<>(PasswordSettings.class);
@Rule
public SettingsActivityTestRule<PasswordEntryEditor> mEditorActivityTestRule =
new SettingsActivityTestRule<>(PasswordEntryEditor.class);
@Mock
private PasswordCheck mPasswordCheck;
......@@ -389,30 +379,6 @@ public class PasswordSettingsTest {
.setPasswordManagerHandlerForTest(mHandler));
}
/**
* Matches any {@link EditText} which has the content visibility matching to |shouldBeVisible|.
* @return The matcher checking the input type.
*/
private static Matcher<View> isVisiblePasswordInput(final boolean shouldBeVisible) {
return new BoundedMatcher<View, EditText>(EditText.class) {
@Override
public boolean matchesSafely(EditText editText) {
return ((editText.getInputType() & TYPE_TEXT_VARIATION_VISIBLE_PASSWORD)
== TYPE_TEXT_VARIATION_VISIBLE_PASSWORD)
== shouldBeVisible;
}
@Override
public void describeTo(Description description) {
if (shouldBeVisible) {
description.appendText("The content should be visible.");
} else {
description.appendText("The content should not be visible.");
}
}
};
}
/**
* Looks for the icon by id. If it cannot be found, it's probably hidden in the overflow
* menu. In that case, open the menu and search for its title.
......@@ -438,15 +404,6 @@ public class PasswordSettingsTest {
return withMenuIdOrText(R.id.menu_id_search, R.string.search);
}
/**
* Looks for the edit saved password icon by id or by its title.
* @return Returns either the icon button or the menu option.
*/
public static Matcher<View> withEditMenuIdOrText() {
return withMenuIdOrText(R.id.action_edit_saved_password,
R.string.password_entry_viewer_edit_stored_password_action_title);
}
/**
* Looks for the save edited password icon by id or by its title.
* @return Returns either the icon button or the menu option.
......@@ -886,64 +843,9 @@ public class PasswordSettingsTest {
.perform(scrollToHolder(hasTextInViewHolder("test user")));
Espresso.onView(withText(containsString("test user"))).perform(click());
Espresso.onView(withEditMenuIdOrText()).perform(click());
Assert.assertEquals(mHandler.getLastEntryIndex(), 1);
}
/**
* Check that the password editing activity displays the data received through arguments.
*/
@Test
@SmallTest
@Feature({"Preferences"})
@EnableFeatures(ChromeFeatureList.EDIT_PASSWORDS_IN_SETTINGS)
public void testPasswordDataDisplayedInEditingActivity() {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
Bundle fragmentArgs = new Bundle();
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_URL, "https://example.com");
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_NAME, "test user");
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_PASSWORD, "test password");
mEditorActivityTestRule.startSettingsActivity(fragmentArgs);
Espresso.onView(withId(R.id.site_edit)).check(matches(withText("https://example.com")));
Espresso.onView(withId(R.id.username_edit)).check(matches(withText("test user")));
Espresso.onView(withId(R.id.password_edit)).check(matches(withText("test password")));
}
/**
* Check that the password editing method from the PasswordEditingDelegate was called when the
* save button in the password editing activity was clicked.
*/
@Test
@SmallTest
@Feature({"Preferences"})
@EnableFeatures(ChromeFeatureList.EDIT_PASSWORDS_IN_SETTINGS)
@DisabledTest(message = "crbug.com/1122310")
public void testPasswordEditingMethodWasCalled() throws Exception {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
setPasswordSource(new SavedPasswordEntry("https://example.com", "test user", "password"));
startPasswordSettingsFromMainSettings();
Espresso.onView(withId(R.id.recycler_view))
.perform(scrollToHolder(hasTextInViewHolder("test user")));
Espresso.onView(withText(containsString("test user"))).perform(click());
Espresso.onView(withEditMenuIdOrText()).perform(click());
Espresso.onView(withId(R.id.username_edit)).perform(typeText(" new"));
Espresso.onView(withSaveMenuIdOrText()).perform(click());
verify(mMockPasswordEditingDelegate).editSavedPasswordEntry("test user new", "password");
// Verify that the delegate was destroyed when the password editing activity finished.
waitForEvent().destroy();
}
/**
* Check that the changes of password data are shown in the password viewing activity and in the
* list of passwords after the save button was clicked.
......@@ -963,57 +865,18 @@ public class PasswordSettingsTest {
.perform(scrollToHolder(hasTextInViewHolder("test user")));
Espresso.onView(withText(containsString("test user"))).perform(click());
Espresso.onView(withEditMenuIdOrText()).perform(click());
// Performing a change of saved credentials.
mHandler.mSavedPasswords.set(
0, new SavedPasswordEntry("https://example.com", "test user new", "password"));
Espresso.onView(withSaveMenuIdOrText()).perform(click());
// Check if the password viewing activity has the updated data.
Espresso.onView(withText("test user new")).check(matches(isDisplayed()));
Espresso.pressBack();
// Check if the password preferences activity has the updated data in the list of passwords.
Espresso.onView(withId(R.id.recycler_view))
.perform(scrollToHolder(hasTextInViewHolder("test user new")));
Espresso.onView(withText("test user new")).check(matches(isDisplayed()));
}
/**
* Check that the stored password is visible after clicking the unmasking icon and invisible
* after another click.
*/
@Test
@SmallTest
@Feature({"Preferences"})
@EnableFeatures(ChromeFeatureList.EDIT_PASSWORDS_IN_SETTINGS)
public void testStoredPasswordCanBeUnmaskedAndMaskedAgain() {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
Bundle fragmentArgs = new Bundle();
fragmentArgs.putString(PasswordSettings.PASSWORD_LIST_NAME, "test user");
fragmentArgs.putString(PasswordSettings.PASSWORD_LIST_URL, "https://example.com");
fragmentArgs.putString(PasswordSettings.PASSWORD_LIST_PASSWORD, "test password");
mEditorActivityTestRule.startSettingsActivity(fragmentArgs);
ReauthenticationManager.setApiOverride(ReauthenticationManager.OverrideState.AVAILABLE);
ReauthenticationManager.setScreenLockSetUpOverride(
ReauthenticationManager.OverrideState.AVAILABLE);
ReauthenticationManager.recordLastReauth(
System.currentTimeMillis(), ReauthenticationManager.ReauthScope.BULK);
Espresso.onView(withId(R.id.password_entry_editor_view_password)).perform(click());
Espresso.onView(withId(R.id.password_edit)).check(matches(isVisiblePasswordInput(true)));
Espresso.onView(withId(R.id.password_entry_editor_view_password)).perform(click());
Espresso.onView(withId(R.id.password_edit)).check(matches(isVisiblePasswordInput(false)));
}
/**
* Check that if there are no saved passwords, the export menu item is disabled.
*/
......@@ -1846,26 +1709,6 @@ public class PasswordSettingsTest {
Espresso.onView(withId(R.id.menu_id_search)).check(matches(isDisplayed()));
}
/**
* Check that the icon for editing saved passwords is visible if the Feature is enabled.
*/
@Test
@SmallTest
@Feature({"Preferences"})
@EnableFeatures(ChromeFeatureList.EDIT_PASSWORDS_IN_SETTINGS)
@FlakyTest(message = "https://crbug.com/1139520")
public void testEditSavedPasswordIconVisibleInActionBarWithFeature() {
setPasswordSource( // Initialize preferences
new SavedPasswordEntry("https://example.com", "test user", "test password"));
startPasswordSettingsFromMainSettings();
Espresso.onView(withId(R.id.recycler_view))
.perform(scrollToHolder(hasTextInViewHolder("test user")));
Espresso.onView(withText(containsString("test user"))).perform(click());
Espresso.onView(withEditMenuIdOrText()).check(matches(isDisplayed()));
}
/**
* Check that the search item is visible if the Feature is enabled.
*/
......@@ -2263,11 +2106,6 @@ public class PasswordSettingsTest {
PasswordCheckFactory.destroy();
}
PasswordEditingDelegate waitForEvent() {
return verify(mMockPasswordEditingDelegate,
timeout(ScalableTimeout.scaleTimeout(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL)));
}
PrefService getPrefService() {
return UserPrefs.get(Profile.getLastUsedRegularProfile());
}
......
......@@ -597,9 +597,6 @@ CHAR-LIMIT guidelines:
<message name="IDS_PASSWORD_ENTRY_VIEWER_DELETE_STORED_PASSWORD" desc='Content description for the button that allows users to delete the stored password.'>
Delete stored password
</message>
<message name="IDS_PASSWORD_ENTRY_VIEWER_EDIT_STORED_PASSWORD" desc='Content description for the button that allows users to edit the stored password.'>
Edit stored password
</message>
<message name="IDS_PASSWORD_ENTRY_VIEWER_USERNAME_COPIED_INTO_CLIPBOARD" desc='Text that announces to the user that the username of a saved account has been copied into clipboard.'>
Username copied
</message>
......
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