Skip to content

Commit

Permalink
[iOS] Fixing NOTREACHED when entering passphrase
Browse files Browse the repository at this point in the history
Removing NOTREACHED in -[SyncEncryptionPassphraseTableViewController
reportDismissalUserAction]. This method is called when the view is
presented by the infobar.
Once the passphrase is entered (or the view is canceled by the user),
then the navigation controller is dismissed, and this method is called.

This bug probably exists from the origin but never caught since
NOTREACHED() was not crashing in canary until recently.

Bug: 1075647
Change-Id: I8b7a3da1620c6cc60a221d1b0d09ed712b8cf46d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2181366
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#767537}
  • Loading branch information
Jérôme Lebel authored and Commit Bot committed May 11, 2020
1 parent 390249f commit 254af47
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 2 deletions.
36 changes: 36 additions & 0 deletions ios/chrome/browser/ui/settings/sync/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,39 @@ source_set("unit_tests") {
"//ui/base",
]
}

source_set("eg_tests") {
defines = [ "CHROME_EARL_GREY_1" ]
configs += [ "//build/config/compiler:enable_arc" ]
testonly = true
sources = [ "sync_encryption_passphrase_table_view_controller_egtest.mm" ]
deps = [
"//base/test:test_support",
"//ios/chrome/app/strings",
"//ios/chrome/browser/ui/authentication:eg_test_support",
"//ios/chrome/test/app:test_support",
"//ios/chrome/test/earl_grey:test_support",
"//ios/public/provider/chrome/browser/signin:fake_chrome_identity",
"//ios/testing/earl_grey:earl_grey_support",
"//ui/base",
]
}

