mac: Don't show "Access Address Book" prompt if Autofill is disabled.

components/autofill/core/browser/autofill_manager.cc

 // 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/autofill_manager.h"
 
 #include <stddef.h>
 
 #include <limits>
 #include <map>
@@ skipping 274 matching lines @@
 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;
 
+  if (!form_structure->IsAutofillable())

[Evan Stade, 2014/08/28 00:33:16]

why are we even getting to this point if the form isn't autofillable?

[erikchen, 2014/08/28 00:39:37]

I don't know. See the first exchange between isherman and myself. It's quite
possible that the Autofill logic needs a refactor.

+    return false;

[Ilya Sherman, 2014/08/28 00:34:19]

Could you add test coverage for this change as well?

[erikchen, 2014/08/28 17:01:17]

Done.

+
   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)
@@ skipping 200 matching lines @@
         // The first time we show suggestions on this page, log the number of
         // suggestions shown.
         if (!has_logged_address_suggestions_count_ && !section_is_autofilled) {
           metric_logger_->LogAddressSuggestionsCount(values.size());
           has_logged_address_suggestions_count_ = true;
         }
       }
     }
   }
 
   // Add the results from AutoComplete.  They come back asynchronously, so we

[Evan Stade, 2014/08/28 00:33:16]

it doesn't seem like we should be doing this either for autocomplete="off", but
I only see an form_structure->IsAutofillable() above, not here.

[erikchen, 2014/08/28 17:01:17]

I'm probably not the best person to answer this. isherman?

[Evan Stade, 2014/08/28 17:18:58]

ignore this question; <form autocomplete="off"> is indeed handled further up
(i.e. we wouldn't get here)

   // hand off what we generated and they will send the results back to the
   // renderer.
   autocomplete_history_manager_->OnGetAutocompleteSuggestions(
       query_id, field.name, field.value, field.form_control_type, values,
       labels, icons, unique_ids);
 }
 
 void AutofillManager::FillOrPreviewForm(
     AutofillDriver::RendererFormDataAction action,
     int query_id,
@@ skipping 695 matching lines @@
     return false;
 
   // Disregard forms that we wouldn't ever autofill in the first place.
   if (!form.ShouldBeParsed())
     return false;
 
   return true;
 }
 
 }  // namespace autofill