Skip to content

Commit

Permalink
Reland: [iOS] Make password leak checks toggleable when Safe Browsing…
Browse files Browse the repository at this point in the history
… is not available

This relands r776646 (previously reviewed at http://crrev.com/c/2237716),
with test fixes:
1. On the settings pages, scroll up when needed (on small devices)
   to make the Safe Browsing and password leak check items visible.
2. In testTogglePasswordLeakCheckWhenSafeBrowsingNotAvailable,
   add coverage for the case where a user opts-out of Safe Browsing
   on another device.

Description of previous CL:
This CL fixes a regression from http://crrev.com/c/2174995
that prevents users who do not yet have Safe Browsing
rolled out to them from toggling the password leak check setting.

This CL ensures that the value of the Safe Browsing opt-out
pref is always initialized in GoogleServicesSettingsMediator,
regardless of whether Safe Browsing has been rolled out. With
this change, the other logic changes in
http://crrev.com/c/2174995 work as expected, whether or not
Safe Browsing is available.

Bug: 1090406, 1093306
Change-Id: Id511dc339be146bcc15d865d90feb5cf93369b51
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239300
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Ali Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776981}
  • Loading branch information
alijuma authored and Commit Bot committed Jun 10, 2020
1 parent d4c5e64 commit b355acf
Show file tree
Hide file tree
Showing 5 changed files with 167 additions and 7 deletions.
6 changes: 6 additions & 0 deletions ios/chrome/browser/ui/settings/google_services/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ source_set("eg_tests") {
":constants",
":test_support",
"//base/test:test_support",
"//components/password_manager/core/common",
"//components/safe_browsing/core:features",
"//components/safe_browsing/core/common:safe_browsing_prefs",
"//ios/chrome/app/strings",
"//ios/chrome/browser/tabs",
"//ios/chrome/browser/ui:feature_flags",
Expand Down Expand Up @@ -177,6 +180,9 @@ source_set("eg2_tests") {
":eg_test_support+eg2",
"//base",
"//base/test:test_support",
"//components/password_manager/core/common",
"//components/safe_browsing/core:features",
"//components/safe_browsing/core/common:safe_browsing_prefs",
"//ios/chrome/app/strings",
"//ios/chrome/browser/ui:feature_flags",
"//ios/chrome/browser/ui/authentication:eg_test_support+eg2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,10 @@ extern NSString* const kManageSyncCellAccessibilityIdentifier;
// Accessibility identifier for the account list cell.
extern NSString* const kAccountListItemAccessibilityIdentifier;

// Accessibility identifier for the password leak check cell.
extern NSString* const kPasswordLeakCheckItemAccessibilityIdentifier;

// Accessibility identifier for the Safe Browsing cell.
extern NSString* const kSafeBrowsingItemAccessibilityIdentifier;

#endif // IOS_CHROME_BROWSER_UI_SETTINGS_GOOGLE_SERVICES_GOOGLE_SERVICES_SETTINGS_CONSTANTS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,9 @@

NSString* const kAccountListItemAccessibilityIdentifier =
@"AccountListItemAccessibilityIdentifier";

NSString* const kPasswordLeakCheckItemAccessibilityIdentifier =
@"PasswordLeakCheckItemAccessibilityIdentifier";

NSString* const kSafeBrowsingItemAccessibilityIdentifier =
@"SafeBrowsingItemAccessibilityIdentifier";
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,22 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
#include "components/safe_browsing/core/features.h"
#import "ios/chrome/browser/ui/authentication/signin_earl_grey_ui.h"
#import "ios/chrome/browser/ui/authentication/signin_earlgrey_utils.h"
#import "ios/chrome/browser/ui/settings/google_services/google_services_settings_app_interface.h"
#import "ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.h"
#import "ios/chrome/browser/ui/settings/google_services/manage_sync_settings_constants.h"
#include "ios/chrome/grit/ios_chromium_strings.h"
#include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/earl_grey/chrome_actions.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h"
#import "ios/chrome/test/earl_grey/chrome_matchers.h"
#import "ios/chrome/test/earl_grey/chrome_test_case.h"
#import "ios/testing/earl_grey/app_launch_manager.h"
#import "ios/testing/earl_grey/earl_grey_test.h"
#import "ui/base/l10n/l10n_util.h"

Expand Down Expand Up @@ -159,6 +164,143 @@ - (void)testOpenSSOAddAccount {
[[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
}

// Tests that password leak detection can be toggled when Safe Browsing isn't
// available.
- (void)testTogglePasswordLeakCheckWhenSafeBrowsingNotAvailable {
AppLaunchConfiguration config;
config.features_disabled.push_back(
safe_browsing::kSafeBrowsingAvailableOnIOS);
[[AppLaunchManager sharedManager] ensureAppLaunchedWithConfiguration:config];

// Ensure that Safe Browsing and password leak detection opt-outs start in
// their default (opted-in) state.
[ChromeEarlGrey setBoolValue:YES forUserPref:prefs::kSafeBrowsingEnabled];
[ChromeEarlGrey
setBoolValue:YES
forUserPref:password_manager::prefs::kPasswordLeakDetectionEnabled];

// Sign in.
FakeChromeIdentity* fakeIdentity = [SigninEarlGreyUtils fakeIdentity1];
[SigninEarlGreyUI signinWithFakeIdentity:fakeIdentity];
// Open "Google Services" settings.
[self openGoogleServicesSettings];

// Check that the password leak check toggle is enabled, and toggle it off.
[[self elementInteractionWithGreyMatcher:
chrome_test_util::SettingsSwitchCell(
kPasswordLeakCheckItemAccessibilityIdentifier,
/*is_toggled_on=*/YES,
/*enabled=*/YES)]
performAction:chrome_test_util::TurnSettingsSwitchOn(NO)];

// Check the underlying pref value.
GREYAssertFalse(
[ChromeEarlGrey userBooleanPref:password_manager::prefs::
kPasswordLeakDetectionEnabled],
@"Failed to toggle-off password leak checks");

// Toggle it back on.
[[self elementInteractionWithGreyMatcher:
chrome_test_util::SettingsSwitchCell(
kPasswordLeakCheckItemAccessibilityIdentifier,
/*is_toggled_on=*/NO,
/*enabled=*/YES)]
performAction:chrome_test_util::TurnSettingsSwitchOn(YES)];

// Check the underlying pref value.
GREYAssertTrue(
[ChromeEarlGrey userBooleanPref:password_manager::prefs::
kPasswordLeakDetectionEnabled],
@"Failed to toggle-on password leak checks");

// Close settings.
[[EarlGrey selectElementWithMatcher:SettingsDoneButton()]
performAction:grey_tap()];

// Simulate the user opting out of Safe Browsing on another device.
[ChromeEarlGrey setBoolValue:NO forUserPref:prefs::kSafeBrowsingEnabled];

// Verify that the password leak check toggle is now disabled.
[self openGoogleServicesSettings];
[[self elementInteractionWithGreyMatcher:
chrome_test_util::SettingsSwitchCell(
kPasswordLeakCheckItemAccessibilityIdentifier,
/*is_toggled_on=*/NO,
/*enabled=*/NO)] assertWithMatcher:grey_notNil()];
}

// Tests that when Safe Browsing is available, password leak detection can only
// be toggled if Safe Browsing is enabled.
- (void)testTogglePasswordLeakCheckWhenSafeBrowsingAvailable {
AppLaunchConfiguration config;
config.features_enabled.push_back(safe_browsing::kSafeBrowsingAvailableOnIOS);
[[AppLaunchManager sharedManager] ensureAppLaunchedWithConfiguration:config];

// Ensure that Safe Browsing and password leak detection opt-outs start in
// their default (opted-in) state.
[ChromeEarlGrey setBoolValue:YES forUserPref:prefs::kSafeBrowsingEnabled];
[ChromeEarlGrey
setBoolValue:YES
forUserPref:password_manager::prefs::kPasswordLeakDetectionEnabled];

// Sign in.
FakeChromeIdentity* fakeIdentity = [SigninEarlGreyUtils fakeIdentity1];
[SigninEarlGreyUI signinWithFakeIdentity:fakeIdentity];
// Open "Google Services" settings.
[self openGoogleServicesSettings];

// Check that Safe Browsing is enabled, and toggle it off.
[[self elementInteractionWithGreyMatcher:
chrome_test_util::SettingsSwitchCell(
kSafeBrowsingItemAccessibilityIdentifier,
/*is_toggled_on=*/YES,
/*enabled=*/YES)]
performAction:chrome_test_util::TurnSettingsSwitchOn(NO)];

// Check that the password leak check toggle is both toggled off and disabled.
[[self elementInteractionWithGreyMatcher:
chrome_test_util::SettingsSwitchCell(
kPasswordLeakCheckItemAccessibilityIdentifier,
/*is_toggled_on=*/NO,
/*enabled=*/NO)] assertWithMatcher:grey_notNil()];

// Toggle Safe Browsing on.
[[self elementInteractionWithGreyMatcher:
chrome_test_util::SettingsSwitchCell(
kSafeBrowsingItemAccessibilityIdentifier,
/*is_toggled_on=*/NO,
/*enabled=*/YES)]
performAction:chrome_test_util::TurnSettingsSwitchOn(YES)];

// Check that the password leak check toggle is enabled, and toggle it off.
[[self elementInteractionWithGreyMatcher:
chrome_test_util::SettingsSwitchCell(
kPasswordLeakCheckItemAccessibilityIdentifier,
/*is_toggled_on=*/YES,
/*enabled=*/YES)]
performAction:chrome_test_util::TurnSettingsSwitchOn(NO)];

// Check the underlying pref value.
GREYAssertFalse(
[ChromeEarlGrey userBooleanPref:password_manager::prefs::
kPasswordLeakDetectionEnabled],
@"Failed to toggle-off password leak checks");

// Toggle password leak check detection back on.
[[self elementInteractionWithGreyMatcher:
chrome_test_util::SettingsSwitchCell(
kPasswordLeakCheckItemAccessibilityIdentifier,
/*is_toggled_on=*/NO,
/*enabled=*/YES)]
performAction:chrome_test_util::TurnSettingsSwitchOn(YES)];

// Check the underlying pref value.
GREYAssertTrue(
[ChromeEarlGrey userBooleanPref:password_manager::prefs::
kPasswordLeakDetectionEnabled],
@"Failed to toggle-on password leak checks");
}

#pragma mark - Helpers

// Opens the Google services settings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,10 @@ - (instancetype)initWithUserPrefService:(PrefService*)userPrefService
initWithPrefService:userPrefService
prefName:prefs::kSearchSuggestEnabled];
_autocompleteSearchPreference.observer = self;
if (base::FeatureList::IsEnabled(kSafeBrowsingAvailableOnIOS)) {
_safeBrowsingPreference = [[PrefBackedBoolean alloc]
initWithPrefService:userPrefService
prefName:prefs::kSafeBrowsingEnabled];
_safeBrowsingPreference.observer = self;
}
_safeBrowsingPreference = [[PrefBackedBoolean alloc]
initWithPrefService:userPrefService
prefName:prefs::kSafeBrowsingEnabled];
_safeBrowsingPreference.observer = self;
_sendDataUsagePreference = [[PrefBackedBoolean alloc]
initWithPrefService:localPrefService
prefName:metrics::prefs::kMetricsReportingEnabled];
Expand Down Expand Up @@ -597,6 +595,8 @@ - (ItemArray)nonPersonalizedItems {
detailStringID:
IDS_IOS_GOOGLE_SERVICES_SETTINGS_SAFE_BROWSING_DETAIL
dataType:0];
safeBrowsingItem.accessibilityIdentifier =
kSafeBrowsingItemAccessibilityIdentifier;
[items addObject:safeBrowsingItem];
}
[items addObject:self.passwordLeakCheckItem];
Expand Down Expand Up @@ -631,7 +631,7 @@ - (SettingsSwitchItem*)passwordLeakCheckItem {
l10n_util::GetNSString(IDS_IOS_LEAK_CHECK_SWITCH);
passwordLeakCheckItem.on = [self passwordLeakCheckItemOnState];
passwordLeakCheckItem.accessibilityIdentifier =
@"passwordLeakCheckItem_switch";
kPasswordLeakCheckItemAccessibilityIdentifier;
passwordLeakCheckItem.enabled = self.isAuthenticated;
_passwordLeakCheckItem = passwordLeakCheckItem;
}
Expand Down

0 comments on commit b355acf

Please sign in to comment.