Componentize autofill dialog common code

Componentize autofill dialog common code

This CL move some code from
autofill_dialog_common and autofill_dialog_types
into the autofill components so that they can be
used on iOS.

It is only a code move (+git cl format).

TBR=sky

Committed: https://crrev.com/b6a7008789ce4a144669fa906a21f7f5722f8448
Cr-Commit-Position: refs/heads/master@{#319855}

[droger, 2015-03-04 17:05:12]

Patchset #1 (id:1) has been deleted

[droger, 2015-03-04 17:38:02]

Patchset #1 (id:20001) has been deleted

[droger, 2015-03-04 17:38:16]

Patchset #1 (id:40001) has been deleted

[droger, 2015-03-04 18:00:44]

Patchset #4 (id:120001) has been deleted

[droger, 2015-03-04 18:01:53]

droger@chromium.org changed reviewers:
+ estade@chromium.org

[droger, 2015-03-04 18:07:54]

Patchset #1 (id:60001) has been deleted

[droger, 2015-03-04 18:08:02]

Patchset #1 (id:80001) has been deleted

[droger, 2015-03-04 18:08:12]

Patchset #1 (id:100001) has been deleted

[droger, 2015-03-04 18:08:31]

Patchset #1 (id:100001) has been deleted

[droger, 2015-03-04 18:08:40]

estade: please review
dconnelly: FYI

Thanks.

[Evan Stade, 2015-03-05 02:29:00]

I'm confused why you'd want this on iOS. This is just for requestAutocomplete.
iOS uses honeygold for requestAutocomplete.

https://codereview.chromium.org/974323002/diff/150001/chrome/browser/ui/andro...
File chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
(left):

https://codereview.chromium.org/974323002/diff/150001/chrome/browser/ui/andro...
chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc:22:
#include "chrome/browser/ui/autofill/autofill_dialog_common.h"
this actually isn't used

[droger, 2015-03-05 09:03:02]

On 2015/03/05 02:29:00, Evan Stade wrote:
> I'm confused why you'd want this on iOS. This is just for requestAutocomplete.
> iOS uses honeygold for requestAutocomplete.

Thanks for the review.

iOS currently uses autofill_dialog_common.h and thus blocks iOS upstreaming, see
here:

https://chrome-internal.googlesource.com/bling/chromium/+/master/ios_internal...

I see calls to
autofill::common::ServerTypeMatchesField()
autofill::common::BuildInputs(()
autofill::common::TypesFromInputs()

It also uses types from autofill_dialog_types, even though the dependency is not
explictly listed:
autofill::DetailInput and autofill::DialogSection

[droger, 2015-03-05 09:03:57]

Patchset #3 (id:170001) has been deleted

[droger, 2015-03-05 09:04:57]

https://codereview.chromium.org/974323002/diff/150001/chrome/browser/ui/andro...
File chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
(left):

https://codereview.chromium.org/974323002/diff/150001/chrome/browser/ui/andro...
chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc:22:
#include "chrome/browser/ui/autofill/autofill_dialog_common.h"
On 2015/03/05 02:28:59, Evan Stade wrote:
> this actually isn't used

I cannot remove it, common::BuildInputs() is used (see for example line 75).

[Evan Stade, 2015-03-05 18:32:00]

sorry, my mistake. Can we just upstream the parts that are actually shared and
leave the rest where they are?

https://codereview.chromium.org/974323002/diff/150001/chrome/browser/ui/autof...
File chrome/browser/ui/autofill/autofill_dialog_types.cc (left):

https://codereview.chromium.org/974323002/diff/150001/chrome/browser/ui/autof...
chrome/browser/ui/autofill/autofill_dialog_types.cc:51: SkColor
DialogNotification::GetBackgroundColor() const {
Parts of this file don't need to be moved (they /can/ be moved but that doesn't
mean it makes sense to move them). UI details like this which are not used by
non-desktop code should stay where they are.

[droger, 2015-03-09 13:24:58]

Patchset #3 (id:190001) has been deleted

[droger, 2015-03-09 13:27:27]

On 2015/03/05 18:32:00, Evan Stade wrote:
> sorry, my mistake. Can we just upstream the parts that are actually shared and
> leave the rest where they are?

Yes, done. Please take a look.

[Evan Stade, 2015-03-09 17:06:31]

lgtm with comments addressed

https://codereview.chromium.org/974323002/diff/230001/components/autofill/cor...
File components/autofill/core/browser/field_types.h (right):

https://codereview.chromium.org/974323002/diff/230001/components/autofill/cor...
components/autofill/core/browser/field_types.h:239: struct DetailInput {
I think it makes sense to move these to their own file --- I don't think they're
sufficiently similar to the above enums to justify being in this file

https://codereview.chromium.org/974323002/diff/230001/components/autofill/cor...
File components/autofill/core/browser/server_field_types_util.h (right):

https://codereview.chromium.org/974323002/diff/230001/components/autofill/cor...
components/autofill/core/browser/server_field_types_util.h:10: #include
"components/autofill/core/browser/autofill_field.h"
forward declare AutofillField?

[droger, 2015-03-09 17:51:49]

Patchset #5 (id:250001) has been deleted

[droger, 2015-03-09 17:54:54]

Thanks for the review.
I split the types to their own files.

https://codereview.chromium.org/974323002/diff/230001/components/autofill/cor...
File components/autofill/core/browser/server_field_types_util.h (right):

https://codereview.chromium.org/974323002/diff/230001/components/autofill/cor...
components/autofill/core/browser/server_field_types_util.h:10: #include
"components/autofill/core/browser/autofill_field.h"
On 2015/03/09 17:06:31, Evan Stade wrote:
> forward declare AutofillField?

Done.

[droger, 2015-03-10 09:24:31]

The CQ bit was checked by droger@chromium.org

[droger, 2015-03-10 09:24:31]

The patchset sent to the CQ was uploaded after l-g-t-m from estade@chromium.org
Link to the patchset: https://codereview.chromium.org/974323002/#ps310001
(title: "add missing includes")

[droger, 2015-03-10 09:24:34]

The CQ bit was unchecked by droger@chromium.org

[droger, 2015-03-10 09:27:00]

droger@chromium.org changed reviewers:
+ sky@chromium.org

[droger, 2015-03-10 09:27:00]

TBR sky for trivial changes (includes and namespaces) in
chrome/browser/ui/cocoa/autofill/
chrome/browser/ui/android/autofill/

[droger, 2015-03-10 09:27:06]

The CQ bit was checked by droger@chromium.org

[commit-bot: I haz the power, 2015-03-10 09:27:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/974323002/310001

[commit-bot: I haz the power, 2015-03-10 09:44:57]

Committed patchset #7 (id:310001)

[commit-bot: I haz the power, 2015-03-10 09:45:27]

Patchset 7 (id:??) landed as
https://crrev.com/b6a7008789ce4a144669fa906a21f7f5722f8448
Cr-Commit-Position: refs/heads/master@{#319855}