Clean-up PasswordsImportM2 feature flag

Fixed: 1471617, 1417650 Change-Id: I34385c2f91328fedc54a814f8b67c2d52c4ad6a7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4980226 Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Auto-Submit: Andrii Natiahlyi <natiahlyi@google.com> Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/main@{#1215589}
chromium · Oct 26, 2023 · aa7b259 · aa7b259
1 parent 254f78b
commit aa7b259
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 279 deletions.
diff --git a/chrome/browser/resources/password_manager/passwords_importer.html b/chrome/browser/resources/password_manager/passwords_importer.html
@@ -222,12 +222,6 @@
         <iron-icon id="successIcon" icon="cr:check-circle"></iron-icon>
         <div id="description">[[successDescription_]]</div>
       </div>
-      <div id="tipBox" class="flex"
-          hidden="[[shouldHideTipBox_(results_)]]">
-         <iron-icon id="infoIcon" icon="cr:info-outline"></iron-icon>
-        <div id="successTip" inner-h-t-m-l="[[getSuccessTipHtml_(results_)]]">
-        </div>
-      </div>
       <cr-checkbox id="deleteFileOption"
         hidden="[[shouldHideDeleteFileOption_(results_)]]"
         inner-h-t-m-l="[[getCheckboxLabelHtml_(results_)]]">

diff --git a/chrome/browser/resources/password_manager/passwords_importer.ts b/chrome/browser/resources/password_manager/passwords_importer.ts
@@ -79,13 +79,6 @@ export class PasswordsImporterElement extends PasswordsImporterElementBase {
 
   static get properties() {
     return {
-      enablePasswordsImportM2_: {
-        type: Boolean,
-        value() {
-          return loadTimeData.getBoolean('enablePasswordsImportM2');
-        },
-      },
-
       dialogState_: Number,
 
       dialogStateEnum_: {
@@ -143,7 +136,6 @@ export class PasswordsImporterElement extends PasswordsImporterElementBase {
   isAccountStoreUser: boolean;
   accountEmail: string;
 
-  private enablePasswordsImportM2_: boolean;
   private dialogState_: DialogState = DialogState.NO_DIALOG;
   // Refers both to syncing users with sync enabled for passwords and account
   // store users who choose to import passwords to their account.
@@ -468,22 +460,9 @@ export class PasswordsImporterElement extends PasswordsImporterElementBase {
     return !this.isAccountStoreUser && !this.isState_(DialogState.IN_PROGRESS);
   }
 
-  private shouldHideTipBox_(): boolean {
-    // Tip box is only shown in "success" state if all passwords were imported.
-    // Only shown in Passwords Import M1.
-    if (this.enablePasswordsImportM2_) {
-      return true;
-    }
-    assert(this.results_);
-    return !!this.results_.displayedEntries.length;
-  }
-
   private shouldHideDeleteFileOption_(): boolean {
     // "Delete file" checkbox is only shown in "success" state if all passwords
     // were imported.
-    if (!this.enablePasswordsImportM2_) {
-      return true;
-    }
     assert(this.results_);
     return !!this.results_.displayedEntries.length;
   }

diff --git a/chrome/browser/ui/passwords/settings/password_manager_porter_unittest.cc b/chrome/browser/ui/passwords/settings/password_manager_porter_unittest.cc
@@ -347,9 +347,6 @@ TEST_F(PasswordManagerPorterTest, ImportDismissedOnCanceledFileSelection) {
 }
 
 TEST_F(PasswordManagerPorterTest, ContinueImportWithConflicts) {
-  base::test::ScopedFeatureList feature_list{
-      password_manager::features::kPasswordsImportM2};
-
   password_manager::PasswordForm test_form;
   test_form.url = GURL("https://test.com");
   test_form.signon_realm = test_form.url.spec();
@@ -395,9 +392,6 @@ TEST_F(PasswordManagerPorterTest, ContinueImportWithConflicts) {
 }
 
 TEST_F(PasswordManagerPorterTest, RejectNewImportsWhenConflictsNotResolved) {
-  base::test::ScopedFeatureList feature_list{
-      password_manager::features::kPasswordsImportM2};
-
   password_manager::PasswordForm test_form;
   test_form.url = GURL("https://test.com");
   test_form.signon_realm = test_form.url.spec();
@@ -441,9 +435,6 @@ TEST_F(PasswordManagerPorterTest, RejectNewImportsWhenConflictsNotResolved) {
 }
 
 TEST_F(PasswordManagerPorterTest, ResetImporterTriggersFileDeletion) {
-  base::test::ScopedFeatureList feature_list{
-      password_manager::features::kPasswordsImportM2};
-
   ASSERT_TRUE(base::WriteFile(temp_file_path(),
                               "origin,username,password\n"
                               "https://test.com,username,secret\n"));

diff --git a/chrome/browser/ui/webui/password_manager/password_manager_ui.cc b/chrome/browser/ui/webui/password_manager/password_manager_ui.cc
@@ -427,10 +427,6 @@ content::WebUIDataSource* CreateAndAddPasswordsUIHTMLSource(
       base::FeatureList::IsEnabled(device::kWebAuthnICloudKeychain));
 #endif
 
-  source->AddBoolean("enablePasswordsImportM2",
-                     base::FeatureList::IsEnabled(
-                         password_manager::features::kPasswordsImportM2));
-
   source->AddBoolean(
       "enableSendPasswords",
       base::FeatureList::IsEnabled(password_manager::features::kSendPasswords));

diff --git a/chrome/test/data/webui/password_manager/passwords_importer_test.ts b/chrome/test/data/webui/password_manager/passwords_importer_test.ts
@@ -204,52 +204,7 @@ suite('PasswordsImporterTest', function() {
     await triggerImportHelper(importer, passwordManager, expectedStore);
   });
 
-  test('M1: has correct success state with no errors', async function() {
-    loadTimeData.overrideValues({enablePasswordsImportM2: false});
-    const importer = createPasswordsImporter();
-    passwordManager.setImportResults({
-      status: chrome.passwordsPrivate.ImportResultsStatus.SUCCESS,
-      numberImported: 42,
-      displayedEntries: [],
-      fileName: 'test.csv',
-    });
-
-    await triggerImportHelper(importer, passwordManager);
-    await pluralString.whenCalled('getPluralString');
-    await flushTasks();
-
-    const dialog =
-        importer.shadowRoot!.querySelector<CrDialogElement>('#dialog');
-    assertTrue(!!dialog);
-    assertTrue(dialog.open);
-
-    assertVisibleTextContent(
-        dialog, '#title', importer.i18n('importPasswordsSuccessTitle'));
-
-    assertTrue(isChildVisible(dialog, '#tipBox', /*checkLightDom=*/ true));
-
-    assertFalse(
-        isChildVisible(dialog, '#deleteFileOption', /*checkLightDom=*/ true));
-    assertFalse(
-        isChildVisible(dialog, '#failuresSummary', /*checkLightDom=*/ true));
-
-    const successTip = dialog.querySelector('#successTip');
-    assertTrue(!!successTip);
-    assertEquals(
-        successTip.innerHTML.toString(),
-        importer
-            .i18nAdvanced(
-                'importPasswordsSuccessTip',
-                {attrs: ['class'], substitutions: ['test.csv']})
-            .toString());
-
-    assertVisibleTextContent(dialog, '#closeButton', importer.i18n('close'));
-
-    await closeDialogHelper(importer, passwordManager, dialog, '#closeButton');
-  });
-
-  test('M2: has correct success state with no errors', async function() {
-    loadTimeData.overrideValues({enablePasswordsImportM2: true});
+  test('Has correct success state with no errors', async function() {
     const importer = createPasswordsImporter();
     passwordManager.setImportResults({
       status: chrome.passwordsPrivate.ImportResultsStatus.SUCCESS,
@@ -293,7 +248,6 @@ suite('PasswordsImporterTest', function() {
   });
 
   test('has correct conflicts state', async function() {
-    loadTimeData.overrideValues({enablePasswordsImportM2: true});
     const importer = createPasswordsImporter();
     passwordManager.setImportResults({
       status: chrome.passwordsPrivate.ImportResultsStatus.CONFLICTS,
@@ -350,7 +304,6 @@ suite('PasswordsImporterTest', function() {
   });
 
   test('can skip conflicts', async function() {
-    loadTimeData.overrideValues({enablePasswordsImportM2: true});
     const importer = createPasswordsImporter();
     passwordManager.setImportResults({
       status: chrome.passwordsPrivate.ImportResultsStatus.CONFLICTS,
@@ -401,7 +354,6 @@ suite('PasswordsImporterTest', function() {
   });
 
   test('can continue import with conflicts', async function() {
-    loadTimeData.overrideValues({enablePasswordsImportM2: true});
     const importer = createPasswordsImporter();
     passwordManager.setImportResults({
       status: chrome.passwordsPrivate.ImportResultsStatus.CONFLICTS,
@@ -456,7 +408,6 @@ suite('PasswordsImporterTest', function() {
   });
 
   test('correct conflicts state after failed re-auth', async function() {
-    loadTimeData.overrideValues({enablePasswordsImportM2: true});
     const importer = createPasswordsImporter();
     passwordManager.setImportResults({
       status: chrome.passwordsPrivate.ImportResultsStatus.CONFLICTS,
@@ -526,9 +477,8 @@ suite('PasswordsImporterTest', function() {
   });
 
   test(
-      'M2: close button triggers file deletion with ticked checkbox',
+      'close button triggers file deletion with ticked checkbox',
       async function() {
-        loadTimeData.overrideValues({enablePasswordsImportM2: true});
         const importer = createPasswordsImporter();
         passwordManager.setImportResults({
           status: chrome.passwordsPrivate.ImportResultsStatus.SUCCESS,
@@ -558,9 +508,8 @@ suite('PasswordsImporterTest', function() {
       });
 
   test(
-      'M2: view passwords triggers file deletion with ticked checkbox',
+      'view passwords triggers file deletion with ticked checkbox',
       async function() {
-        loadTimeData.overrideValues({enablePasswordsImportM2: true});
         const importer = createPasswordsImporter();
         passwordManager.setImportResults({
           status: chrome.passwordsPrivate.ImportResultsStatus.SUCCESS,

diff --git a/components/password_manager/core/browser/import/password_importer.cc b/components/password_manager/core/browser/import/password_importer.cc
@@ -585,9 +585,7 @@ void PasswordImporter::ConsumePasswords(
   base::UmaHistogramCounts1M("PasswordManager.Import.PerFile.Duplicates",
                              duplicates_count);
 
-  if (!base::FeatureList::IsEnabled(
-          password_manager::features::kPasswordsImportM2) ||
-      conflicts.empty()) {
+  if (conflicts.empty()) {
     for (const std::vector<PasswordForm>& forms : conflicts) {
       results.displayed_entries.push_back(CreateFailedImportEntry(
           CredentialUIEntry(forms), GetConflictType(to_store)));
@@ -639,9 +637,7 @@ void PasswordImporter::ImportFinished(ImportResultsCallback results_callback,
                                       size_t conflicts_count) {
   ReportImportResultsMetrics(results, start_time, conflicts_count);
 
-  if (base::FeatureList::IsEnabled(
-          password_manager::features::kPasswordsImportM2) &&
-      results.displayed_entries.empty()) {
+  if (results.displayed_entries.empty()) {
     // After successful import with no errors, the user has an option to delete
     // the imported file.
     state_ = kFinished;