Commit cad67bd4 authored by Ioana Pandele's avatar Ioana Pandele Committed by Commit Bot

Fix export reset when completionWithItemsHandler is not called

Re-enable password export when the activity view disappears, instead of when completionWithItemsHandler is called. This has the potential to be a premature re-enabling, if the user decides to background the app launched from the activity view and come back to Chrome’s password settings. Re-triggering the export in this case might cause a conflict between multiple exported files.

To mitigate this issue, each exported file is stored in its own uniquely identified folder in tmp/passwords/. If completionWithItemsHandler is called, the file and its folder are removed. For the cases in which the completion handler is not called, the rest of the files are deleted at startup.


Bug: 820053
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I5683acfe8483876c0aeb81ce25ba83b24f109676
Reviewed-on: https://chromium-review.googlesource.com/974181
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546148}
parent bf6b8a35
...@@ -139,6 +139,7 @@ source_set("app_internal") { ...@@ -139,6 +139,7 @@ source_set("app_internal") {
"//components/keyed_service/ios", "//components/keyed_service/ios",
"//components/metrics", "//components/metrics",
"//components/ntp_snippets", "//components/ntp_snippets",
"//components/password_manager/core/common",
"//components/payments/core", "//components/payments/core",
"//components/prefs", "//components/prefs",
"//components/proxy_config", "//components/proxy_config",
...@@ -175,6 +176,7 @@ source_set("app_internal") { ...@@ -175,6 +176,7 @@ source_set("app_internal") {
"//ios/chrome/browser/net", "//ios/chrome/browser/net",
"//ios/chrome/browser/ntp_snippets", "//ios/chrome/browser/ntp_snippets",
"//ios/chrome/browser/omaha", "//ios/chrome/browser/omaha",
"//ios/chrome/browser/passwords",
"//ios/chrome/browser/payments", "//ios/chrome/browser/payments",
"//ios/chrome/browser/payments:constants", "//ios/chrome/browser/payments:constants",
"//ios/chrome/browser/prefs", "//ios/chrome/browser/prefs",
......
...@@ -13,6 +13,7 @@ include_rules = [ ...@@ -13,6 +13,7 @@ include_rules = [
"+components/history/core/browser", "+components/history/core/browser",
"+components/metrics", "+components/metrics",
"+components/ntp_snippets", "+components/ntp_snippets",
"+components/password_manager/core/common",
"+components/payments/core", "+components/payments/core",
"+components/prefs", "+components/prefs",
"+components/reading_list/core", "+components/reading_list/core",
......
...@@ -33,6 +33,7 @@ ...@@ -33,6 +33,7 @@
#include "components/metrics/metrics_pref_names.h" #include "components/metrics/metrics_pref_names.h"
#include "components/metrics/metrics_service.h" #include "components/metrics/metrics_service.h"
#include "components/ntp_snippets/content_suggestions_service.h" #include "components/ntp_snippets/content_suggestions_service.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "components/payments/core/features.h" #include "components/payments/core/features.h"
#include "components/prefs/pref_change_registrar.h" #include "components/prefs/pref_change_registrar.h"
#include "components/signin/core/browser/signin_manager.h" #include "components/signin/core/browser/signin_manager.h"
...@@ -86,6 +87,7 @@ ...@@ -86,6 +87,7 @@
#import "ios/chrome/browser/metrics/previous_session_info.h" #import "ios/chrome/browser/metrics/previous_session_info.h"
#import "ios/chrome/browser/net/cookie_util.h" #import "ios/chrome/browser/net/cookie_util.h"
#include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h" #include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
#import "ios/chrome/browser/passwords/passwords_directory_util.h"
#include "ios/chrome/browser/payments/ios_payment_instrument_launcher.h" #include "ios/chrome/browser/payments/ios_payment_instrument_launcher.h"
#include "ios/chrome/browser/payments/ios_payment_instrument_launcher_factory.h" #include "ios/chrome/browser/payments/ios_payment_instrument_launcher_factory.h"
#import "ios/chrome/browser/payments/payment_request_constants.h" #import "ios/chrome/browser/payments/payment_request_constants.h"
...@@ -197,6 +199,9 @@ NSString* const kCheckForFirstPartyApps = @"CheckForFirstPartyApps"; ...@@ -197,6 +199,9 @@ NSString* const kCheckForFirstPartyApps = @"CheckForFirstPartyApps";
// Constants for deferred deletion of leftover user downloaded files. // Constants for deferred deletion of leftover user downloaded files.
NSString* const kDeleteDownloads = @"DeleteDownloads"; NSString* const kDeleteDownloads = @"DeleteDownloads";
// Constants for deferred deletion of leftover temporary passwords files.
NSString* const kDeleteTempPasswords = @"DeleteTempPasswords";
// Constants for deferred sending of queued feedback. // Constants for deferred sending of queued feedback.
NSString* const kSendQueuedFeedback = @"SendQueuedFeedback"; NSString* const kSendQueuedFeedback = @"SendQueuedFeedback";
...@@ -502,6 +507,9 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData( ...@@ -502,6 +507,9 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
// Schedules the deletion of user downloaded files that might be leftover // Schedules the deletion of user downloaded files that might be leftover
// from the last time Chrome was run. // from the last time Chrome was run.
- (void)scheduleDeleteDownloadsDirectory; - (void)scheduleDeleteDownloadsDirectory;
// Schedule the deletion of the temporary passwords files that might
// be left over from incomplete export operations.
- (void)scheduleDeleteTempPasswordsDirectory;
// Returns whether or not the app can launch in incognito mode. // Returns whether or not the app can launch in incognito mode.
- (BOOL)canLaunchInIncognito; - (BOOL)canLaunchInIncognito;
// Determines which UI should be shown on startup, and shows it. // Determines which UI should be shown on startup, and shows it.
...@@ -1150,6 +1158,10 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData( ...@@ -1150,6 +1158,10 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
[self sendQueuedFeedback]; [self sendQueuedFeedback];
[self scheduleSpotlightResync]; [self scheduleSpotlightResync];
[self scheduleDeleteDownloadsDirectory]; [self scheduleDeleteDownloadsDirectory];
if (base::FeatureList::IsEnabled(
password_manager::features::kPasswordExport)) {
[self scheduleDeleteTempPasswordsDirectory];
}
[self scheduleStartupAttemptReset]; [self scheduleStartupAttemptReset];
[self startFreeMemoryMonitoring]; [self startFreeMemoryMonitoring];
[self scheduleAppDistributionPings]; [self scheduleAppDistributionPings];
...@@ -1177,6 +1189,14 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData( ...@@ -1177,6 +1189,14 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
}]; }];
} }
- (void)scheduleDeleteTempPasswordsDirectory {
[[DeferredInitializationRunner sharedInstance]
enqueueBlockNamed:kDeleteTempPasswords
block:^{
DeletePasswordsDirectory();
}];
}
- (void)scheduleSpotlightResync { - (void)scheduleSpotlightResync {
if (!_spotlightManager) { if (!_spotlightManager) {
return; return;
......
...@@ -42,6 +42,8 @@ source_set("passwords") { ...@@ -42,6 +42,8 @@ source_set("passwords") {
"password_manager_internals_service_factory.h", "password_manager_internals_service_factory.h",
"password_tab_helper.h", "password_tab_helper.h",
"password_tab_helper.mm", "password_tab_helper.mm",
"passwords_directory_util.h",
"passwords_directory_util.mm",
"update_password_infobar_controller.h", "update_password_infobar_controller.h",
"update_password_infobar_controller.mm", "update_password_infobar_controller.mm",
] ]
...@@ -122,6 +124,7 @@ source_set("unit_tests") { ...@@ -122,6 +124,7 @@ source_set("unit_tests") {
"js_credential_manager_unittest.mm", "js_credential_manager_unittest.mm",
"password_controller_js_unittest.mm", "password_controller_js_unittest.mm",
"password_controller_unittest.mm", "password_controller_unittest.mm",
"passwords_directory_util_unittest.mm",
"test_helpers.cc", "test_helpers.cc",
"test_helpers.h", "test_helpers.h",
] ]
......
// Copyright 2018 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.
#ifndef IOS_CHROME_BROWSER_PASSWORDS_PASSWORDS_DIRECTORY_UTIL_H_
#define IOS_CHROME_BROWSER_PASSWORDS_PASSWORDS_DIRECTORY_UTIL_H_
#import <Foundation/Foundation.h>
// Returns the URL to the passwords directory.
NSURL* GetPasswordsDirectoryURL();
// Asynchronously deletes the temporary passwords directory.
void DeletePasswordsDirectory();
#endif // IOS_CHROME_BROWSER_PASSWORDS_PASSWORDS_DIRECTORY_UTIL_H_
// Copyright 2018 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.
#import "ios/chrome/browser/passwords/passwords_directory_util.h"
#include "base/task_scheduler/post_task.h"
#include "base/threading/thread_restrictions.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
namespace {
// Synchronously deletes passwords directoy.
void DeletePasswordsDirectorySync() {
NSFileManager* file_manager = [NSFileManager defaultManager];
base::AssertBlockingAllowed();
NSURL* passwords_directory = GetPasswordsDirectoryURL();
[file_manager removeItemAtURL:passwords_directory error:nil];
}
} // namespace
NSURL* GetPasswordsDirectoryURL() {
return [[NSURL fileURLWithPath:NSTemporaryDirectory() isDirectory:YES]
URLByAppendingPathComponent:@"passwords"
isDirectory:YES];
}
void DeletePasswordsDirectory() {
base::PostTaskWithTraits(FROM_HERE,
{base::MayBlock(), base::TaskPriority::BACKGROUND},
base::BindOnce(&DeletePasswordsDirectorySync));
}
// Copyright 2018 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.
#include "ios/chrome/browser/passwords/passwords_directory_util.h"
#include "base/mac/bind_objc_block.h"
#include "base/task_scheduler/post_task.h"
#include "base/test/scoped_task_environment.h"
#import "ios/testing/wait_util.h"
#include "testing/platform_test.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
using testing::WaitUntilConditionOrTimeout;
using testing::kWaitForFileOperationTimeout;
using PasswordsDirectoryUtilTest = PlatformTest;
// Tests that DeletePasswordsDirectory() actually deletes the directory.
TEST_F(PasswordsDirectoryUtilTest, Deletion) {
base::test::ScopedTaskEnvironment environment;
NSURL* directory_url = GetPasswordsDirectoryURL();
NSURL* file_url =
[directory_url URLByAppendingPathComponent:@"TestPasswords.csv"];
// Create a new file in the passwords directory.
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
base::BindBlockArc(^() {
NSFileManager* file_manager = [NSFileManager defaultManager];
if ([file_manager createDirectoryAtURL:directory_url
withIntermediateDirectories:NO
attributes:nil
error:nil]) {
[@"test" writeToURL:file_url
atomically:YES
encoding:NSUTF8StringEncoding
error:nil];
}
}));
// Verify that the file was created in the passwords directory.
EXPECT_TRUE(
WaitUntilConditionOrTimeout(kWaitForFileOperationTimeout, ^bool() {
return [[NSFileManager defaultManager] fileExistsAtPath:file_url.path];
}));
// Delete download directory.
DeletePasswordsDirectory();
// Verify passwords directory deletion.
EXPECT_TRUE(
WaitUntilConditionOrTimeout(kWaitForFileOperationTimeout, ^bool() {
return ![[NSFileManager defaultManager] fileExistsAtPath:file_url.path];
}));
}
...@@ -363,6 +363,7 @@ source_set("eg_tests") { ...@@ -363,6 +363,7 @@ source_set("eg_tests") {
"//ios/chrome/browser/signin", "//ios/chrome/browser/signin",
"//ios/chrome/browser/sync", "//ios/chrome/browser/sync",
"//ios/chrome/browser/ui:ui_internal", "//ios/chrome/browser/ui:ui_internal",
"//ios/chrome/browser/ui:ui_util",
"//ios/chrome/browser/ui/authentication:authentication_ui", "//ios/chrome/browser/ui/authentication:authentication_ui",
"//ios/chrome/browser/ui/authentication:eg_test_support", "//ios/chrome/browser/ui/authentication:eg_test_support",
"//ios/chrome/browser/ui/settings:test_support", "//ios/chrome/browser/ui/settings:test_support",
......
...@@ -96,6 +96,9 @@ enum class ExportState { ...@@ -96,6 +96,9 @@ enum class ExportState {
// Called when the user cancels the export operation. // Called when the user cancels the export operation.
- (void)cancelExport; - (void)cancelExport;
// Called to re-enable export functionality when the export UI flow finishes.
- (void)resetExportState;
// State of the export operation. // State of the export operation.
@property(nonatomic, readonly, assign) ExportState exportState; @property(nonatomic, readonly, assign) ExportState exportState;
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "components/password_manager/core/browser/export/password_csv_writer.h" #include "components/password_manager/core/browser/export/password_csv_writer.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#import "ios/chrome/browser/passwords/passwords_directory_util.h"
#import "ios/chrome/browser/ui/settings/reauthentication_module.h" #import "ios/chrome/browser/ui/settings/reauthentication_module.h"
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#include "ui/base/l10n/l10n_util_mac.h" #include "ui/base/l10n/l10n_util_mac.h"
...@@ -62,6 +63,17 @@ enum class ReauthenticationStatus { ...@@ -62,6 +63,17 @@ enum class ReauthenticationStatus {
WriteToURLStatus (^writeToFile)() = ^() { WriteToURLStatus (^writeToFile)() = ^() {
base::AssertBlockingAllowed(); base::AssertBlockingAllowed();
NSError* error = nil; NSError* error = nil;
NSURL* directoryURL = [fileURL URLByDeletingLastPathComponent];
NSFileManager* fileManager = [NSFileManager defaultManager];
if (![fileManager createDirectoryAtURL:directoryURL
withIntermediateDirectories:YES
attributes:nil
error:nil]) {
return WriteToURLStatus::UNKNOWN_ERROR;
}
BOOL success = [data writeToURL:fileURL BOOL success = [data writeToURL:fileURL
atomically:YES atomically:YES
encoding:NSUTF8StringEncoding encoding:NSUTF8StringEncoding
...@@ -258,10 +270,13 @@ enum class ReauthenticationStatus { ...@@ -258,10 +270,13 @@ enum class ReauthenticationStatus {
[self resetExportState]; [self resetExportState];
return; return;
} }
NSURL* tempPasswordsFileURL =
[[NSURL fileURLWithPath:NSTemporaryDirectory() isDirectory:YES] NSURL* uniqueDirectoryURL = [GetPasswordsDirectoryURL()
URLByAppendingPathComponent:_tempPasswordsFileName URLByAppendingPathComponent:[[NSUUID UUID] UUIDString]
isDirectory:NO]; isDirectory:YES];
NSURL* passwordsTempFileURL =
[uniqueDirectoryURL URLByAppendingPathComponent:_tempPasswordsFileName
isDirectory:NO];
__weak PasswordExporter* weakSelf = self; __weak PasswordExporter* weakSelf = self;
void (^onFileWritten)(WriteToURLStatus) = ^(WriteToURLStatus status) { void (^onFileWritten)(WriteToURLStatus) = ^(WriteToURLStatus status) {
...@@ -275,7 +290,7 @@ enum class ReauthenticationStatus { ...@@ -275,7 +290,7 @@ enum class ReauthenticationStatus {
} }
switch (status) { switch (status) {
case WriteToURLStatus::SUCCESS: case WriteToURLStatus::SUCCESS:
[strongSelf showActivityView]; [strongSelf showActivityView:passwordsTempFileURL];
break; break;
case WriteToURLStatus::OUT_OF_DISK_SPACE_ERROR: case WriteToURLStatus::OUT_OF_DISK_SPACE_ERROR:
[strongSelf [strongSelf
...@@ -310,33 +325,26 @@ enum class ReauthenticationStatus { ...@@ -310,33 +325,26 @@ enum class ReauthenticationStatus {
self.serializedPasswords = nil; self.serializedPasswords = nil;
[_passwordFileWriter writeData:serializedPasswords [_passwordFileWriter writeData:serializedPasswords
toURL:tempPasswordsFileURL toURL:passwordsTempFileURL
handler:onFileWritten]; handler:onFileWritten];
} }
- (void)deleteTemporaryFile:(NSURL*)passwordsTempFileURL { - (void)deleteTemporaryFile:(NSURL*)passwordsTempFileURL {
__weak PasswordExporter* weakSelf = self; NSURL* uniqueDirectoryURL =
base::PostTaskWithTraitsAndReply( [passwordsTempFileURL URLByDeletingLastPathComponent];
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND}, FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
base::BindBlockArc(^() { base::BindBlockArc(^() {
base::AssertBlockingAllowed(); base::AssertBlockingAllowed();
NSFileManager* fileManager = [NSFileManager defaultManager]; NSFileManager* fileManager = [NSFileManager defaultManager];
[fileManager removeItemAtURL:passwordsTempFileURL error:nil]; [fileManager removeItemAtURL:uniqueDirectoryURL error:nil];
}),
base::BindBlockArc(^() {
[weakSelf resetExportState];
})); }));
} }
- (void)showActivityView { - (void)showActivityView:(NSURL*)passwordsTempFileURL {
NSURL* passwordsTempFileURL =
[[NSURL fileURLWithPath:NSTemporaryDirectory() isDirectory:YES]
URLByAppendingPathComponent:_tempPasswordsFileName
isDirectory:NO];
if (self.exportState == ExportState::CANCELLING) { if (self.exportState == ExportState::CANCELLING) {
// Initiate cleanup. Once the file is deleted, the export state will be
// reset;
[self deleteTemporaryFile:passwordsTempFileURL]; [self deleteTemporaryFile:passwordsTempFileURL];
[self resetExportState];
return; return;
} }
__weak PasswordExporter* weakSelf = self; __weak PasswordExporter* weakSelf = self;
......
...@@ -56,6 +56,9 @@ ...@@ -56,6 +56,9 @@
// error. // error.
@property(nonatomic, assign) WriteToURLStatus writingStatus; @property(nonatomic, assign) WriteToURLStatus writingStatus;
// Whether a write operation was attempted.
@property(nonatomic, assign) BOOL writeAttempted;
@end @end
@implementation FakePasswordFileWriter { @implementation FakePasswordFileWriter {
...@@ -64,10 +67,20 @@ ...@@ -64,10 +67,20 @@
} }
@synthesize writingStatus = _writingStatus; @synthesize writingStatus = _writingStatus;
@synthesize writeAttempted = _writeAttempted;
- (instancetype)init {
self = [super init];
if (self) {
_writeAttempted = NO;
}
return self;
}
- (void)writeData:(NSString*)data - (void)writeData:(NSString*)data
toURL:(NSURL*)fileURL toURL:(NSURL*)fileURL
handler:(void (^)(WriteToURLStatus))handler { handler:(void (^)(WriteToURLStatus))handler {
_writeAttempted = YES;
_writeStatusHandler = handler; _writeStatusHandler = handler;
} }
...@@ -104,30 +117,6 @@ class PasswordExporterTest : public PlatformTest { ...@@ -104,30 +117,6 @@ class PasswordExporterTest : public PlatformTest {
return password_forms; return password_forms;
} }
void TearDown() override {
NSURL* passwords_file_url = GetPasswordsFileURL();
if ([[NSFileManager defaultManager]
fileExistsAtPath:[passwords_file_url path]]) {
[[NSFileManager defaultManager] removeItemAtURL:passwords_file_url
error:nil];
}
PlatformTest::TearDown();
}
NSURL* GetPasswordsFileURL() {
NSString* passwords_file_name =
[l10n_util::GetNSString(IDS_PASSWORD_MANAGER_DEFAULT_EXPORT_FILENAME)
stringByAppendingString:@".csv"];
return [[NSURL fileURLWithPath:NSTemporaryDirectory() isDirectory:YES]
URLByAppendingPathComponent:passwords_file_name
isDirectory:NO];
}
BOOL PasswordFileExists() {
return [[NSFileManager defaultManager]
fileExistsAtPath:[GetPasswordsFileURL() path]];
}
id password_exporter_delegate_; id password_exporter_delegate_;
PasswordExporter* password_exporter_; PasswordExporter* password_exporter_;
MockReauthenticationModule* mock_reauthentication_module_; MockReauthenticationModule* mock_reauthentication_module_;
...@@ -135,14 +124,17 @@ class PasswordExporterTest : public PlatformTest { ...@@ -135,14 +124,17 @@ class PasswordExporterTest : public PlatformTest {
base::HistogramTester histogram_tester_; base::HistogramTester histogram_tester_;
}; };
// Tests that when reauthentication is successful, the passwords file is written // Tests that when reauthentication is successful, writing the passwords file
// and showing the activity view is requested. // is attempted and a call to show the activity view is made.
TEST_F(PasswordExporterTest, PasswordFileWriteReauthSucceeded) { TEST_F(PasswordExporterTest, PasswordFileWriteReauthSucceeded) {
mock_reauthentication_module_.shouldSucceed = YES; mock_reauthentication_module_.shouldSucceed = YES;
NSURL* passwords_file_url = GetPasswordsFileURL(); FakePasswordFileWriter* fake_password_file_writer =
[[FakePasswordFileWriter alloc] init];
fake_password_file_writer.writingStatus = WriteToURLStatus::SUCCESS;
[password_exporter_ setPasswordFileWriter:fake_password_file_writer];
OCMExpect([password_exporter_delegate_ OCMExpect([password_exporter_delegate_
showActivityViewWithActivityItems:[OCMArg isEqual:@[ passwords_file_url ]] showActivityViewWithActivityItems:[OCMArg any]
completionHandler:[OCMArg any]]); completionHandler:[OCMArg any]]);
[password_exporter_ startExportFlow:CreatePasswordList()]; [password_exporter_ startExportFlow:CreatePasswordList()];
...@@ -150,45 +142,12 @@ TEST_F(PasswordExporterTest, PasswordFileWriteReauthSucceeded) { ...@@ -150,45 +142,12 @@ TEST_F(PasswordExporterTest, PasswordFileWriteReauthSucceeded) {
// Wait for all asynchronous tasks to complete. // Wait for all asynchronous tasks to complete.
scoped_task_environment_.RunUntilIdle(); scoped_task_environment_.RunUntilIdle();
EXPECT_TRUE(PasswordFileExists()); EXPECT_TRUE(fake_password_file_writer.writeAttempted);
EXPECT_OCMOCK_VERIFY(password_exporter_delegate_);
histogram_tester_.ExpectTotalCount(
"PasswordManager.TimeReadingExportedPasswords", 1);
histogram_tester_.ExpectUniqueSample(
"PasswordManager.ExportPasswordsToCSVResult",
password_manager::metrics_util::ExportPasswordsResult::SUCCESS, 1);
histogram_tester_.ExpectUniqueSample(
"PasswordManager.ExportedPasswordsPerUserInCSV", 1, 1);
}
// Tests that the exporter becomes idle after the export finishes.
TEST_F(PasswordExporterTest, ExportIdleAfterFinishing) {
mock_reauthentication_module_.shouldSucceed = YES;
NSURL* passwords_file_url = GetPasswordsFileURL();
OCMStub(
[password_exporter_delegate_
showActivityViewWithActivityItems:[OCMArg
isEqual:@[ passwords_file_url ]]
completionHandler:[OCMArg any]])
.andDo(^(NSInvocation* invocation) {
void (^completionHandler)(NSString* activityType, BOOL completed,
NSArray* returnedItems,
NSError* activityError);
[invocation getArgument:&completionHandler atIndex:3];
// Since the completion handler doesn't use any of the
// passed in parameters, dummy arguments are passed for
// convenience.
completionHandler(@"", YES, @[], nil);
});
[password_exporter_ startExportFlow:CreatePasswordList()]; // Execute file write completion handler to continue the flow.
[fake_password_file_writer executeHandler];
// Wait for all asynchronous tasks to complete. EXPECT_OCMOCK_VERIFY(password_exporter_delegate_);
scoped_task_environment_.RunUntilIdle();
EXPECT_EQ(ExportState::IDLE, password_exporter_.exportState);
histogram_tester_.ExpectTotalCount( histogram_tester_.ExpectTotalCount(
"PasswordManager.TimeReadingExportedPasswords", 1); "PasswordManager.TimeReadingExportedPasswords", 1);
...@@ -297,6 +256,11 @@ TEST_F(PasswordExporterTest, ExportInterruptedWhenReauthFails) { ...@@ -297,6 +256,11 @@ TEST_F(PasswordExporterTest, ExportInterruptedWhenReauthFails) {
[[FakePasswordSerialzerBridge alloc] init]; [[FakePasswordSerialzerBridge alloc] init];
[password_exporter_ [password_exporter_
setPasswordSerializerBridge:fake_password_serializer_bridge]; setPasswordSerializerBridge:fake_password_serializer_bridge];
FakePasswordFileWriter* fake_password_file_writer =
[[FakePasswordFileWriter alloc] init];
[password_exporter_ setPasswordFileWriter:fake_password_file_writer];
[[password_exporter_delegate_ reject] [[password_exporter_delegate_ reject]
showActivityViewWithActivityItems:[OCMArg any] showActivityViewWithActivityItems:[OCMArg any]
completionHandler:[OCMArg any]]; completionHandler:[OCMArg any]];
...@@ -324,8 +288,8 @@ TEST_F(PasswordExporterTest, ExportInterruptedWhenReauthFails) { ...@@ -324,8 +288,8 @@ TEST_F(PasswordExporterTest, ExportInterruptedWhenReauthFails) {
scoped_task_environment_.RunUntilIdle(); scoped_task_environment_.RunUntilIdle();
// Serializing passwords has finished, but reauthentication was not // Serializing passwords has finished, but reauthentication was not
// successful, so the file is not written. // successful, so writing the file was not attempted.
EXPECT_FALSE(PasswordFileExists()); EXPECT_FALSE(fake_password_file_writer.writeAttempted);
EXPECT_EQ(ExportState::IDLE, password_exporter_.exportState); EXPECT_EQ(ExportState::IDLE, password_exporter_.exportState);
histogram_tester_.ExpectTotalCount( histogram_tester_.ExpectTotalCount(
...@@ -346,6 +310,10 @@ TEST_F(PasswordExporterTest, CancelWaitsForSerializationFinished) { ...@@ -346,6 +310,10 @@ TEST_F(PasswordExporterTest, CancelWaitsForSerializationFinished) {
[password_exporter_ [password_exporter_
setPasswordSerializerBridge:fake_password_serializer_bridge]; setPasswordSerializerBridge:fake_password_serializer_bridge];
FakePasswordFileWriter* fake_password_file_writer =
[[FakePasswordFileWriter alloc] init];
[password_exporter_ setPasswordFileWriter:fake_password_file_writer];
[[password_exporter_delegate_ reject] [[password_exporter_delegate_ reject]
showActivityViewWithActivityItems:[OCMArg any] showActivityViewWithActivityItems:[OCMArg any]
completionHandler:[OCMArg any]]; completionHandler:[OCMArg any]];
...@@ -366,7 +334,7 @@ TEST_F(PasswordExporterTest, CancelWaitsForSerializationFinished) { ...@@ -366,7 +334,7 @@ TEST_F(PasswordExporterTest, CancelWaitsForSerializationFinished) {
// is invoked. As this should not happen, mark the test as failed. // is invoked. As this should not happen, mark the test as failed.
GTEST_FAIL(); GTEST_FAIL();
} }
EXPECT_FALSE(PasswordFileExists()); EXPECT_FALSE(fake_password_file_writer.writeAttempted);
EXPECT_EQ(ExportState::IDLE, password_exporter_.exportState); EXPECT_EQ(ExportState::IDLE, password_exporter_.exportState);
histogram_tester_.ExpectTotalCount( histogram_tester_.ExpectTotalCount(
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h" #include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h"
#import "ios/chrome/browser/ui/settings/reauthentication_module.h" #import "ios/chrome/browser/ui/settings/reauthentication_module.h"
#include "ios/chrome/browser/ui/tools_menu/public/tools_menu_constants.h" #include "ios/chrome/browser/ui/tools_menu/public/tools_menu_constants.h"
#include "ios/chrome/browser/ui/ui_util.h"
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/app/chrome_test_util.h" #import "ios/chrome/test/app/chrome_test_util.h"
#import "ios/chrome/test/app/password_test_util.h" #import "ios/chrome/test/app/password_test_util.h"
...@@ -57,6 +58,7 @@ using chrome_test_util::ButtonWithAccessibilityLabel; ...@@ -57,6 +58,7 @@ using chrome_test_util::ButtonWithAccessibilityLabel;
using chrome_test_util::SettingsMenuBackButton; using chrome_test_util::SettingsMenuBackButton;
using chrome_test_util::NavigationBarDoneButton; using chrome_test_util::NavigationBarDoneButton;
using chrome_test_util::SetUpAndReturnMockReauthenticationModule; using chrome_test_util::SetUpAndReturnMockReauthenticationModule;
using chrome_test_util::SetUpAndReturnMockReauthenticationModuleForExport;
namespace { namespace {
...@@ -1580,4 +1582,72 @@ PasswordForm CreateSampleFormWithIndex(int index) { ...@@ -1580,4 +1582,72 @@ PasswordForm CreateSampleFormWithIndex(int index) {
assertWithMatcher:grey_notNil()]; assertWithMatcher:grey_notNil()];
} }
// Test export flow
- (void)testExportFlow {
if (!base::FeatureList::IsEnabled(
password_manager::features::kPasswordExport)) {
return;
}
// Saving a form is needed for exporting passwords.
SaveExamplePasswordForm();
OpenPasswordSettings();
MockReauthenticationModule* mock_reauthentication_module =
SetUpAndReturnMockReauthenticationModuleForExport();
mock_reauthentication_module.shouldSucceed = YES;
[[EarlGrey
selectElementWithMatcher:chrome_test_util::ButtonWithAccessibilityLabelId(
IDS_IOS_EXPORT_PASSWORDS)]
performAction:grey_tap()];
// Tap the alert's Export passwords... button to confirm. Check
// accessibilityTrait to differentiate against the above matching element,
// which has UIAccessibilityTraitSelected.
// TODO(crbug.com/751311): Revisit and check if there is a better solution to
// match the button.
id<GREYMatcher> exportConfirmationButton = grey_allOf(
ButtonWithAccessibilityLabel(
l10n_util::GetNSString(IDS_IOS_EXPORT_PASSWORDS)),
grey_not(grey_accessibilityTrait(UIAccessibilityTraitSelected)), nil);
[[EarlGrey selectElementWithMatcher:exportConfirmationButton]
performAction:grey_tap()];
// Wait until the alerts are dismissed.
[[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
// Check that export button is disabled
[[EarlGrey
selectElementWithMatcher:chrome_test_util::ButtonWithAccessibilityLabelId(
IDS_IOS_EXPORT_PASSWORDS)]
assertWithMatcher:grey_accessibilityTrait(
UIAccessibilityTraitNotEnabled)];
if (IsIPadIdiom()) {
// Tap outside the activity view to dismiss it, because it is not
// accompanied by a "Cancel" button on iPad.
[[EarlGrey selectElementWithMatcher:
chrome_test_util::ButtonWithAccessibilityLabelId(
IDS_IOS_EXPORT_PASSWORDS)] performAction:grey_tap()];
} else {
// Tap on the "Cancel" button accompanying the activity view to dismiss it.
[[EarlGrey
selectElementWithMatcher:grey_allOf(
ButtonWithAccessibilityLabel(@"Cancel"),
grey_interactable(), nullptr)]
performAction:grey_tap()];
}
// Wait until the activity view is dismissed.
[[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
// Check that export button is re-enabled.
[[EarlGrey
selectElementWithMatcher:chrome_test_util::ButtonWithAccessibilityLabelId(
IDS_IOS_EXPORT_PASSWORDS)]
assertWithMatcher:grey_not(grey_accessibilityTrait(
UIAccessibilityTraitNotEnabled))];
}
@end @end
...@@ -127,11 +127,51 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -127,11 +127,51 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
@implementation BlacklistedFormContentItem @implementation BlacklistedFormContentItem
@end @end
@protocol PasswordExportActivityViewControllerDelegate<NSObject>
// Used to reset the export state when the activity view disappears.
- (void)resetExport;
@end
@interface PasswordExportActivityViewController : UIActivityViewController
- (PasswordExportActivityViewController*)
initWithActivityItems:(NSArray*)activityItems
delegate:
(id<PasswordExportActivityViewControllerDelegate>)delegate;
@end
@implementation PasswordExportActivityViewController {
__weak id<PasswordExportActivityViewControllerDelegate> _weakDelegate;
}
- (PasswordExportActivityViewController*)
initWithActivityItems:(NSArray*)activityItems
delegate:
(id<PasswordExportActivityViewControllerDelegate>)delegate {
self = [super initWithActivityItems:activityItems applicationActivities:nil];
if (self) {
_weakDelegate = delegate;
}
return self;
}
- (void)viewDidDisappear:(BOOL)animated {
[_weakDelegate resetExport];
[super viewDidDisappear:animated];
}
@end
@interface SavePasswordsCollectionViewController ()< @interface SavePasswordsCollectionViewController ()<
BooleanObserver, BooleanObserver,
PasswordDetailsCollectionViewControllerDelegate, PasswordDetailsCollectionViewControllerDelegate,
SuccessfulReauthTimeAccessor, SuccessfulReauthTimeAccessor,
PasswordExporterDelegate> { PasswordExporterDelegate,
PasswordExportActivityViewControllerDelegate> {
// The observable boolean that binds to the password manager setting state. // The observable boolean that binds to the password manager setting state.
// Saved passwords are only on if the password manager is enabled. // Saved passwords are only on if the password manager is enabled.
PrefBackedBoolean* passwordManagerEnabled_; PrefBackedBoolean* passwordManagerEnabled_;
...@@ -516,10 +556,10 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -516,10 +556,10 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
if (enabled) { if (enabled) {
DCHECK(exportReady_ && ![self.editor isEditing]); DCHECK(exportReady_ && ![self.editor isEditing]);
exportPasswordsItem_.textColor = [[MDCPalette greyPalette] tint900]; exportPasswordsItem_.textColor = [[MDCPalette greyPalette] tint900];
exportPasswordsItem_.accessibilityTraits = UIAccessibilityTraitButton; exportPasswordsItem_.accessibilityTraits &= ~UIAccessibilityTraitNotEnabled;
} else { } else {
exportPasswordsItem_.textColor = [[MDCPalette greyPalette] tint500]; exportPasswordsItem_.textColor = [[MDCPalette greyPalette] tint500];
exportPasswordsItem_.accessibilityTraits = UIAccessibilityTraitNotEnabled; exportPasswordsItem_.accessibilityTraits |= UIAccessibilityTraitNotEnabled;
} }
[self reconfigureCellsForItems:@[ exportPasswordsItem_ ]]; [self reconfigureCellsForItems:@[ exportPasswordsItem_ ]];
} }
...@@ -858,9 +898,10 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -858,9 +898,10 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
NSArray* returnedItems, NSArray* returnedItems,
NSError* activityError)) NSError* activityError))
completionHandler { completionHandler {
UIActivityViewController* activityViewController = PasswordExportActivityViewController* activityViewController =
[[UIActivityViewController alloc] initWithActivityItems:activityItems [[PasswordExportActivityViewController alloc]
applicationActivities:nil]; initWithActivityItems:activityItems
delegate:self];
NSArray* excludedActivityTypes = @[ NSArray* excludedActivityTypes = @[
UIActivityTypeAddToReadingList, UIActivityTypeAirDrop, UIActivityTypeAddToReadingList, UIActivityTypeAirDrop,
UIActivityTypeCopyToPasteboard, UIActivityTypeOpenInIBooks, UIActivityTypeCopyToPasteboard, UIActivityTypeOpenInIBooks,
...@@ -893,6 +934,14 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -893,6 +934,14 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
[self presentViewController:activityViewController]; [self presentViewController:activityViewController];
} }
#pragma mark - PasswordExportActivityViewControllerDelegate
- (void)resetExport {
[self.passwordExporter resetExportState];
}
#pragma mark Helper methods
- (void)presentViewController:(UIViewController*)viewController { - (void)presentViewController:(UIViewController*)viewController {
if (self.presentedViewController == preparingPasswordsAlert_ && if (self.presentedViewController == preparingPasswordsAlert_ &&
!preparingPasswordsAlert_.beingDismissed) { !preparingPasswordsAlert_.beingDismissed) {
...@@ -908,8 +957,6 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -908,8 +957,6 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
} }
} }
#pragma mark Helper methods
// Sets the save passwords switch item's enabled status to |enabled| and // Sets the save passwords switch item's enabled status to |enabled| and
// reconfigures the corresponding cell. // reconfigures the corresponding cell.
- (void)setSavePasswordsSwitchItemEnabled:(BOOL)enabled { - (void)setSavePasswordsSwitchItemEnabled:(BOOL)enabled {
......
...@@ -333,14 +333,16 @@ TEST_P(SavePasswordsCollectionViewControllerTest, ...@@ -333,14 +333,16 @@ TEST_P(SavePasswordsCollectionViewControllerTest,
UIColor* disabledColor = [[MDCPalette greyPalette] tint500]; UIColor* disabledColor = [[MDCPalette greyPalette] tint500];
EXPECT_NSEQ(disabledColor, exportButton.textColor); EXPECT_NSEQ(disabledColor, exportButton.textColor);
EXPECT_EQ(UIAccessibilityTraitNotEnabled, exportButton.accessibilityTraits); EXPECT_TRUE(exportButton.accessibilityTraits &
UIAccessibilityTraitNotEnabled);
// Add blacklisted form. // Add blacklisted form.
AddBlacklistedForm1(); AddBlacklistedForm1();
// The export button should still be disabled as exporting blacklisted forms // The export button should still be disabled as exporting blacklisted forms
// is not currently supported. // is not currently supported.
EXPECT_NSEQ(disabledColor, exportButton.textColor); EXPECT_NSEQ(disabledColor, exportButton.textColor);
EXPECT_EQ(UIAccessibilityTraitNotEnabled, exportButton.accessibilityTraits); EXPECT_TRUE(exportButton.accessibilityTraits &
UIAccessibilityTraitNotEnabled);
} }
TEST_P(SavePasswordsCollectionViewControllerTest, TEST_P(SavePasswordsCollectionViewControllerTest,
...@@ -352,7 +354,8 @@ TEST_P(SavePasswordsCollectionViewControllerTest, ...@@ -352,7 +354,8 @@ TEST_P(SavePasswordsCollectionViewControllerTest,
CheckTextCellTitleWithId(IDS_IOS_EXPORT_PASSWORDS, 3, 0); CheckTextCellTitleWithId(IDS_IOS_EXPORT_PASSWORDS, 3, 0);
EXPECT_NSEQ([[MDCPalette greyPalette] tint900], exportButton.textColor); EXPECT_NSEQ([[MDCPalette greyPalette] tint900], exportButton.textColor);
EXPECT_NE(UIAccessibilityTraitNotEnabled, exportButton.accessibilityTraits); EXPECT_FALSE(exportButton.accessibilityTraits &
UIAccessibilityTraitNotEnabled);
} }
// Tests that the "Export Passwords..." button is greyed out in edit mode. // Tests that the "Export Passwords..." button is greyed out in edit mode.
...@@ -371,7 +374,8 @@ TEST_P(SavePasswordsCollectionViewControllerTest, ...@@ -371,7 +374,8 @@ TEST_P(SavePasswordsCollectionViewControllerTest,
collectionViewWillBeginEditing:save_passwords_controller.collectionView]; collectionViewWillBeginEditing:save_passwords_controller.collectionView];
EXPECT_NSEQ([[MDCPalette greyPalette] tint500], exportButton.textColor); EXPECT_NSEQ([[MDCPalette greyPalette] tint500], exportButton.textColor);
EXPECT_EQ(UIAccessibilityTraitNotEnabled, exportButton.accessibilityTraits); EXPECT_TRUE(exportButton.accessibilityTraits &
UIAccessibilityTraitNotEnabled);
} }
// Tests that the "Export Passwords..." button is enabled after exiting // Tests that the "Export Passwords..." button is enabled after exiting
...@@ -393,7 +397,8 @@ TEST_P(SavePasswordsCollectionViewControllerTest, ...@@ -393,7 +397,8 @@ TEST_P(SavePasswordsCollectionViewControllerTest,
collectionViewWillEndEditing:save_passwords_controller.collectionView]; collectionViewWillEndEditing:save_passwords_controller.collectionView];
EXPECT_NSEQ([[MDCPalette greyPalette] tint900], exportButton.textColor); EXPECT_NSEQ([[MDCPalette greyPalette] tint900], exportButton.textColor);
EXPECT_NE(UIAccessibilityTraitNotEnabled, exportButton.accessibilityTraits); EXPECT_FALSE(exportButton.accessibilityTraits &
UIAccessibilityTraitNotEnabled);
} }
TEST_P(SavePasswordsCollectionViewControllerTest, PropagateDeletionToStore) { TEST_P(SavePasswordsCollectionViewControllerTest, PropagateDeletionToStore) {
......
...@@ -28,6 +28,11 @@ namespace chrome_test_util { ...@@ -28,6 +28,11 @@ namespace chrome_test_util {
// blocked with a reauth prompt, and return the fake reauthentication module. // blocked with a reauth prompt, and return the fake reauthentication module.
MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule(); MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule();
// Replace the reauthentication module in
// PasswordExporter with a fake one to avoid being
// blocked with a reauth prompt, and return the fake reauthentication module.
MockReauthenticationModule* SetUpAndReturnMockReauthenticationModuleForExport();
} // namespace chrome_test_util } // namespace chrome_test_util
#endif // IOS_CHROME_TEST_APP_PASSWORD_TEST_UTIL_H_ #endif // IOS_CHROME_TEST_APP_PASSWORD_TEST_UTIL_H_
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/mac/foundation_util.h" #include "base/mac/foundation_util.h"
#import "ios/chrome/browser/ui/settings/password_details_collection_view_controller_for_testing.h" #import "ios/chrome/browser/ui/settings/password_details_collection_view_controller_for_testing.h"
#import "ios/chrome/browser/ui/settings/save_passwords_collection_view_controller.h"
#import "ios/chrome/browser/ui/settings/settings_navigation_controller.h" #import "ios/chrome/browser/ui/settings/settings_navigation_controller.h"
#import "ios/chrome/browser/ui/util/top_view_controller.h" #import "ios/chrome/browser/ui/util/top_view_controller.h"
...@@ -60,4 +61,24 @@ MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule() { ...@@ -60,4 +61,24 @@ MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule() {
return mock_reauthentication_module; return mock_reauthentication_module;
} }
// Replace the reauthentication module in
// PasswordExporter with a fake one to avoid being
// blocked with a reauth prompt, and return the fake reauthentication module.
MockReauthenticationModule*
SetUpAndReturnMockReauthenticationModuleForExport() {
MockReauthenticationModule* mock_reauthentication_module =
[[MockReauthenticationModule alloc] init];
// TODO(crbug.com/754642): Stop using TopPresentedViewController();
SettingsNavigationController* settings_navigation_controller =
base::mac::ObjCCastStrict<SettingsNavigationController>(
top_view_controller::TopPresentedViewController());
SavePasswordsCollectionViewController*
save_passwords_collection_view_controller =
base::mac::ObjCCastStrict<SavePasswordsCollectionViewController>(
settings_navigation_controller.topViewController);
[save_passwords_collection_view_controller
setReauthenticationModuleForExporter:mock_reauthentication_module];
return mock_reauthentication_module;
}
} // namespace } // namespace
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