Add Glue as delegate to AutofillDialog

Add Glue as delegate to AutofillDialog

This starts making the layout transitions depend on calls coming from native.
It adds calls that notify native for item selection and also makes it possible
to change and select accounts. Also pulled default menu items to AutofillDialog
to control the flow better making the UI logic simpler.

BUG=196264
NOTRY=true

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

[Ted C, 2013-03-20 19:04:31]

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java
(right):

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:33:
private static final int EDIT_MENU_ITEM_INDEX = -2;
blank line after the statics

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:42:
private static final List<String> mDefaultAccountItems = new
ArrayList<String>();
static?

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:135:
getResources().getString(R.string.autofill_use_local));
everywhere in this method, you should be using context instead of getContext().

Also you use Resources a lot, so I would create a local reference to Resources
and just this down a bit

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:165:
long id) {
since you're here, will long id fit on the previous line?

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogGlue.java
(right):

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogGlue.java:52:
private void modelChanged(boolean fetchingIsActive) {
indenting is off on these two methods

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogTitleView.java
(right):

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogTitleView.java:65:
Spinner accounts_spinner = (Spinner)findViewById(R.id.accounts_spinner);
space after case

accounts_spinner isn't java style

[Yusuf, 2013-03-21 00:53:17]

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java
(right):

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:33:
private static final int EDIT_MENU_ITEM_INDEX = -2;
On 2013/03/20 19:04:31, Ted C wrote:
> blank line after the statics

Done.

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:42:
private static final List<String> mDefaultAccountItems = new
ArrayList<String>();
On 2013/03/20 19:04:31, Ted C wrote:
> static?

Done.

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:135:
getResources().getString(R.string.autofill_use_local));
On 2013/03/20 19:04:31, Ted C wrote:
> everywhere in this method, you should be using context instead of
getContext().
> 
> Also you use Resources a lot, so I would create a local reference to Resources
> and just this down a bit

Done.

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:165:
long id) {
On 2013/03/20 19:04:31, Ted C wrote:
> since you're here, will long id fit on the previous line?

Done.

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogGlue.java
(right):

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogGlue.java:52:
private void modelChanged(boolean fetchingIsActive) {
On 2013/03/20 19:04:31, Ted C wrote:
> indenting is off on these two methods

Done.

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogTitleView.java
(right):

https://codereview.chromium.org/12942004/diff/5001/chrome/android/java/src/or...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogTitleView.java:65:
Spinner accounts_spinner = (Spinner)findViewById(R.id.accounts_spinner);
On 2013/03/20 19:04:31, Ted C wrote:
> space after case
> 
> accounts_spinner isn't java style

Done.

[Ted C, 2013-03-21 01:01:14]

https://codereview.chromium.org/12942004/diff/10001/chrome/android/java/src/o...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java
(right):

https://codereview.chromium.org/12942004/diff/10001/chrome/android/java/src/o...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:44:
private static final List<String> mDefaultAccountItems = new
ArrayList<String>();
I actually meant this should't be static.

I also don't think mDefaultMenuItems should be static.

You set the values every time you construct an AutofillDialog so the statics
aren't necessary or needed.

[Yusuf, 2013-03-21 17:53:36]

https://codereview.chromium.org/12942004/diff/10001/chrome/android/java/src/o...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java
(right):

https://codereview.chromium.org/12942004/diff/10001/chrome/android/java/src/o...
chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialog.java:44:
private static final List<String> mDefaultAccountItems = new
ArrayList<String>();
On 2013/03/21 01:01:14, Ted C wrote:
> I actually meant this should't be static.
> 
> I also don't think mDefaultMenuItems should be static.
> 
> You set the values every time you construct an AutofillDialog so the statics
> aren't necessary or needed.

Done.

[Ted C, 2013-03-21 18:18:38]

lgtm

[commit-bot: I haz the power, 2013-03-21 20:54:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/12942004/17001

[commit-bot: I haz the power, 2013-03-21 22:57:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/12942004/17001

[commit-bot: I haz the power, 2013-03-21 22:57:45]

Change committed as 189711