Bug 578691 adds most of the infrastructure to support showing contact information in form autocomplete. We need to add a data provider for Android.

flagging for fennec 2.0 to get on triage plate

forgot to add the new files

Comment on attachment 495542 [details] [diff] [review] m-b patch >-#ifndef ANDROID >+#ifdef ANDROID >+function AndroidContactsProvider() { >+} >+AndroidContactsProvider.prototype = { nit: Add a blank line >+ getContacts: function() { >+ return JSON.parse(Cc["@mozilla.org/dom/contacts;1"].getService(Ci.nsIContacts).contacts); As I mentioned on IRC, since this is going in platform, I think .getContactsAsJSON() might be better. Then, in the future, we could add other means of retrieving contacts, which would be wrapped nicely in a JSM. but that is for the platform review. This looks good to me.

This seems like something that needs superreview. It looks mostly ok to me otherwise - I'll do a more thorough review with superreview.

Might take if reviewed, but not blocking at this point

Comment on attachment 495541 [details] [diff] [review] patch sr=jst (verbally)

Comment on attachment 495541 [details] [diff] [review] patch >+++ b/embedding/android/AndroidManifest.xml.in >+ <uses-permission android:name="android.permission.READ_CONTACTS" /> Outside the scope of this bug, but we need to think about how to tell users why Firefox requests these permissions. (I've already seen some comment threads speculating about why we request location permissions, and this one probably looks even weirder/more dangerous.) >+++ b/embedding/android/GeckoAppShell.java >+import android.provider.Contacts.*; Do you actually use anything from this deprecated class? >+ Cursor cur = cr.query(ContactsContract.Contacts.CONTENT_URI, >+ null, null, null, null); We should pass a projection argument to avoid retrieving all columns, which the docs say is "inefficient." >+ if (cur.getCount() > 0) { >+ while (cur.moveToNext()) { You can remove the "if" statement. >+ JSONObject obj = new JSONObject(); >+ String id = cur.getString( >+ cur.getColumnIndex(ContactsContract.Contacts._ID)); >+ String name = cur.getString( >+ cur.getColumnIndex(ContactsContract.Contacts.DISPLAY_NAME)); Move the getColumnIndex calls out of the while loops and save the results in local variables. >+ if (Integer.parseInt(cur.getString(cur.getColumnIndex(ContactsContract.Contacts.HAS_PHONE_NUMBER))) > 0) { Can't you use cur.getInt instead? >+ Cursor pCur = cr.query( >+ ContactsContract.CommonDataKinds.Phone.CONTENT_URI, >+ null, >+ ContactsContract.CommonDataKinds.Phone.CONTACT_ID +" = ?", >+ new String[]{id}, null); It would probably be much faster if we could query all the tables at once with a join, or query each table once and join the results. But the only way I can see to do this is CursorJoiner, which looks kind of annoying. Since we're only using this data for form suggestions, could we change the format to include separate arrays of names, emails, and phone numbers, without joining them at all? This would probably be most efficient. I'd like to see some timing of this function with a large (say 500+) number of contacts. If there's a chance it will block for a significant amount of time on a very large (say 5000+) contact database, then we should either limit the number of rows we read, or make this API async.

Matt - Want to take a shot at refactoring? Possible things to tweak besides the cleanup: * Projection * Timing * Async

This feature has implications to the form autocomplete system, but also for add-ons and a future built-in sharing system.

... and possibly any future web content contact APIs

This patch replaces the two separate previous patches. This patch also addresses some of Matt's review comments. The Android code uses projects to limit the queries and removes the need for the column index calls. The form autocomplete code now reuses the previous list of contacts, filtering _that_ list instead of filtering the entire list of contacts. We also limit the number of suggestions shown in the popup bubble - to speed up the display of the UI.

The last patch had speed issues. On my Nexus One, with 391 contacts, it was taking >4400ms to load the contacts from Android. Brad has many more contacts and it was taking over 40secs. I tried to turn the query into an async query, but I ran into trouble with using Looper. I converted the XPCOM part of the patch to work async, but I left the Java part sync for now. I did change the Java query part into two separate queries: 1 to fetch the emails, and 1 to fetch the phone numbers. Each query also returns the contact's display name. I use a HashMap to create a master list of contacts with emails and phoneNumbers properties, just like before. However, this approach is 40x faster. Queries on my Nexus One now take 108ms. Posting the patch for feedback and to let people stress test the performance.

filter on [mass-p5]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.