Mac: Autofill should not immediately request access to address book.

components/autofill/core/browser/personal_data_manager_mac.mm

 // Copyright 2013 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/personal_data_manager.h"
 
 #include <math.h>
 
 #import <AddressBook/AddressBook.h>
 
 #include "base/format_macros.h"
 #include "base/guid.h"
 #include "base/logging.h"
 #import "base/mac/scoped_nsexception_enabler.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
 #include "components/autofill/core/browser/autofill_country.h"
 #include "components/autofill/core/browser/autofill_profile.h"
 #include "components/autofill/core/browser/autofill_type.h"
+#include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/phone_number.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
+#include "components/autofill/core/common/form_data.h"
 #include "grit/component_strings.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 
 namespace autofill {
 namespace {
 
 const char kAddressBookOrigin[] = "OS X Address Book";
 
+ABAddressBook* GetAddressBook(PrefService* pref_service) {
+  // +[ABAddressBook sharedAddressBook] throws an exception internally in
+  // circumstances that aren't clear. The exceptions are only observed in crash
+  // reports, so it is unknown whether they would be caught by AppKit and nil
+  // returned, or if they would take down the app. In either case, avoid
+  // crashing. http://crbug.com/129022
+  ABAddressBook* addressBook = base::mac::RunBlockIgnoringExceptions(
+      ^{ return [ABAddressBook sharedAddressBook]; });
+  UMA_HISTOGRAM_BOOLEAN("Autofill.AddressBookAvailable", addressBook != nil);
+  pref_service->SetBoolean(prefs::kAutofillAuxiliaryProfilesQueried, true);
+  return addressBook;
+}
+
+// Whether Chrome has prompted the user for permission to access the user's
+// address book.
+bool HasPromptedForAccessToAddressBook(PrefService* pref_service) {
+  return pref_service->GetBoolean(prefs::kAutofillAuxiliaryProfilesQueried);
+}
+
 // This implementation makes use of the Address Book API.  Profiles are
 // generated that correspond to addresses in the "me" card that reside in the
 // user's Address Book.  The caller passes a vector of profiles into the
 // the constructer and then initiate the fetch from the Mac Address Book "me"
 // card using the main |GetAddressBookMeCard()| method.  This clears any
 // existing addresses and populates new addresses derived from the data found
 // in the "me" card.
 class AuxiliaryProfilesImpl {
  public:
   // Constructor takes a reference to the |profiles| that will be filled in
@@ skipping 30 matching lines @@
 };
 
 // This method uses the |ABAddressBook| system service to fetch the "me" card
 // from the active user's address book.  It looks for the user address
 // information and translates it to the internal list of |AutofillProfile| data
 // structures.
 void AuxiliaryProfilesImpl::GetAddressBookMeCard(const std::string& app_locale,
                                                  PrefService* pref_service) {
   profiles_.clear();
 
-  // +[ABAddressBook sharedAddressBook] throws an exception internally in
-  // circumstances that aren't clear. The exceptions are only observed in crash
-  // reports, so it is unknown whether they would be caught by AppKit and nil
-  // returned, or if they would take down the app. In either case, avoid
-  // crashing. http://crbug.com/129022
-  ABAddressBook* addressBook = base::mac::RunBlockIgnoringExceptions(^{
-      return [ABAddressBook sharedAddressBook];
-  });
-  UMA_HISTOGRAM_BOOLEAN("Autofill.AddressBookAvailable", addressBook != nil);
-  if (!pref_service->GetBoolean(prefs::kAutofillAuxiliaryProfilesQueried)) {
-    pref_service->SetBoolean(prefs::kAutofillAuxiliaryProfilesQueried, true);
-    UMA_HISTOGRAM_BOOLEAN("Autofill.AddressBookAvailableOnFirstAttempt",
-                          addressBook != nil);
-  }
+  // Chrome has not yet requested address book permissions. Attempting to do so
+  // presents a blocking modal dialog, which is undesirable. Instead, just show
+  // no results.
+  if (!HasPromptedForAccessToAddressBook(pref_service))
+    return;
+
+  ABAddressBook* addressBook = GetAddressBook(pref_service);
 
   ABPerson* me = [addressBook me];
   if (!me)
     return;
 
   ABMultiValue* addresses = [me valueForProperty:kABAddressProperty];
 
   // The number of characters at the end of the GUID to reserve for
   // distinguishing addresses within the "me" card.  Cap the number of addresses
   // we will fetch to the number that can be distinguished by this fragment of
@@ skipping 166 matching lines @@
 }
 
 }  // namespace
 
 // Populate |auxiliary_profiles_| with the Address Book data.
 void PersonalDataManager::LoadAuxiliaryProfiles() const {
   AuxiliaryProfilesImpl impl(&auxiliary_profiles_);
   impl.GetAddressBookMeCard(app_locale_, pref_service_);
 }
 
+bool PersonalDataManager::AccessAddressBook() {
+  if (!pref_service_->GetBoolean(prefs::kAutofillAuxiliaryProfilesEnabled))
+    return false;
+
+  if (HasPromptedForAccessToAddressBook(pref_service_))
+    return false;
+
+  // Request permissions.
+  ABAddressBook* addressBook = GetAddressBook(pref_service_);

[erikchen, 2014/05/21 22:00:54]

It's possible for a user to mess with the configuration file, which would cause
this method to not present a blocking modal dialog. There's no good way to
detect if this has happened. I considered grabbing a timestamp before and after
this API call to try to heuristically determine whether a dialog was presented,
but decided that that would cause more problems than it solved.

+
+  UMA_HISTOGRAM_BOOLEAN("Autofill.AddressBookAvailableOnFirstAttempt",
+                        addressBook != nil);

[Ilya Sherman, 2014/05/22 15:13:59]

I'd prefer for this to be in GetAddressBook() as well, for the same reason.

[erikchen, 2014/05/22 20:40:59]

Done.

+  return true;
+}
+
+bool PersonalDataManager::ShouldShowAccessAddressBookSuggestion(
+    const FormData& data,
+    const FormFieldData& field_data,
+    const ScopedVector<FormStructure>& form_structures) {
+  if (!pref_service_->GetBoolean(prefs::kAutofillAuxiliaryProfilesEnabled))
+    return false;
+
+  if (HasPromptedForAccessToAddressBook(pref_service_))
+    return false;
+
+  for (ScopedVector<FormStructure>::const_iterator it = form_structures.begin();
+       it != form_structures.end();
+       ++it) {
+    // The form does not match.
+    if ((*it)->ToFormData().name != data.name)
+      continue;
+    for (size_t index = 0; index < (*it)->field_count(); ++index) {
+      const AutofillField* field = (*it)->field(index);
+      // The field does not match.
+      if (field->name != field_data.name)
+        continue;
+
+      switch (field->Type().group()) {
+        case ADDRESS_BILLING:
+        case ADDRESS_HOME:
+        case EMAIL:
+        case NAME:
+        case NAME_BILLING:
+        case PHONE_BILLING:
+        case PHONE_HOME:
+          return true;
+        case NO_GROUP:
+        case COMPANY:
+        case CREDIT_CARD:
+        case PASSWORD_FIELD:
+          return false;
+      }
+    }
+  }
+
+  return false;
+}
+
 }  // namespace autofill