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

If the browser does not yet have permission to access the user's contacts, one
of the Autofill suggestions has the text "Enable Autofill using Contacts". If
the user selects that suggestion, the browser will prompt the user for access
to the user's address book.

The act of clicking the permissions dialog causes Blink to lose its focus on
the text field, which also dismisses the Autofill popup. If the user has
granted Chrome access to the address book, the autofill popup will be presented
again, and filled in.

This CL does not include the image asset, nor does it include the logic to
display the image asset on the left of the autofill entry.

BUG=139154
TEST=Run the command `tccutil reset AddressBook` to reset AddressBook
permissions for all applications. Clear the default Chromium profile:
`rm -rf ~/Library/Application\ Support/Chromium/Default/`.  Launch Chromium by
double clicking on it from Finder.  Navigate to
`https://www.mycontactform.com/samples/change_address.php`. Double click the
"First Name" field. There should not be a prompt to access your "Contacts".
There should be exactly 1 autofill entry: "Enable Autofill using Contacts...".
Clicking that autofill entry should open a system prompt that asks for Contacts
permissions for Chromium.

Review URL: https://codereview.chromium.org/286243002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274040 0039d316-1c4b-4281-b951-d872f2087c98

components/autofill.gypi

@@ -183,6 +183,7 @@
         'autofill/core/browser/phone_number.h',
         'autofill/core/browser/phone_number_i18n.cc',
         'autofill/core/browser/phone_number_i18n.h',
+        'autofill/core/browser/popup_item_ids.h',
         'autofill/core/browser/state_names.cc',
         'autofill/core/browser/state_names.h',
         'autofill/core/browser/validation.cc',

components/autofill/content/browser/content_autofill_driver.cc

@@ -108,6 +108,13 @@ void ContentAutofillDriver::SendFormDataToRenderer(
   }
 }
 
