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

components/autofill/core/browser/autofill_external_delegate.cc

Index: components/autofill/core/browser/autofill_external_delegate.cc
diff --git a/components/autofill/core/browser/autofill_external_delegate.cc b/components/autofill/core/browser/autofill_external_delegate.cc
index 95cf8a14a1aff8ea0f06f75969d6685ee4919d12..4f791534c9333a78b246167db23b8ebedaca9459 100644
--- a/components/autofill/core/browser/autofill_external_delegate.cc
+++ b/components/autofill/core/browser/autofill_external_delegate.cc
@@ -14,15 +14,17 @@
 
 namespace autofill {
 
-AutofillExternalDelegate::AutofillExternalDelegate(
-    AutofillManager* manager,
-    AutofillDriver* driver)
+AutofillExternalDelegate::AutofillExternalDelegate(AutofillManager* manager,
+                                                   AutofillDriver* driver)
     : manager_(manager),
       driver_(driver),
       query_id_(0),
       display_warning_if_disabled_(false),
       has_suggestion_(false),
       has_shown_popup_for_current_edit_(false),
+#if defined(OS_MACOSX)
+      redisplay_autofill_(false),
+#endif
       weak_ptr_factory_(this) {
   DCHECK(manager);
 }
@@ -91,6 +93,18 @@ void AutofillExternalDelegate::OnSuggestionsReturned(
   // updated to match.
   InsertDataListValues(&values, &labels, &icons, &ids);
 
+#if defined(OS_MACOSX)
+  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 +171,15 @@ 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)
+    // User wants to give Chrome access to user's address book.
+    bool modal_presented = manager_->AccessAddressBook();
+    if (modal_presented)
+      redisplay_autofill_ = true;

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

Why does it matter whether the modal dialog was presented?  If the user was able
to click on the relevant entry in the popup, then we should re-issue the query,
even if this particular click wasn't the cause of access being granted.

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

It's important to not set the redisplay_popop_on_focus_lost_ flag if a modal was
not presented, as that would cause an undesired popup redisplay. I've added
logic to immediately refresh the popup if a modal was not presented.

+#else
+    NOTREACHED();
+#endif
   } else {
     FillAutofillFormData(identifier, false);
   }
@@ -176,6 +199,22 @@ void AutofillExternalDelegate::DidEndTextFieldEditing() {
   manager_->delegate()->HideAutofillPopup();
 
   has_shown_popup_for_current_edit_ = false;
+
+#if defined(OS_MACOSX)
+  // The user was presented a modal dialog, which they had to interact with
+  // before the Chrome process could resume. Their interaction is guaranteed to
+  // cause the text field to lose focus. They were either granting or denying
+  // Chrome access to their address book, so immediately try to redisplay the
+  // autofill popup.
+  if (redisplay_autofill_) {
+    manager_->OnQueryFormFieldAutofill(query_id_,
+                                       query_form_,
+                                       query_field_,
+                                       element_bounds_,
+                                       display_warning_if_disabled_);
+    redisplay_autofill_ = false;
+  }
+#endif
 }
 
 void AutofillExternalDelegate::ClearPreviewedForm() {