Fixing missing password Autofill popup for Android

Fixing missing password Autofill popup for Android

Adding support for password Autofill popups for Android.

BUG=163619
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=171102

[aurimas (slooooooooow), 2012-11-30 23:21:58]

Hey Ted,

Could you take a look at my java side changes in chrome/android ?

Thanks,
Aurimas

[aurimas (slooooooooow), 2012-11-30 23:22:59]

Hey Ilya,

Could you take a look at this CL please?

Thanks!
Aurimas

[Ted C, 2012-12-01 00:02:23]

lgtm for androidy bits

https://codereview.chromium.org/11299287/diff/1/chrome/android/java/src/org/c...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopup.java
(right):

https://codereview.chromium.org/11299287/diff/1/chrome/android/java/src/org/c...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopup.java:140:
if (itemId > 0 || itemId == ITEM_ID_AUTOCOMPLETE_ENTRY ||
should this be >= 0 like before?

[Ilya Sherman, 2012-12-03 23:27:16]

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopup.java
(right):

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopup.java:38:
private static final int ITEM_ID_PASSWORD_ENTRY = -2;
How come this doesn't include the other ids from WebAutofillClient.h?  At a
minimum, I'd expect that you also want to show <datalist> suggestions.  For the
others, you should probably add comments for why they're not being included.

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/browser/autofil...
File chrome/browser/autofill/autofill_external_delegate.cc (right):

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/browser/autofil...
chrome/browser/autofill/autofill_external_delegate.cc:211: popup_visible_ =
true;
Hmm, why is this change needed?  It's not obvious to me that the popup is
guaranteed to be visible at this point, and I'm surprised that popup_visible_
wasn't updated earlier, when the popup actually got shown.

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/renderer/autofi...
File chrome/renderer/autofill/password_autofill_manager.cc (right):

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/renderer/autofi...
chrome/renderer/autofill/password_autofill_manager.cc:610: // only fills in the
fields after the user clicks on the suggestion popup.
Hmm, why does the Android UI differ in this way from desktop?

[aurimas (slooooooooow), 2012-12-04 02:05:00]

Fixed the nits.

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopup.java
(right):

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopup.java:38:
private static final int ITEM_ID_PASSWORD_ENTRY = -2;
On 2012/12/03 23:27:17, Ilya Sherman wrote:
> How come this doesn't include the other ids from WebAutofillClient.h?  At a
> minimum, I'd expect that you also want to show <datalist> suggestions.  For
the
> others, you should probably add comments for why they're not being included.

Done.

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopup.java:140:
if (itemId > 0 || itemId == ITEM_ID_AUTOCOMPLETE_ENTRY ||
On 2012/12/01 00:02:23, Ted C wrote:
> should this be >= 0 like before?

Done.

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/browser/autofil...
File chrome/browser/autofill/autofill_external_delegate.cc (right):

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/browser/autofil...
chrome/browser/autofill/autofill_external_delegate.cc:211: popup_visible_ =
true;
On 2012/12/03 23:27:17, Ilya Sherman wrote:
> Hmm, why is this change needed?  It's not obvious to me that the popup is
> guaranteed to be visible at this point, and I'm surprised that popup_visible_
> wasn't updated earlier, when the popup actually got shown.

Done.

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/renderer/autofi...
File chrome/renderer/autofill/password_autofill_manager.cc (right):

https://chromiumcodereview.appspot.com/11299287/diff/1/chrome/renderer/autofi...
chrome/renderer/autofill/password_autofill_manager.cc:610: // only fills in the
fields after the user clicks on the suggestion popup.
On 2012/12/03 23:27:17, Ilya Sherman wrote:
> Hmm, why does the Android UI differ in this way from desktop?

Done.

[Ilya Sherman, 2012-12-04 20:50:09]

LGTM, thanks

[commit-bot: I haz the power, 2012-12-04 21:00:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aurimas@chromium.org/11299287/8001

[commit-bot: I haz the power, 2012-12-05 00:48:06]

Change committed as 171102