Commit 20af6379 authored by Ivana Zuzic's avatar Ivana Zuzic Committed by Commit Bot

Revert "[PWD Editing Android] Save changes through PasswordEditingDelegateProvider"

This reverts commit 6c35d28d.

Reason for revert: 
If there's multiple credentials in the password list and one is edited, the rest of the credentials disappear and only the one that was edited stays in the list. That shouldn't be happening and needs to be corrected before landing this CL.

Original change's description:
> [PWD Editing Android] Save changes through PasswordEditingDelegateProvider
> 
> PasswordEntryEditor saves changes for a credential record when the
> Save button is clicked through PasswordEditingDelegateProvider.
> 
> Bug: 377410
> Change-Id: I200e4702945212b847488835a04fad3e5581b139
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807308
> Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
> Commit-Queue: Ivana Zuzic <izuzic@google.com>
> Cr-Commit-Position: refs/heads/master@{#698945}

TBR=fhorschig@chromium.org,izuzic@google.com

Change-Id: I5ce2e43b226ed47b49dfc069e0e9100b8598567b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 377410
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1820997Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Ivana Zuzic <izuzic@google.com>
Cr-Commit-Position: refs/heads/master@{#699259}
parent 4e99609d
...@@ -134,10 +134,7 @@ public class PasswordEntryEditor extends Fragment { ...@@ -134,10 +134,7 @@ public class PasswordEntryEditor extends Fragment {
mPasswordLabel.setError(getContext().getString( mPasswordLabel.setError(getContext().getString(
R.string.pref_edit_dialog_field_required_validation_message)); R.string.pref_edit_dialog_field_required_validation_message));
} else { } else {
PasswordEditingDelegateProvider.getInstance() // TODO(crbug.com/377410): Save the changes if everything was ok.
.getPasswordEditingDelegate()
.editSavedPasswordEntry(
mUsernameText.getText().toString(), mPasswordText.getText().toString());
getActivity().finish(); getActivity().finish();
} }
} }
...@@ -147,10 +144,4 @@ public class PasswordEntryEditor extends Fragment { ...@@ -147,10 +144,4 @@ public class PasswordEntryEditor extends Fragment {
super.onSaveInstanceState(savedInstanceState); super.onSaveInstanceState(savedInstanceState);
savedInstanceState.putBoolean(VIEW_BUTTON_PRESSED, mViewButtonPressed); savedInstanceState.putBoolean(VIEW_BUTTON_PRESSED, mViewButtonPressed);
} }
@Override
public void onDestroy() {
super.onDestroy();
PasswordEditingDelegateProvider.getInstance().getPasswordEditingDelegate().destroy();
}
} }
\ No newline at end of file
...@@ -199,9 +199,6 @@ public class PasswordEntryViewer ...@@ -199,9 +199,6 @@ public class PasswordEntryViewer
if (mCopyButtonPressed) copyPassword(); if (mCopyButtonPressed) copyPassword();
} }
PasswordManagerHandlerProvider.getInstance().addObserver(this); PasswordManagerHandlerProvider.getInstance().addObserver(this);
PasswordManagerHandlerProvider.getInstance()
.getPasswordManagerHandler()
.updatePasswordLists();
} }
@Override @Override
...@@ -437,13 +434,13 @@ public class PasswordEntryViewer ...@@ -437,13 +434,13 @@ public class PasswordEntryViewer
@Override @Override
public void passwordListAvailable(int count) { public void passwordListAvailable(int count) {
if (mException) return; if (mException) return;
TextView passwordTextView = mView.findViewById(R.id.password_entry_viewer_password); TextView passwordsLinkTextView = mView.findViewById(R.id.passwords_link);
SavedPasswordEntry SavedPasswordEntry = PasswordManagerHandlerProvider.getInstance() SavedPasswordEntry SavedPasswordEntry = PasswordManagerHandlerProvider.getInstance()
.getPasswordManagerHandler() .getPasswordManagerHandler()
.getSavedPasswordEntry(mID); .getSavedPasswordEntry(mID);
setRowText(R.id.url_row, SavedPasswordEntry.getUrl()); setRowText(R.id.url_row, SavedPasswordEntry.getUrl());
setRowText(R.id.username_row, SavedPasswordEntry.getUserName()); setRowText(R.id.username_row, SavedPasswordEntry.getUserName());
passwordTextView.setText(SavedPasswordEntry.getPassword()); passwordsLinkTextView.setText(SavedPasswordEntry.getPassword());
} }
@Override @Override
......
...@@ -37,8 +37,6 @@ import static org.hamcrest.Matchers.is; ...@@ -37,8 +37,6 @@ import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance; 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_GONE;
import static org.chromium.chrome.test.util.ViewUtils.VIEW_INVISIBLE; import static org.chromium.chrome.test.util.ViewUtils.VIEW_INVISIBLE;
...@@ -76,19 +74,17 @@ import org.hamcrest.Description; ...@@ -76,19 +74,17 @@ import org.hamcrest.Description;
import org.hamcrest.Matcher; import org.hamcrest.Matcher;
import org.junit.After; import org.junit.After;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.TestRule; import org.junit.rules.TestRule;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.CollectionUtil; import org.chromium.base.CollectionUtil;
import org.chromium.base.IntStringCallback; import org.chromium.base.IntStringCallback;
import org.chromium.base.test.BaseJUnit4ClassRunner; import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.ScalableTimeout;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.history.HistoryActivity; import org.chromium.chrome.browser.history.HistoryActivity;
...@@ -105,7 +101,6 @@ import org.chromium.chrome.test.ChromeBrowserTestRule; ...@@ -105,7 +101,6 @@ import org.chromium.chrome.test.ChromeBrowserTestRule;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.signin.ChromeSigninController; import org.chromium.components.signin.ChromeSigninController;
import org.chromium.components.sync.ModelType; import org.chromium.components.sync.ModelType;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils; import org.chromium.content_public.browser.test.util.TestThreadUtils;
import java.io.File; import java.io.File;
...@@ -124,9 +119,6 @@ import java.util.concurrent.atomic.AtomicReference; ...@@ -124,9 +119,6 @@ import java.util.concurrent.atomic.AtomicReference;
@RunWith(BaseJUnit4ClassRunner.class) @RunWith(BaseJUnit4ClassRunner.class)
public class SavePasswordsPreferencesTest { public class SavePasswordsPreferencesTest {
private static final long UI_UPDATING_TIMEOUT_MS = 3000; private static final long UI_UPDATING_TIMEOUT_MS = 3000;
@Mock
private PasswordEditingDelegate mMockPasswordEditingDelegate;
@Rule @Rule
public final ChromeBrowserTestRule mBrowserTestRule = new ChromeBrowserTestRule(); public final ChromeBrowserTestRule mBrowserTestRule = new ChromeBrowserTestRule();
...@@ -233,15 +225,6 @@ public class SavePasswordsPreferencesTest { ...@@ -233,15 +225,6 @@ public class SavePasswordsPreferencesTest {
@Override @Override
public void showPasswordEntryEditingView(Context context, int index) { public void showPasswordEntryEditingView(Context context, int index) {
mLastEntryIndex = index; mLastEntryIndex = index;
Bundle fragmentArgs = new Bundle();
fragmentArgs.putString(
PasswordEntryEditor.CREDENTIAL_URL, getSavedPasswordEntry(index).getUrl());
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_NAME,
getSavedPasswordEntry(index).getUserName());
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_PASSWORD,
getSavedPasswordEntry(index).getPassword());
PreferencesLauncher.launchSettingsPage(
context, PasswordEntryEditor.class, fragmentArgs);
} }
} }
...@@ -273,10 +256,6 @@ public class SavePasswordsPreferencesTest { ...@@ -273,10 +256,6 @@ public class SavePasswordsPreferencesTest {
*/ */
private final ManualCallbackDelayer mManualDelayer = new ManualCallbackDelayer(); private final ManualCallbackDelayer mManualDelayer = new ManualCallbackDelayer();
public SavePasswordsPreferencesTest() {
MockitoAnnotations.initMocks(this);
}
private void overrideProfileSyncService( private void overrideProfileSyncService(
final boolean usingPassphrase, final boolean syncingPasswords) { final boolean usingPassphrase, final boolean syncingPasswords) {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
...@@ -744,8 +723,6 @@ public class SavePasswordsPreferencesTest { ...@@ -744,8 +723,6 @@ public class SavePasswordsPreferencesTest {
@Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID) @Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID)
public void testSelectedStoredPasswordIndexIsSameAsInShowPasswordEntryEditingView() public void testSelectedStoredPasswordIndexIsSameAsInShowPasswordEntryEditingView()
throws Exception { throws Exception {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
setPasswordSourceWithMultipleEntries( // Initialize preferences setPasswordSourceWithMultipleEntries( // Initialize preferences
new SavedPasswordEntry[] {new SavedPasswordEntry("https://example.com", new SavedPasswordEntry[] {new SavedPasswordEntry("https://example.com",
"example user", "example password"), "example user", "example password"),
...@@ -769,8 +746,6 @@ public class SavePasswordsPreferencesTest { ...@@ -769,8 +746,6 @@ public class SavePasswordsPreferencesTest {
@Feature({"Preferences"}) @Feature({"Preferences"})
@Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID) @Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID)
public void testPasswordDataDisplayedInEditingActivity() throws Exception { public void testPasswordDataDisplayedInEditingActivity() throws Exception {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
Bundle fragmentArgs = new Bundle(); Bundle fragmentArgs = new Bundle();
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_URL, "https://example.com"); fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_URL, "https://example.com");
fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_NAME, "test user"); fragmentArgs.putString(PasswordEntryEditor.CREDENTIAL_NAME, "test user");
...@@ -784,16 +759,17 @@ public class SavePasswordsPreferencesTest { ...@@ -784,16 +759,17 @@ public class SavePasswordsPreferencesTest {
} }
/** /**
* Check that the password editing method from the PasswordEditingDelegate was called when the * Check that the changes of password data in the password editing activity are preserved and
* save button in the password editing activity was clicked. * shown in the password viewing activity and in the list of passwords after the save button
* was clicked.
*/ */
// TODO(izuzic): Remove @Ignore when saving of changes is enabled again.
@Ignore("The test is ignored because saving the changes of credentials is currently disabled.")
@Test @Test
@SmallTest @SmallTest
@Feature({"Preferences"}) @Feature({"Preferences"})
@Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID) @Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID)
public void testPasswordEditingMethodWasCalled() throws Exception { public void testChangeOfStoredPasswordDataIsPreserved() throws Exception {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
setPasswordSource(new SavedPasswordEntry("https://example.com", "test user", "password")); setPasswordSource(new SavedPasswordEntry("https://example.com", "test user", "password"));
PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(), PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(),
...@@ -807,38 +783,6 @@ public class SavePasswordsPreferencesTest { ...@@ -807,38 +783,6 @@ public class SavePasswordsPreferencesTest {
Espresso.onView(withSaveMenuIdOrText()).perform(click()); 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.
*/
@Test
@SmallTest
@Feature({"Preferences"})
@Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID)
public void testChangeOfStoredPasswordDataIsPropagated() throws Exception {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
setPasswordSource(new SavedPasswordEntry("https://example.com", "test user", "password"));
PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(),
SavePasswordsPreferences.class.getName());
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. // Check if the password viewing activity has the updated data.
Espresso.onView(withText("test user new")).check(matches(isDisplayed())); Espresso.onView(withText("test user new")).check(matches(isDisplayed()));
...@@ -856,8 +800,6 @@ public class SavePasswordsPreferencesTest { ...@@ -856,8 +800,6 @@ public class SavePasswordsPreferencesTest {
@Feature({"Preferences"}) @Feature({"Preferences"})
@Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID) @Features.EnableFeatures(ChromeFeatureList.PASSWORD_EDITING_ANDROID)
public void testStoredPasswordCanBeUnmaskedAndMaskedAgain() throws Exception { public void testStoredPasswordCanBeUnmaskedAndMaskedAgain() throws Exception {
PasswordEditingDelegateProvider.getInstance().setPasswordEditingDelegate(
mMockPasswordEditingDelegate);
Bundle fragmentArgs = new Bundle(); Bundle fragmentArgs = new Bundle();
fragmentArgs.putString(SavePasswordsPreferences.PASSWORD_LIST_NAME, "test user"); fragmentArgs.putString(SavePasswordsPreferences.PASSWORD_LIST_NAME, "test user");
fragmentArgs.putString(SavePasswordsPreferences.PASSWORD_LIST_URL, "https://example.com"); fragmentArgs.putString(SavePasswordsPreferences.PASSWORD_LIST_URL, "https://example.com");
...@@ -2195,9 +2137,4 @@ public class SavePasswordsPreferencesTest { ...@@ -2195,9 +2137,4 @@ public class SavePasswordsPreferencesTest {
allOf(withId(R.id.search_src_text), withText("Zeu")))); allOf(withId(R.id.search_src_text), withText("Zeu"))));
Espresso.onView(withId(R.id.search_src_text)).check(matches(withText("Zeu"))); Espresso.onView(withId(R.id.search_src_text)).check(matches(withText("Zeu")));
} }
PasswordEditingDelegate waitForEvent() {
return verify(mMockPasswordEditingDelegate,
timeout(ScalableTimeout.scaleTimeout(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL)));
}
} }
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