+void ContentAutofillDriver::PingRenderer() {
+  if (!RendererIsAvailable())
+    return;
+  content::RenderViewHost* host = web_contents()->GetRenderViewHost();
+  host->Send(new AutofillMsg_Ping(host->GetRoutingID()));
+}
+
 void ContentAutofillDriver::SendAutofillTypePredictionsToRenderer(
     const std::vector<FormStructure*>& forms) {
   if (!CommandLine::ForCurrentProcess()->HasSwitch(
@@ -181,6 +188,9 @@ bool ContentAutofillDriver::OnMessageReceived(const IPC::Message& message) {
   IPC_MESSAGE_FORWARD(AutofillHostMsg_DidPreviewAutofillFormData,
                       autofill_manager_.get(),
                       AutofillManager::OnDidPreviewAutofillFormData)
+  IPC_MESSAGE_FORWARD(AutofillHostMsg_PingAck,
+                      &autofill_external_delegate_,
+                      AutofillExternalDelegate::OnPingAck)
   IPC_MESSAGE_FORWARD(AutofillHostMsg_DidFillAutofillFormData,
                       autofill_manager_.get(),
                       AutofillManager::OnDidFillAutofillFormData)

components/autofill/content/browser/content_autofill_driver.h

@@ -50,6 +50,7 @@ class ContentAutofillDriver : public AutofillDriver,
   virtual void SendFormDataToRenderer(int query_id,
                                       RendererFormDataAction action,
                                       const FormData& data) OVERRIDE;
+  virtual void PingRenderer() OVERRIDE;
   virtual void SendAutofillTypePredictionsToRenderer(
       const std::vector<FormStructure*>& forms) OVERRIDE;
   virtual void RendererShouldAcceptDataListSuggestion(

components/autofill/content/common/autofill_messages.h

@@ -94,6 +94,11 @@ IPC_ENUM_TRAITS_MAX_VALUE(
 
 // Autofill messages sent from the browser to the renderer.
 
+// Instructs the renderer to immediately return an IPC acknowledging the ping.
+// This is used to correctly sequence events, since IPCs are guaranteed to be
+// processed in order.
+IPC_MESSAGE_ROUTED0(AutofillMsg_Ping)
+
 // Instructs the renderer to fill the active form with the given form data.
 IPC_MESSAGE_ROUTED2(AutofillMsg_FillForm,
                     int /* query_id */,
@@ -227,6 +232,9 @@ IPC_MESSAGE_ROUTED5(AutofillHostMsg_QueryFormFieldAutofill,
 // Sent when a form is previewed with Autofill suggestions.
 IPC_MESSAGE_ROUTED0(AutofillHostMsg_DidPreviewAutofillFormData)
 
+// Sent immediately after the renderer receives a ping IPC.
+IPC_MESSAGE_ROUTED0(AutofillHostMsg_PingAck)
+
 // Sent when a form is filled with Autofill suggestions.
 IPC_MESSAGE_ROUTED1(AutofillHostMsg_DidFillAutofillFormData,
                     base::TimeTicks /* timestamp */)

components/autofill/content/renderer/autofill_agent.cc

@@ -150,6 +150,7 @@ AutofillAgent::~AutofillAgent() {}
 bool AutofillAgent::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(AutofillAgent, message)
+    IPC_MESSAGE_HANDLER(AutofillMsg_Ping, OnPing)
     IPC_MESSAGE_HANDLER(AutofillMsg_FillForm, OnFillForm)
     IPC_MESSAGE_HANDLER(AutofillMsg_PreviewForm, OnPreviewForm)
     IPC_MESSAGE_HANDLER(AutofillMsg_FieldTypePredictionsAvailable,
@@ -454,6 +455,10 @@ void AutofillAgent::OnFillForm(int query_id, const FormData& form) {
                                                    base::TimeTicks::Now()));
 }
 
+void AutofillAgent::OnPing() {
+  Send(new AutofillHostMsg_PingAck(routing_id()));
+}
+
 void AutofillAgent::OnPreviewForm(int query_id, const FormData& form) {
   if (!render_view()->GetWebView() || query_id != autofill_query_id_)
     return;

components/autofill/content/renderer/autofill_agent.h

@@ -97,6 +97,7 @@ class AutofillAgent : public content::RenderViewObserver,
   void OnFieldTypePredictionsAvailable(
       const std::vector<FormDataPredictions>& forms);
   void OnFillForm(int query_id, const FormData& form);
+  void OnPing();
   void OnPreviewForm(int query_id, const FormData& form);
 
   // For external Autofill selection.

components/autofill/core/browser/autofill_driver.h

@@ -58,6 +58,9 @@ class AutofillDriver {
                                       RendererFormDataAction action,
                                       const FormData& data) = 0;
 
+  // Pings renderer. The renderer will return an IPC acknowledging the ping.
+  virtual void PingRenderer() = 0;
+
   // Sends the field type predictions specified in |forms| to the renderer. This
   // method is a no-op if the renderer is not available or the appropriate
   // command-line flag is not set.
@@ -81,7 +84,6 @@ class AutofillDriver {
   // Tells the renderer to preview the node with suggested text.
   virtual void RendererShouldPreviewFieldWithValue(
       const base::string16& value) = 0;
-
 };
 
 }  // namespace autofill

components/autofill/core/browser/autofill_external_delegate.cc

@@ -4,6 +4,7 @@
 
 #include "components/autofill/core/browser/autofill_external_delegate.h"
 
+#include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autocomplete_history_manager.h"
 #include "components/autofill/core/browser/autofill_driver.h"
@@ -14,9 +15,8 @@
 
 namespace autofill {
 
-AutofillExternalDelegate::AutofillExternalDelegate(
-    AutofillManager* manager,
-    AutofillDriver* driver)
+AutofillExternalDelegate::AutofillExternalDelegate(AutofillManager* manager,
+                                                   AutofillDriver* driver)
     : manager_(manager),
       driver_(driver),
       query_id_(0),
@@ -91,6 +91,18 @@ void AutofillExternalDelegate::OnSuggestionsReturned(
   // updated to match.
   InsertDataListValues(&values, &labels, &icons, &ids);
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  if (values.empty() &&
+      manager_->ShouldShowAccessAddressBookSuggestion(query_form_,
+                                                      query_field_)) {
+    values.push_back(
+        l10n_util::GetStringUTF16(IDS_AUTOFILL_ACCESS_MAC_CONTACTS));
+    labels.push_back(base::string16());
+    icons.push_back(base::string16());
+    ids.push_back(POPUP_ITEM_ID_MAC_ACCESS_CONTACTS);
+  }
+#endif
+
   if (values.empty()) {
     // No suggestions, any popup currently showing is obsolete.
     manager_->delegate()->HideAutofillPopup();
@@ -157,6 +169,38 @@ void AutofillExternalDelegate::DidAcceptSuggestion(const base::string16& value,
   } else if (identifier == POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY) {
     // User selected an Autocomplete, so we fill directly.
     driver_->RendererShouldFillFieldWithValue(value);
+  } else if (identifier == POPUP_ITEM_ID_MAC_ACCESS_CONTACTS) {
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+    // User wants to give Chrome access to user's address book.
+    manager_->AccessAddressBook();
+
+    // There is no deterministic method for deciding whether a blocking dialog
+    // was presented. The following comments and code assume that a blocking
+    // dialog was presented, but still behave correctly if no dialog was
+    // presented.
+
+    // A blocking dialog was presented, and the user has already responded to
+    // the dialog. The presentation of the dialog added an NSEvent to the
+    // NSRunLoop which will cause all windows to lose focus. When the NSEvent
+    // is processed, it will be sent to the renderer which will cause the text
+    // field to lose focus. This returns an IPC to Chrome which will dismiss
+    // the autofill popup. We post a task which we expect to run after the
+    // NSEvent has been processed by the NSRunLoop. It pings the renderer,
+    // which returns an IPC acknowledging the ping.  At that time, redisplay
+    // the popup. FIFO processing of IPCs ensures that all side effects of the
+    // NSEvent will have been processed.
+
+    // 10ms sits nicely under the 16ms threshold for 60 fps, and likely gives
+    // the NSApplication run loop sufficient time to process the NSEvent. In
+    // testing, a delay of 0ms was always sufficient.
+    base::TimeDelta delay(base::TimeDelta::FromMilliseconds(10));
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&AutofillExternalDelegate::PingRenderer, GetWeakPtr()),
+        delay);
+#else
+    NOTREACHED();
+#endif
   } else {
     FillAutofillFormData(identifier, false);
   }
@@ -186,6 +230,15 @@ void AutofillExternalDelegate::Reset() {
   manager_->delegate()->HideAutofillPopup();
 }
 
+void AutofillExternalDelegate::OnPingAck() {
+  // Reissue the most recent query, which will reopen the autofill popup.
+  manager_->OnQueryFormFieldAutofill(query_id_,
+                                     query_form_,
+                                     query_field_,
+                                     element_bounds_,
+                                     display_warning_if_disabled_);
+}
+
 base::WeakPtr<AutofillExternalDelegate> AutofillExternalDelegate::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
@@ -308,4 +361,10 @@ void AutofillExternalDelegate::InsertDataListValues(
                      POPUP_ITEM_ID_DATALIST_ENTRY);
 }
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+void AutofillExternalDelegate::PingRenderer() {
+  driver_->PingRenderer();
+}
+#endif
+
 }  // namespace autofill

components/autofill/core/browser/autofill_external_delegate.h

@@ -80,6 +80,9 @@ class AutofillExternalDelegate
   // values or settings.
   void Reset();
 
+  // The renderer sent an IPC acknowledging an earlier ping IPC.
+  void OnPingAck();
+
  protected:
   base::WeakPtr<AutofillExternalDelegate> GetWeakPtr();
 
@@ -111,6 +114,11 @@ class AutofillExternalDelegate
                             std::vector<base::string16>* icons,
                             std::vector<int>* unique_ids);
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // Pings the renderer.
+  void PingRenderer();
+#endif
+
   AutofillManager* manager_;  // weak.
 
   // Provides driver-level context to the shared code of the component. Must

components/autofill/core/browser/autofill_manager.cc

@@ -240,6 +240,28 @@ void AutofillManager::ShowAutofillSettings() {
   manager_delegate_->ShowAutofillSettings();
 }
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+bool AutofillManager::ShouldShowAccessAddressBookSuggestion(
+    const FormData& form,
+    const FormFieldData& field) {
+  if (!personal_data_)
+    return false;
+  FormStructure* form_structure = NULL;
+  AutofillField* autofill_field = NULL;
+  if (!GetCachedFormAndField(form, field, &form_structure, &autofill_field))
+    return false;
+
+  return personal_data_->ShouldShowAccessAddressBookSuggestion(
+      autofill_field->Type());
+}
+
+bool AutofillManager::AccessAddressBook() {
+  if (!personal_data_)
+    return false;
+  return personal_data_->AccessAddressBook();
+}
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
+
 bool AutofillManager::OnFormSubmitted(const FormData& form,
                                       const TimeTicks& timestamp) {
   if (!IsValidFormData(form))

components/autofill/core/browser/autofill_manager.h

@@ -84,6 +84,19 @@ class AutofillManager : public AutofillDownloadManager::Observer {
 
   void ShowAutofillSettings();
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // Whether the field represented by |fieldData| should show an entry to prompt
+  // the user to give Chrome access to the user's address book.
+  bool ShouldShowAccessAddressBookSuggestion(const FormData& data,
+                                             const FormFieldData& field_data);
+
+  // If Chrome has not prompted for access to the user's address book, the
+  // method prompts the user for permission and blocks the process. Otherwise,
+  // this method has no effect. The return value reflects whether the user was
+  // prompted with a modal dialog.
+  bool AccessAddressBook();
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
+
   // Called from our external delegate so they cannot be private.
   virtual void FillOrPreviewForm(AutofillDriver::RendererFormDataAction action,
                                  int query_id,

components/autofill/core/browser/personal_data_manager.h

@@ -200,6 +200,18 @@ class PersonalDataManager : public KeyedService,
   // will only update when Chrome is restarted.
   virtual const std::string& GetDefaultCountryCodeForNewAddress() const;
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // If Chrome has not prompted for access to the user's address book, the
+  // method prompts the user for permission and blocks the process. Otherwise,
+  // this method has no effect. The return value reflects whether the user was
+  // prompted with a modal dialog.
+  bool AccessAddressBook();
+
+  // Whether an autofill suggestion should be displayed to prompt the user to
+  // grant Chrome access to the user's address book.
+  bool ShouldShowAccessAddressBookSuggestion(AutofillType type);
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
+
  protected:
   // Only PersonalDataManagerFactory and certain tests can create instances of
   // PersonalDataManager.

components/autofill/core/browser/personal_data_manager_mac.mm

@@ -21,8 +21,10 @@
 #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/components_strings.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 
@@ -31,6 +33,33 @@ namespace {
 
 const char kAddressBookOrigin[] = "OS X Address Book";
 
+// 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);
+}
+
+ABAddressBook* GetAddressBook(PrefService* pref_service) {
+  bool first_access = !HasPromptedForAccessToAddressBook(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);
+
+  if (first_access) {
+    UMA_HISTOGRAM_BOOLEAN("Autofill.AddressBookAvailableOnFirstAttempt",
+                          addressBook != nil);
+  }
+
+  pref_service->SetBoolean(prefs::kAutofillAuxiliaryProfilesQueried, true);
+  return addressBook;
+}
+
 // 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
@@ -81,20 +110,13 @@ 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)
@@ -281,4 +303,43 @@ void PersonalDataManager::LoadAuxiliaryProfiles() const {
   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.
+  GetAddressBook(pref_service_);
+  return true;
+}
+
+bool PersonalDataManager::ShouldShowAccessAddressBookSuggestion(
+    AutofillType type) {
+  if (!pref_service_->GetBoolean(prefs::kAutofillAuxiliaryProfilesEnabled))
+    return false;
+
+  if (HasPromptedForAccessToAddressBook(pref_service_))
+    return false;
+
+  switch (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

components/autofill/core/browser/popup_item_ids.h

@@ -16,7 +16,8 @@ enum PopupItemId {
   POPUP_ITEM_ID_SEPARATOR = -3,
   POPUP_ITEM_ID_CLEAR_FORM = -4,
   POPUP_ITEM_ID_AUTOFILL_OPTIONS = -5,
-  POPUP_ITEM_ID_DATALIST_ENTRY = -6
+  POPUP_ITEM_ID_DATALIST_ENTRY = -6,
+  POPUP_ITEM_ID_MAC_ACCESS_CONTACTS = -7,
 };
 
 }  // namespace autofill

components/autofill/core/browser/test_autofill_driver.cc

@@ -39,6 +39,8 @@ void TestAutofillDriver::SendFormDataToRenderer(int query_id,
                                                 const FormData& form_data) {
 }
 
+void TestAutofillDriver::PingRenderer() {}
+
 void TestAutofillDriver::SendAutofillTypePredictionsToRenderer(
     const std::vector<FormStructure*>& forms) {
 }

components/autofill/core/browser/test_autofill_driver.h

@@ -32,6 +32,7 @@ class TestAutofillDriver : public AutofillDriver {
   virtual void SendFormDataToRenderer(int query_id,
                                       RendererFormDataAction action,
                                       const FormData& data) OVERRIDE;
+  virtual void PingRenderer() OVERRIDE;
   virtual void SendAutofillTypePredictionsToRenderer(
       const std::vector<FormStructure*>& forms) OVERRIDE;
   virtual void RendererShouldAcceptDataListSuggestion(

components/autofill_strings.grdp

@@ -110,4 +110,11 @@
     This type of card is not supported by Google Wallet for this merchant. Please select a different card.
   </message>
 
+  <!-- Autofill on OSX -->
+  <if expr="is_macosx">
+    <message name="IDS_AUTOFILL_ACCESS_MAC_CONTACTS" desc="When the user selects this Autofill entry, the browser prompts the user for access to the user's OSX contacts.">
+      Enable Autofill using Contacts…
+    </message>
+  </if>
+
 </grit-part>