Added personal_data_manager android implementation for auto-populating auto-fill on android builds …

chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalAutofillPopulator.java

Index: chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalAutofillPopulator.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalAutofillPopulator.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalAutofillPopulator.java
new file mode 100644
index 0000000000000000000000000000000000000000..9fee0efc9a85ef2f82353c6456b125f187a2301f
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalAutofillPopulator.java
@@ -0,0 +1,295 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[aurimas (slooooooooow), 2013/02/15 19:08:35]

2013

[apiccion, 2013/02/26 23:51:51]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Populates data fields from Android contacts profile API (i.e. "me" contact).

[aurimas (slooooooooow), 2013/02/15 19:08:35]

We do not use // style comments in Java that much. Move this to the /* */ style
comment above the class definition

[apiccion, 2013/02/26 23:51:51]

Done.

+
+package org.chromium.chrome.browser.autofill;
+
+import org.chromium.base.CalledByNative;
+
+import java.util.ArrayList;
+
+import android.app.Activity;

[aurimas (slooooooooow), 2013/02/15 19:08:35]

There imports should be in alpha oder with no line breaks in between android.*

[apiccion, 2013/02/26 23:51:51]

Done.

+import android.content.ContentProviderOperation;
+import android.content.ContentResolver;
+import android.content.OperationApplicationException;
+import android.database.Cursor;
+
+import android.os.Bundle;
+import android.os.RemoteException;
+import android.provider.ContactsContract;
+import android.provider.ContactsContract.Profile;
+import android.provider.ContactsContract.CommonDataKinds.Phone;
+
+import android.util.Log;
+import android.view.View;
+import android.view.View.OnClickListener;
+import android.widget.Button;
+import android.widget.Toast;
+import android.net.Uri;
+
+import android.content.Context;
+import android.database.DatabaseUtils;
+
+/**
+ * Loads user profile information stored under the "Me" contact

[aurimas (slooooooooow), 2013/02/15 19:08:35]

Put periods at the end of the end of your comments.

[apiccion, 2013/02/26 23:51:51]

Done.

+ */
+public class PersonalAutofillPopulator {
+  /**
+     SQL query definitions for obtaining specific profile information

[aurimas (slooooooooow), 2013/02/15 19:08:35]

Missing a star for this line

[apiccion, 2013/02/26 23:51:51]

Done.

+  */
+  private abstract static class ProfileQuery {
+    Uri profileDataUri = Uri.withAppendedPath(
+      ContactsContract.Profile.CONTENT_URI, ContactsContract.Contacts.Data.CONTENT_DIRECTORY
+    );
+    public abstract String[] projection();
+    public abstract String mimeType();
+  }
+
+  private static class EmailProfileQuery extends ProfileQuery {
+      public static final int EMAIL_ADDRESS = 0;
+      @Override

[aurimas (slooooooooow), 2013/02/15 19:08:35]

Should put an empty line above @Override

[apiccion, 2013/02/26 23:51:51]

Done.

+      public String[] projection() {
+        return new String[] {
+          ContactsContract.CommonDataKinds.Email.ADDRESS,
+        };
+      }
+
+      @Override
+      public String mimeType() {
+        return ContactsContract.CommonDataKinds.Email.CONTENT_ITEM_TYPE;
+      }
+  }
+
+  private static class PhoneProfileQuery extends ProfileQuery {
+    public static final int NUMBER = 0;
+
+    @Override
+    public String[] projection() {
+      return new String[] {
+        ContactsContract.CommonDataKinds.Phone.NUMBER,
+       };
+    }
+
+    @Override
+    public String mimeType() {
+      return ContactsContract.CommonDataKinds.Phone.CONTENT_ITEM_TYPE;
+    }
+  }
+
+  private static class AddressProfileQuery extends ProfileQuery {
+    public static final int STREET = 0;

[aurimas (slooooooooow), 2013/02/15 19:08:35]

Are these contants defined somewhere in Android so we would not have to redefine
it? Maybe use DATA1, DATA2, DATA3, ... define in ContactsContract.java

[David Trainor- moved to gerrit, 2013/02/19 19:03:13]

It looks like DATA1/DATA2/etc. are strings?  I think these just correlate to
which column in his query below (the String[] map) his result will be in.

On 2013/02/15 19:08:35, aurimas wrote:

[apiccion, 2013/02/26 23:51:51]

Not defined in Android. They correspond to the order of entries in the
projection array. 

On 2013/02/15 19:08:35, aurimas wrote:

+    public static final int POBOX = 1;
+    public static final int NEIGHBORHOOD = 2;
+    public static final int CITY = 3;
+    public static final int REGION= 4;

[David Trainor- moved to gerrit, 2013/02/19 19:03:13]

space before =

[apiccion, 2013/02/26 23:51:51]

Done.

+    public static final int POSTALCODE = 5;
+    public static final int COUNTRY = 6;
+
+    @Override
+    public String[] projection() {
+      return new String[] {
+        ContactsContract.CommonDataKinds.StructuredPostal.STREET,
+        ContactsContract.CommonDataKinds.StructuredPostal.POBOX,
+        ContactsContract.CommonDataKinds.StructuredPostal.NEIGHBORHOOD,
+        ContactsContract.CommonDataKinds.StructuredPostal.CITY,
+        ContactsContract.CommonDataKinds.StructuredPostal.REGION,
+        ContactsContract.CommonDataKinds.StructuredPostal.POSTCODE,
+        ContactsContract.CommonDataKinds.StructuredPostal.COUNTRY,
+      };
+    }
+
+    public String mimeType() {
+      return ContactsContract.CommonDataKinds.StructuredPostal.CONTENT_ITEM_TYPE;
+    }
+  }
+
+  private static class NameProfileQuery extends ProfileQuery {
+    public static final int GIVEN_NAME = 0;
+    public static final int MIDDLE_NAME = 1;
+    public static final int FAMILY_NAME = 2;
+    public static final int DISPLAY_NAME = 3;
+    public static final int SUFFIX = 4;
+
+    public String[] projection() {
+      return new String[] {
+        ContactsContract.CommonDataKinds.StructuredName.GIVEN_NAME,
+        ContactsContract.CommonDataKinds.StructuredName.MIDDLE_NAME,
+        ContactsContract.CommonDataKinds.StructuredName.FAMILY_NAME,
+        ContactsContract.CommonDataKinds.StructuredName.DISPLAY_NAME,
+        ContactsContract.CommonDataKinds.StructuredName.SUFFIX
+      };
+    }
+
+    public String mimeType() {
+      return ContactsContract.CommonDataKinds.StructuredName.CONTENT_ITEM_TYPE;
+    }
+  }
+
+  /** Takes a query object, transforms into actual query and returns cursor.

[David Trainor- moved to gerrit, 2013/02/19 19:03:13]

I think we leave the line with /** blank and start the description on the second
line for all /** comments.

+     Primary contact values will be first.
+   */
+  private Cursor cursorFromProfileQuery(ProfileQuery query, ContentResolver contentResolver) {
+    String sortDescriptor = ContactsContract.Contacts.Data.IS_PRIMARY + " DESC";
+    return contentResolver.query(
+      query.profileDataUri,
+      query.projection(),
+      ContactsContract.Contacts.Data.MIMETYPE + " = ?",
+      new String[]{query.mimeType()},
+      sortDescriptor
+    );
+  }
+  /* Extracted data variables */

[aurimas (slooooooooow), 2013/02/15 19:08:35]

For one line comments use //

+  private String[] mEmailAddresses;
+  private String mGivenName;
+  private String mMiddleName;
+  private String mFamilyName;
+  private String mDisplayName;
+  private String mSuffix;
+  private String address;

[Ilya Sherman, 2013/02/16 04:14:37]

nit: Should this be mAddress?

[apiccion, 2013/02/26 23:51:51]

Actually dead code. Will remove.

+  private String mPobox;
+  private String mStreet;
+  private String mNeighborhood;
+  private String mCity;
+  private String mRegion;
+  private String mCountry;
+  private String mPostalCode;
+  private String[] mPhoneNumbers;
+
+  /** Constructor
+    @param context a valid android context reference

[aurimas (slooooooooow), 2013/02/15 19:08:35]

Missing a star. Update it for every /** */ comment.

[apiccion, 2013/02/26 23:51:51]

Done.

+   */
+  PersonalAutofillPopulator(Context context) {
+    ContentResolver contentResolver = context.getContentResolver();
+    populateName(contentResolver);
+    populateEmail(contentResolver);
+    populateAddress(contentResolver);
+    populatePhone(contentResolver);
+  }
+
+  /* Populating data fields */
+  private void populateName(ContentResolver contentResolver) {

[David Trainor- moved to gerrit, 2013/02/19 19:03:13]

Should we have error handling here?  I'm wondering what happens if no me contact
is defined (is that possible?).  If not, we should still add some kind of assert
or check in these methods making sure we have a valid row.

+      NameProfileQuery nameProfileQuery = new NameProfileQuery();
+      Cursor nameCursor = cursorFromProfileQuery(nameProfileQuery, contentResolver);
+      nameCursor.moveToFirst();

[David Trainor- moved to gerrit, 2013/02/19 19:03:13]

Do you need this here?  If so, should it be in every populate* method?

[apiccion, 2013/02/26 23:51:51]

Actually a crash-bug because cursor could be empty. Either one of moveToFirst()
or moveToNext() is sufficient. The cursor starts at index -1. 

On 2013/02/19 19:03:13, David Trainor wrote:

+      mGivenName = nameCursor.getString(nameProfileQuery.GIVEN_NAME);
+      mMiddleName = nameCursor.getString(nameProfileQuery.MIDDLE_NAME);
+      mFamilyName = nameCursor.getString(nameProfileQuery.FAMILY_NAME);
+      mDisplayName = nameCursor.getString(nameProfileQuery.DISPLAY_NAME);
+      mSuffix = nameCursor.getString(nameProfileQuery.SUFFIX);
+      nameCursor.close();
+  }
+
+  private void populateEmail(ContentResolver contentResolver) {
+    EmailProfileQuery emailProfileQuery = new EmailProfileQuery();
+    Cursor emailCursor = cursorFromProfileQuery(emailProfileQuery, contentResolver);
+    mEmailAddresses = new String[emailCursor.getCount()];
+    for (int i = 0; emailCursor.moveToNext(); i++) {
+      mEmailAddresses[i] = emailCursor.getString(emailProfileQuery.EMAIL_ADDRESS);
+    }
+    emailCursor.close();
+  }
+
+  private void populateAddress(ContentResolver contentResolver) {
+    AddressProfileQuery addressProfileQuery = new AddressProfileQuery();
+    Cursor addressCursor = cursorFromProfileQuery(addressProfileQuery, contentResolver);
+    addressCursor.moveToFirst();
+    mPobox = addressCursor.getString(addressProfileQuery.POBOX);
+    mStreet = addressCursor.getString(addressProfileQuery.STREET);
+    mNeighborhood = addressCursor.getString(addressProfileQuery.NEIGHBORHOOD);
+    mCity = addressCursor.getString(addressProfileQuery.CITY);
+    mRegion = addressCursor.getString(addressProfileQuery.REGION);
+    mPostalCode = addressCursor.getString(addressProfileQuery.POSTALCODE);
+    mCountry = addressCursor.getString(addressProfileQuery.COUNTRY);
+    addressCursor.close();
+  }
+
+  private void populatePhone(ContentResolver contentResolver) {
+    PhoneProfileQuery phoneProfileQuery = new PhoneProfileQuery();
+    Cursor phoneCursor = cursorFromProfileQuery(phoneProfileQuery, contentResolver);
+    mPhoneNumbers = new String[phoneCursor.getCount()];
+    for (int i = 0; phoneCursor.moveToNext(); i++) {
+      mPhoneNumbers[i] = phoneCursor.getString(phoneProfileQuery.NUMBER);
+    }
+    phoneCursor.close();
+  }
+
+  /** Static factory method for instance creation
+      @param context valid Android context
+  */
+  @CalledByNative
+  static PersonalAutofillPopulator create(Context context) {
+    return new PersonalAutofillPopulator(context);
+  }
+
+  @CalledByNative
+  public String getFirstName() {

[aurimas (slooooooooow), 2013/02/15 19:08:35]

Can't all of these methods be private since they are called from native code
only?

[David Trainor- moved to gerrit, 2013/02/19 19:03:13]

Yeah they can.  If they're not, they need javadocs!
Maybe leave them public and add a test to make sure we're pulling the right
data?

[apiccion, 2013/02/26 23:51:51]

I think the distinction of public/private should reflect the intent of the
component interactions (e.g. object interactions).

On 2013/02/15 19:08:35, aurimas wrote:

+    return mGivenName;
+  }
+
+  @CalledByNative
+  public String getLastName() {
+    return mFamilyName;
+  }
+
+  @CalledByNative
+  public String getMiddleName() {
+    return mMiddleName;
+  }
+
+  @CalledByNative
+  public String getSuffix() {
+    return mSuffix;
+  }
+
+  @CalledByNative
+  public String getFullName() {
+    return mDisplayName;
+  }
+
+  @CalledByNative
+  public String[] getEmailAddresses() {
+    return mEmailAddresses;
+  }
+
+  @CalledByNative
+  public String getStreet() {
+    return mStreet;
+  }
+
+  @CalledByNative
+  public String getPobox() {
+    return mPobox;
+  }
+
+  @CalledByNative
+  public String getNeighborhood() {
+    return mNeighborhood;
+  }
+
+  @CalledByNative
+  public String getCity() {
+    return mCity;
+  }
+
+  @CalledByNative
+  public String getRegion() {
+    return mRegion;
+  }
+
+  @CalledByNative
+  public String getPostalCode() {
+    return mPostalCode;
+  }
+
+  @CalledByNative
+  public String getCountry() {
+    return mCountry;
+  }
+
+  @CalledByNative
+  public String[] getPhoneNumbers() {
+    return mPhoneNumbers;
+  }
+}