source_set("eg2_tests") {
defines = [ "CHROME_EARL_GREY_2" ]
configs += [
"//build/config/compiler:enable_arc",
"//build/config/ios:xctest_config",
]
testonly = true
sources = [ "sync_encryption_passphrase_table_view_controller_egtest.mm" ]
deps = [
"//base/test:test_support",
"//ios/chrome/app/strings",
"//ios/chrome/browser/ui/authentication:eg_test_support+eg2",
"//ios/chrome/test/earl_grey:eg_test_support+eg2",
"//ios/testing/earl_grey:eg_test_support+eg2",
"//ios/third_party/earl_grey2:test_lib",
]
libs = [ "UIKit.framework" ]
}
Original file line number Diff line number Diff line change
Expand Up @@ -509,8 +509,8 @@ - (void)onEndBatchOfRefreshTokenStateChanges {
#pragma mark - SettingsControllerProtocol callbacks

- (void)reportDismissalUserAction {
// Sync Passphrase Settings screen does not have Done button.
NOTREACHED();
// Sync Passphrase Settings screen can be closed when being presented from
// an infobar.
}

- (void)settingsWillBeDismissed {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// 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.

#import <UIKit/UIKit.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/grit/ios_strings.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/earl_grey_test.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif

using chrome_test_util::ButtonWithAccessibilityLabelId;
using chrome_test_util::NavigationBarCancelButton;

@interface SyncEncryptionPassphraseTestCase : ChromeTestCase
@end

@implementation SyncEncryptionPassphraseTestCase

// Tests to open the sync passphrase view, and to close it.
- (void)testShowSyncPassphraseAndDismiss {
[ChromeEarlGrey addBookmarkWithSyncPassphrase:@"hello"];
// Signin.
FakeChromeIdentity* fakeIdentity = [SigninEarlGreyUtils fakeIdentity1];
[SigninEarlGreyUI signinWithFakeIdentity:fakeIdentity];
[ChromeEarlGrey openNewTab];
[[EarlGrey selectElementWithMatcher:ButtonWithAccessibilityLabelId(
IDS_IOS_SYNC_ENTER_PASSPHRASE)]
performAction:grey_tap()];
[[EarlGrey selectElementWithMatcher:NavigationBarCancelButton()]
performAction:grey_tap()];
// Wait until the settings is fully removed.
[[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
}

@end
1 change: 1 addition & 0 deletions ios/chrome/test/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ source_set("test_support") {
"//components/metrics:demographics_test_support",
"//components/prefs",
"//components/signin/public/base",
"//components/sync:test_support_nigori",
"//components/sync/base",
"//components/sync/test/fake_server",
"//components/sync_device_info",
Expand Down
4 changes: 4 additions & 0 deletions ios/chrome/test/app/sync_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ void DeleteTypedUrlFromClient(const GURL& url);
// Deletes typed URL on FakeServer by injecting a tombstone.
void DeleteTypedUrlFromFakeSyncServer(std::string url);

// Adds a bookmark with a sync passphrase. The sync server will need the sync
// passphrase to start.
void AddBookmarkWithSyncPassphrase(const std::string& sync_passphrase);

} // namespace chrome_test_util

#endif // IOS_CHROME_TEST_APP_SYNC_TEST_UTIL_H_
26 changes: 26 additions & 0 deletions ios/chrome/test/app/sync_test_util.mm
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
#include "components/sync/base/pref_names.h"
#include "components/sync/driver/profile_sync_service.h"
#include "components/sync/driver/sync_service.h"
#include "components/sync/engine_impl/loopback_server/loopback_server_entity.h"
#include "components/sync/nigori/nigori_test_utils.h"
#include "components/sync/test/fake_server/entity_builder_factory.h"
#include "components/sync/test/fake_server/fake_server.h"
#include "components/sync/test/fake_server/fake_server_network_resources.h"
#include "components/sync/test/fake_server/fake_server_nigori_helper.h"
#include "components/sync/test/fake_server/fake_server_verifier.h"
#include "components/sync/test/fake_server/sessions_hierarchy.h"
#include "components/sync_device_info/device_info.h"
Expand Down Expand Up @@ -62,6 +65,16 @@ void OverrideSyncNetwork(const syncer::CreateHttpPostProviderFactory&
service->OverrideNetworkForTest(create_http_post_provider_factory_cb);
}

// Returns a bookmark server entity based on |title| and |url|.
std::unique_ptr<syncer::LoopbackServerEntity> CreateBookmarkServerEntity(
const std::string& title,
const GURL& url) {
fake_server::EntityBuilderFactory entity_builder_factory;
fake_server::BookmarkEntityBuilder bookmark_builder =
entity_builder_factory.NewBookmarkEntityBuilder(title);
return bookmark_builder.BuildBookmark(url);
}

} // namespace

namespace chrome_test_util {
Expand Down Expand Up @@ -374,4 +387,17 @@ void DeleteTypedUrlFromFakeSyncServer(std::string url) {
}
}

void AddBookmarkWithSyncPassphrase(const std::string& sync_passphrase) {
syncer::KeyParamsForTesting key_params = {
syncer::KeyDerivationParams::CreateForPbkdf2(), sync_passphrase};
std::unique_ptr<syncer::LoopbackServerEntity> server_entity =
CreateBookmarkServerEntity("PBKDF2-encrypted bookmark",
GURL("http://example.com/doesnt-matter"));
server_entity->SetSpecifics(GetEncryptedBookmarkEntitySpecifics(
server_entity->GetSpecifics().bookmark(), key_params));
gSyncFakeServer->InjectEntity(std::move(server_entity));
fake_server::SetNigoriInFakeServer(CreateCustomPassphraseNigori(key_params),
gSyncFakeServer);
}

} // namespace chrome_test_util
1 change: 1 addition & 0 deletions ios/chrome/test/earl_grey/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ chrome_ios_eg_test("ios_chrome_settings_egtests") {
"//ios/chrome/browser/ui/settings/google_services:eg_tests",
"//ios/chrome/browser/ui/settings/language:eg_tests",
"//ios/chrome/browser/ui/settings/password:eg_tests",
"//ios/chrome/browser/ui/settings/sync:eg_tests",
]

executable_args = [
Expand Down
4 changes: 4 additions & 0 deletions ios/chrome/test/earl_grey/chrome_earl_grey.h
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,10 @@ id ExecuteJavaScript(NSString* javascript, NSError* __autoreleasing* out_error);
// calling this.
- (std::string)syncCacheGUID;

// Adds a bookmark with a sync passphrase. The sync server will need the sync
// passphrase to start.
- (void)addBookmarkWithSyncPassphrase:(NSString*)syncPassphrase;

#pragma mark - WebState Utilities (EG2)

// Taps html element with |elementID| in the current web state.
Expand Down
4 changes: 4 additions & 0 deletions ios/chrome/test/earl_grey/chrome_earl_grey.mm
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,10 @@ - (void)signOutAndClearAccounts {
[self signOutAndClearIdentities];
}

- (void)addBookmarkWithSyncPassphrase:(NSString*)syncPassphrase {
[ChromeEarlGreyAppInterface addBookmarkWithSyncPassphrase:syncPassphrase];
}

#pragma mark - Bookmarks Utilities (EG2)

- (void)waitForBookmarksToFinishLoading {
Expand Down
4 changes: 4 additions & 0 deletions ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,10 @@
name:(NSString*)name
count:(NSUInteger)count;

// Adds a bookmark with a sync passphrase. The sync server will need the sync
// passphrase to start.
+ (void)addBookmarkWithSyncPassphrase:(NSString*)syncPassphrase;

#pragma mark - JavaScript Utilities (EG2)

// Executes JavaScript on current WebState, and waits for either the completion
Expand Down
7 changes: 7 additions & 0 deletions ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.mm
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,13 @@ + (NSError*)verifySessionsOnSyncServerWithSpecs:(NSArray<NSString*>*)specs {
return error;
}

+ (void)addBookmarkWithSyncPassphrase:(NSString*)syncPassphrase {
chrome_test_util::AddBookmarkWithSyncPassphrase(
base::SysNSStringToUTF8(syncPassphrase));
}

#pragma mark - JavaScript Utilities (EG2)

+ (id)executeJavaScript:(NSString*)javaScript error:(NSError**)outError {
__block bool handlerCalled = false;
__block id blockResult = nil;
Expand Down
1 change: 1 addition & 0 deletions ios/chrome/test/earl_grey2/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ chrome_ios_eg2_test("ios_chrome_settings_eg2tests_module") {
"//ios/chrome/browser/ui/settings/google_services:eg2_tests",
"//ios/chrome/browser/ui/settings/language:eg2_tests",
"//ios/chrome/browser/ui/settings/password:eg2_tests",
"//ios/chrome/browser/ui/settings/sync:eg2_tests",
]
data_deps = [ ":ios_chrome_eg2tests" ]
}
Expand Down

0 comments on commit 254af47

Please sign in to comment.