From 7117eadc2c598e53d61945f9d93f4ff5fcec205a Mon Sep 17 00:00:00 2001 From: jdonnelly Date: Wed, 13 Jul 2016 12:51:10 -0700 Subject: [PATCH] Make full-form autofill the only implementation on iOS. Removes support for the old field-at-a-time autofill on iOS. Full-form has been enabled for all iOS users for many months. Corresponding internal change: https://chromereviews.googleplex.com/470777015 BUG=504644,268279 Review-Url: https://codereview.chromium.org/2146523004 Cr-Commit-Position: refs/heads/master@{#405237} --- components/autofill.gypi | 2 -- components/autofill/core/browser/BUILD.gn | 2 -- .../core/browser/autofill_field_trial_ios.cc | 33 ------------------- .../core/browser/autofill_field_trial_ios.h | 28 ---------------- .../autofill/core/browser/autofill_manager.cc | 8 ----- .../core/browser/autofill_manager_unittest.cc | 26 ++------------- .../autofill/core/common/autofill_switches.cc | 6 ---- .../autofill/core/common/autofill_switches.h | 2 -- .../ios/browser/js_autofill_manager.h | 11 +------ .../ios/browser/js_autofill_manager.mm | 10 ------ .../browser/resources/autofill_controller.js | 31 +---------------- ios/chrome/browser/about_flags.mm | 9 ----- .../autofill/form_suggestion_controller.mm | 8 +---- 13 files changed, 5 insertions(+), 171 deletions(-) delete mode 100644 components/autofill/core/browser/autofill_field_trial_ios.cc delete mode 100644 components/autofill/core/browser/autofill_field_trial_ios.h diff --git a/components/autofill.gypi b/components/autofill.gypi index b0bc8115462166..510807d2472b27 100644 --- a/components/autofill.gypi +++ b/components/autofill.gypi @@ -250,8 +250,6 @@ 'conditions': [ ['OS=="ios"', { 'sources': [ - 'autofill/core/browser/autofill_field_trial_ios.cc', - 'autofill/core/browser/autofill_field_trial_ios.h', 'autofill/core/browser/keyboard_accessory_metrics_logger.h', 'autofill/core/browser/keyboard_accessory_metrics_logger.mm', ], diff --git a/components/autofill/core/browser/BUILD.gn b/components/autofill/core/browser/BUILD.gn index b51755e6f88f7f..67374ae5138c8e 100644 --- a/components/autofill/core/browser/BUILD.gn +++ b/components/autofill/core/browser/BUILD.gn @@ -147,8 +147,6 @@ source_set("browser") { if (is_ios) { sources += [ - "autofill_field_trial_ios.cc", - "autofill_field_trial_ios.h", "keyboard_accessory_metrics_logger.h", "keyboard_accessory_metrics_logger.mm", ] diff --git a/components/autofill/core/browser/autofill_field_trial_ios.cc b/components/autofill/core/browser/autofill_field_trial_ios.cc deleted file mode 100644 index 95290e046fcf45..00000000000000 --- a/components/autofill/core/browser/autofill_field_trial_ios.cc +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2015 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 "components/autofill/core/browser/autofill_field_trial_ios.h" - -#include "base/command_line.h" -#include "base/metrics/field_trial.h" -#include "components/autofill/core/common/autofill_switches.h" - -namespace autofill { - -// The full-form Autofill field trial name. -const char kFullFormFieldTrialName[] = "FullFormAutofill"; - -// static -bool AutofillFieldTrialIOS::IsFullFormAutofillEnabled() { - // Query the field trial state first to ensure that UMA reports the correct - // group. - std::string field_trial_state = - base::FieldTrialList::FindFullName(kFullFormFieldTrialName); - - const base::CommandLine* command_line = - base::CommandLine::ForCurrentProcess(); - if (command_line->HasSwitch(switches::kDisableFullFormAutofillIOS)) - return false; - if (command_line->HasSwitch(switches::kEnableFullFormAutofillIOS)) - return true; - - return !field_trial_state.empty() && field_trial_state != "Disabled"; -} - -} // namespace autofill diff --git a/components/autofill/core/browser/autofill_field_trial_ios.h b/components/autofill/core/browser/autofill_field_trial_ios.h deleted file mode 100644 index 9750b7135d9152..00000000000000 --- a/components/autofill/core/browser/autofill_field_trial_ios.h +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2015 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 COMPONENTS_AUTOFILL_CORE_BROWSER_AUTOFILL_FIELD_TRIAL_IOS_H_ -#define COMPONENTS_AUTOFILL_CORE_BROWSER_AUTOFILL_FIELD_TRIAL_IOS_H_ - -#include "base/macros.h" - -namespace autofill { - -// This class manages the iOS Autofill field trials. -class AutofillFieldTrialIOS { - public: - // Returns whether the user is in a full-form Autofill field trial. Full-form - // Autofill fills all fields of the form at once, similar to the desktop and - // Clank Autofill implementations. Previous iOS implementation requires user - // to select an Autofill value for each field individually, automatically - // advancing focus to the next field after each selection. - static bool IsFullFormAutofillEnabled(); - - private: - DISALLOW_IMPLICIT_CONSTRUCTORS(AutofillFieldTrialIOS); -}; - -} // namespace autofill - -#endif // COMPONENTS_AUTOFILL_CORE_BROWSER_AUTOFILL_FIELD_TRIAL_IOS_H_ diff --git a/components/autofill/core/browser/autofill_manager.cc b/components/autofill/core/browser/autofill_manager.cc index 04650708eb405a..c42ca66e27ebbd 100644 --- a/components/autofill/core/browser/autofill_manager.cc +++ b/components/autofill/core/browser/autofill_manager.cc @@ -66,7 +66,6 @@ #include "url/gurl.h" #if defined(OS_IOS) -#include "components/autofill/core/browser/autofill_field_trial_ios.h" #include "components/autofill/core/browser/keyboard_accessory_metrics_logger.h" #endif @@ -595,13 +594,6 @@ bool AutofillManager::WillFillCreditCardNumber(const FormData& form, if (autofill_field->Type().GetStorableType() == CREDIT_CARD_NUMBER) return true; -#if defined(OS_IOS) - // On iOS, we only fill out one field at a time (assuming the new full-form - // feature isn't enabled). So we only need to check the current field. - if (!AutofillFieldTrialIOS::IsFullFormAutofillEnabled()) - return false; -#endif - // If the relevant section is already autofilled, the new fill operation will // only fill |autofill_field|. if (SectionIsAutofilled(*form_structure, form, autofill_field->section())) diff --git a/components/autofill/core/browser/autofill_manager_unittest.cc b/components/autofill/core/browser/autofill_manager_unittest.cc index 5ebd6204cb7769..7e9fb10c5c5400 100644 --- a/components/autofill/core/browser/autofill_manager_unittest.cc +++ b/components/autofill/core/browser/autofill_manager_unittest.cc @@ -923,20 +923,6 @@ class AutofillManagerTest : public testing::Test { autofill_manager_->FillOrPreviewCreditCardForm( AutofillDriver::FORM_DATA_ACTION_FILL, kDefaultPageID, *form, form->fields[0], *card); - -#if defined(OS_IOS) - // Filling out the entire form on iOS requires requesting autofill on each - // of the form fields. - autofill_manager_->FillOrPreviewCreditCardForm( - AutofillDriver::FORM_DATA_ACTION_FILL, kDefaultPageID, *form, - form->fields[1], *card); - autofill_manager_->FillOrPreviewCreditCardForm( - AutofillDriver::FORM_DATA_ACTION_FILL, kDefaultPageID, *form, - form->fields[2], *card); - autofill_manager_->FillOrPreviewCreditCardForm( - AutofillDriver::FORM_DATA_ACTION_FILL, kDefaultPageID, *form, - form->fields[3], *card); -#endif // defined(OS_IOS) } // Convenience method to cast the FullCardRequest into a CardUnmaskDelegate. @@ -1827,26 +1813,18 @@ TEST_F(AutofillManagerTest, WillFillCreditCardNumber) { month_field = &form.fields[i]; } - // Empty form - whole form is Autofilled (except on iOS). + // Empty form - whole form is Autofilled. EXPECT_TRUE(WillFillCreditCardNumber(form, *number_field)); -#if defined(OS_IOS) - EXPECT_FALSE(WillFillCreditCardNumber(form, *name_field)); -#else EXPECT_TRUE(WillFillCreditCardNumber(form, *name_field)); -#endif // defined(OS_IOS) // If the user has entered a value, it won't be overridden. number_field->value = ASCIIToUTF16("gibberish"); EXPECT_TRUE(WillFillCreditCardNumber(form, *number_field)); EXPECT_FALSE(WillFillCreditCardNumber(form, *name_field)); - // But if that value is removed, it will be Autofilled (except on iOS). + // But if that value is removed, it will be Autofilled. number_field->value.clear(); -#if defined(OS_IOS) - EXPECT_FALSE(WillFillCreditCardNumber(form, *name_field)); -#else EXPECT_TRUE(WillFillCreditCardNumber(form, *name_field)); -#endif // defined(OS_IOS) // When part of the section is Autofilled, only fill the initiating field. month_field->is_autofilled = true; diff --git a/components/autofill/core/common/autofill_switches.cc b/components/autofill/core/common/autofill_switches.cc index 42276f839329bb..cf1b859f00d7cd 100644 --- a/components/autofill/core/common/autofill_switches.cc +++ b/components/autofill/core/common/autofill_switches.cc @@ -12,9 +12,6 @@ namespace switches { // credit card form. const char kDisableCreditCardScan[] = "disable-credit-card-scan"; -// Disables the experimental Full Form Autofill on iOS feature. -const char kDisableFullFormAutofillIOS[] = "disable-full-form-autofill-ios"; - // Force hiding the local save checkbox in the autofill dialog box for getting // the full credit card number for a wallet card. The card will never be stored // locally. @@ -36,9 +33,6 @@ const char kDisableSingleClickAutofill[] = "disable-single-click-autofill"; // credit card form. const char kEnableCreditCardScan[] = "enable-credit-card-scan"; -// Enables the experimental Full Form Autofill on iOS feature. -const char kEnableFullFormAutofillIOS[] = "enable-full-form-autofill-ios"; - // Force showing the local save checkbox in the autofill dialog box for getting // the full credit card number for a wallet card. const char kEnableOfferStoreUnmaskedWalletCards[] = diff --git a/components/autofill/core/common/autofill_switches.h b/components/autofill/core/common/autofill_switches.h index 75c7a93fcf22f9..8d419ae0942fc2 100644 --- a/components/autofill/core/common/autofill_switches.h +++ b/components/autofill/core/common/autofill_switches.h @@ -13,13 +13,11 @@ namespace switches { // All switches in alphabetical order. The switches should be documented // alongside the definition of their values in the .cc file. extern const char kDisableCreditCardScan[]; -extern const char kDisableFullFormAutofillIOS[]; extern const char kDisableOfferStoreUnmaskedWalletCards[]; extern const char kDisableOfferUploadCreditCards[]; extern const char kDisablePasswordGeneration[]; extern const char kDisableSingleClickAutofill[]; extern const char kEnableCreditCardScan[]; -extern const char kEnableFullFormAutofillIOS[]; extern const char kEnableOfferStoreUnmaskedWalletCards[]; extern const char kEnableOfferUploadCreditCards[]; extern const char kEnablePasswordGeneration[]; diff --git a/components/autofill/ios/browser/js_autofill_manager.h b/components/autofill/ios/browser/js_autofill_manager.h index a1a797a941faab..6b957ca71e8b3b 100644 --- a/components/autofill/ios/browser/js_autofill_manager.h +++ b/components/autofill/ios/browser/js_autofill_manager.h @@ -23,17 +23,8 @@ completionHandler: (void (^)(NSString*))completionHandler; -// Stores the current active element. This is used to make the element active -// again in case the web view loses focus when a dialog is presented over it. -- (void)storeActiveElement; - -// Clears the current active element. -- (void)clearActiveElement; - // Fills the data in JSON string |dataString| into the active form field, then -// executes the |completionHandler|. The active form field is either -// document.activeElement or the field stored by a call to storeActiveElement. -// non-null. +// executes the |completionHandler|. - (void)fillActiveFormField:(NSString*)dataString completionHandler:(ProceduralBlock)completionHandler; diff --git a/components/autofill/ios/browser/js_autofill_manager.mm b/components/autofill/ios/browser/js_autofill_manager.mm index 84d5a7235d8f92..68aa42d1ce1865 100644 --- a/components/autofill/ios/browser/js_autofill_manager.mm +++ b/components/autofill/ios/browser/js_autofill_manager.mm @@ -34,16 +34,6 @@ - (NSString*)presenceBeacon { return @"__gCrWeb.autofill"; } -- (void)storeActiveElement { - NSString* js = @"__gCrWeb.autofill.storeActiveElement()"; - [self evaluate:js stringResultHandler:nil]; -} - -- (void)clearActiveElement { - NSString* js = @"__gCrWeb.autofill.clearActiveElement()"; - [self evaluate:js stringResultHandler:nil]; -} - - (void)fillActiveFormField:(NSString*)dataString completionHandler:(ProceduralBlock)completionHandler { web::JavaScriptCompletion resultHandler = ^void(NSString*, NSError*) { diff --git a/components/autofill/ios/browser/resources/autofill_controller.js b/components/autofill/ios/browser/resources/autofill_controller.js index a7600892fa6d03..50ec7364308aab 100644 --- a/components/autofill/ios/browser/resources/autofill_controller.js +++ b/components/autofill/ios/browser/resources/autofill_controller.js @@ -136,13 +136,6 @@ __gCrWeb.autofill.ROLE_ATTRIBUTE_PRESENTATION = 0; */ __gCrWeb.autofill.lastAutoFilledElement = null; -/** - * The last element that was active (used to restore focus if necessary). - * - * @type {Element} - */ -__gCrWeb.autofill.lastActiveElement = null; - /** * Whether CSS for autofilled elements has been injected into the page. * @@ -556,34 +549,12 @@ __gCrWeb.autofill['extractForms'] = function(requiredFields) { }; /** - * Stores the current active element. This is used to make the element active - * again in case the web view loses focus when a dialog is presented over it. - */ -__gCrWeb.autofill['storeActiveElement'] = function() { - __gCrWeb.autofill.lastActiveElement = document.activeElement; -} - -/** - * Clears the current active element by setting it to null. - */ -__gCrWeb.autofill['clearActiveElement'] = function() { - __gCrWeb.autofill.lastActiveElement = null; -} - -/** - * Fills data into the active form field. The active form field is either - * document.activeElement or the value of lastActiveElement if that value is - * non-null. + * Fills data into the active form field. * * @param {AutofillFormFieldData} data The data to fill in. */ __gCrWeb.autofill['fillActiveFormField'] = function(data) { var activeElement = document.activeElement; - if (__gCrWeb.autofill.lastActiveElement) { - activeElement = __gCrWeb.autofill.lastActiveElement; - activeElement.focus(); - __gCrWeb.autofill.lastActiveElement = null; - } if (data['name'] !== __gCrWeb['common'].nameForAutofill(activeElement)) { return; } diff --git a/ios/chrome/browser/about_flags.mm b/ios/chrome/browser/about_flags.mm index 9f19234497a032..79a462836cbd95 100644 --- a/ios/chrome/browser/about_flags.mm +++ b/ios/chrome/browser/about_flags.mm @@ -19,7 +19,6 @@ #include "base/strings/stringprintf.h" #include "base/strings/sys_string_conversions.h" #include "base/sys_info.h" -#include "components/autofill/core/common/autofill_switches.h" #include "components/dom_distiller/core/dom_distiller_switches.h" #include "components/flags_ui/feature_entry.h" #include "components/flags_ui/feature_entry_macros.h" @@ -144,14 +143,6 @@ void AppendSwitchesFromExperimentalSettings(base::CommandLine* command_line) { if ([defaults boolForKey:@"EnableCredentialManagement"]) command_line->AppendSwitch(switches::kEnableCredentialManagerAPI); - // Populate command line flags from FullFormAutofill. - NSString* fullFormAutofillValue = [defaults stringForKey:@"FullFormAutofill"]; - if ([fullFormAutofillValue isEqualToString:@"Enabled"]) { - command_line->AppendSwitch(autofill::switches::kEnableFullFormAutofillIOS); - } else if ([fullFormAutofillValue isEqualToString:@"Disabled"]) { - command_line->AppendSwitch(autofill::switches::kDisableFullFormAutofillIOS); - } - NSString* autoReloadEnabledValue = [defaults stringForKey:@"AutoReloadEnabled"]; if ([autoReloadEnabledValue isEqualToString:@"Enabled"]) { diff --git a/ios/chrome/browser/autofill/form_suggestion_controller.mm b/ios/chrome/browser/autofill/form_suggestion_controller.mm index 71a5d783d5b4e5..2499d402ec3c72 100644 --- a/ios/chrome/browser/autofill/form_suggestion_controller.mm +++ b/ios/chrome/browser/autofill/form_suggestion_controller.mm @@ -13,7 +13,6 @@ #include "base/mac/scoped_nsobject.h" #include "base/strings/sys_string_conversions.h" #include "base/strings/utf_string_conversions.h" -#include "components/autofill/core/browser/autofill_field_trial_ios.h" #include "components/autofill/core/browser/autofill_popup_delegate.h" #import "components/autofill/ios/browser/form_suggestion.h" #import "ios/chrome/browser/autofill/form_input_accessory_view_controller.h" @@ -304,12 +303,7 @@ - (void)didSelectSuggestion:(FormSuggestion*)suggestion { forField:base::SysUTF8ToNSString(_suggestionState->field_name) form:base::SysUTF8ToNSString(_suggestionState->form_name) completionHandler:^{ - if (autofill::AutofillFieldTrialIOS::IsFullFormAutofillEnabled()) { - [[weakSelf accessoryViewDelegate] closeKeyboardWithoutButtonPress]; - } else { - [[weakSelf accessoryViewDelegate] - selectNextElementWithoutButtonPress]; - } + [[weakSelf accessoryViewDelegate] closeKeyboardWithoutButtonPress]; }]; _provider = nil; }