[Autofill] Add typemap for PasswordForm and PasswordFormGenerationData.

[Autofill] Add typemap for PasswordForm and PasswordFormGenerationData.

This CL is basis to mojofy IPCs between:
ContentPasswordManagerDriver<-->PasswordAutofillAgent
ContentPasswordManagerDriver<-->PasswordGenerationAgent

TEST=components_unittests --gtest_filter=AutofillTypeTraitsTestImpl*
BUG=582391
Committed: https://crrev.com/4638cbe08ea8775a448e0537545f2f7e5f9ad336
Cr-Commit-Position: refs/heads/master@{#403658}

[leonhsl(Using Gerrit), 2016-06-27 10:13:16]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 10:13:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-27 10:16:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-27 10:16:57]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[leonhsl(Using Gerrit), 2016-06-28 03:33:36]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-28 03:34:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 04:12:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-28 04:12:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[leonhsl(Using Gerrit), 2016-06-28 05:16:42]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-28 05:17:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 06:12:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-28 06:12:25]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2016-06-28 07:47:30]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-28 07:47:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 08:56:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-28 08:56:01]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2016-06-28 09:03:10]

leon.han@intel.com changed reviewers:
+ dcheng@chromium.org, mathp@chromium.org, vabr@chromium.org,
yzshen@chromium.org

[leonhsl(Using Gerrit), 2016-06-28 09:03:11]

Another typemap support CL, would you PTAL at ps#6? Thanks.

[vabr (Chromium), 2016-06-28 09:57:45]

LGTM, thanks for the next stride towards Mojo in autofill!
Vaclav

https://codereview.chromium.org/2101533002/diff/100001/components/autofill/co...
File
components/autofill/content/public/cpp/autofill_types_struct_traits_unittest.cc
(right):

https://codereview.chromium.org/2101533002/diff/100001/components/autofill/co...
components/autofill/content/public/cpp/autofill_types_struct_traits_unittest.cc:178:
EXPECT_EQ(expected.scheme, actual.scheme);
Why not rely on operator== and just write EXPECT_EQ(expected, actual)?
Apart from shorter code, the test will not need to be updated once PasswordForm
is changed.

[yzshen1, 2016-06-28 20:50:22]

lgtm

[leonhsl(Using Gerrit), 2016-06-29 03:16:56]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 03:17:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[leonhsl(Using Gerrit), 2016-06-29 03:42:36]

Thanks Vaclav and Yuzhu for review. Uploaded ps#7 to use
PasswordForm::operator==().

And, about follow-up CLs, I'm planning another 2 CLs: One is to add typemap for
FormsPredictionsMap and PasswordFormFieldPredictionMap(As this CL is already big
enough), and then another one is to mojofy the left autofill IPCs all at once.
We're moving forward steadily ;-)

https://codereview.chromium.org/2101533002/diff/100001/components/autofill/co...
File
components/autofill/content/public/cpp/autofill_types_struct_traits_unittest.cc
(right):

https://codereview.chromium.org/2101533002/diff/100001/components/autofill/co...
components/autofill/content/public/cpp/autofill_types_struct_traits_unittest.cc:178:
EXPECT_EQ(expected.scheme, actual.scheme);
On 2016/06/28 09:57:45, vabr (Chromium) wrote:
> Why not rely on operator== and just write EXPECT_EQ(expected, actual)?
> Apart from shorter code, the test will not need to be updated once
PasswordForm
> is changed.

Done and big thanks!
I remember that I tried to check the existence of PasswordForm::operator==(),
but somehow missed, uuu..

[commit-bot: I haz the power, 2016-06-29 04:24:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-29 04:24:05]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2016-06-29 06:36:47]

Thanks for addressing the comment, LGTM!
Vaclav

[dcheng, 2016-06-30 12:33:42]

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
File components/autofill/content/public/cpp/autofill_types_struct_traits.h
(right):

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
components/autofill/content/public/cpp/autofill_types_struct_traits.h:374:
static const std::string& signon_realm(const autofill::PasswordForm& r) {
Btw, yzshen@, is there any best practice on whether it's better to return const
std::string& or StringPiece? For example, the URL struct traits use StringPiece
here. Which is better? I'm guessing it doesn't matter, but I'm not 100% sure.

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
File components/autofill/content/public/interfaces/autofill_types.mojom (right):

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
components/autofill/content/public/interfaces/autofill_types.mojom:141:
url.mojom.Url origin;
Can we have a comment on this field that it's not really an origin?

I'm wondering what we can do for documentation here too... there's a lot of
fields here, and while it's basically a reflection of autofill::PasswordForm, it
seems like it'd be nice to have some of it here. On the flip side... duplicating
the comments everywhere doesn't seem good either.

[vabr (Chromium), 2016-06-30 12:57:18]

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
File components/autofill/content/public/interfaces/autofill_types.mojom (right):

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
components/autofill/content/public/interfaces/autofill_types.mojom:141:
url.mojom.Url origin;
On 2016/06/30 12:33:42, dcheng wrote:
> Can we have a comment on this field that it's not really an origin?
> 
> I'm wondering what we can do for documentation here too... there's a lot of
> fields here, and while it's basically a reflection of autofill::PasswordForm,
it
> seems like it'd be nice to have some of it here. On the flip side...
duplicating
> the comments everywhere doesn't seem good either.

PasswordForm has bloated a bit in the past. It serves at least 3 different
use-cases, and we are soon going to split it up into more reasonable pieces
(http://crbug.com/498048). Some of the fields are obsolete and about to be
removed (http://crbug.com/408757, http://crbug.com/413020). At that point a
sizeable piece of the documentation currently present in password_form.h will
disappear (comments such as "this is only used for use-case 1, and not 2 and
3"). So I would suggest not to worry with documenting the Mojo version at this
moment.

[yzshen1, 2016-06-30 15:19:32]

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
File components/autofill/content/public/cpp/autofill_types_struct_traits.h
(right):

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
components/autofill/content/public/cpp/autofill_types_struct_traits.h:374:
static const std::string& signon_realm(const autofill::PasswordForm& r) {
On 2016/06/30 12:33:42, dcheng wrote:
> Btw, yzshen@, is there any best practice on whether it's better to return
const
> std::string& or StringPiece? For example, the URL struct traits use
StringPiece
> here. Which is better? I'm guessing it doesn't matter, but I'm not 100% sure.

It shouldn't matter. The url struct traits were done when we only supported
StringPiece. If it were written today, it would be a little easier to directly
return r.possible_invalid_spec() which is const std::string&.

[leonhsl(Using Gerrit), 2016-07-02 11:05:22]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-02 11:05:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-02 12:11:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-02 12:11:09]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2016-07-02 12:15:45]

Uploaded ps#8 to address comments from Daniel. PTAnL, thanks.

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
File components/autofill/content/public/interfaces/autofill_types.mojom (right):

https://codereview.chromium.org/2101533002/diff/120001/components/autofill/co...
components/autofill/content/public/interfaces/autofill_types.mojom:141:
url.mojom.Url origin;
On 2016/06/30 12:57:17, vabr (Chromium) wrote:
> On 2016/06/30 12:33:42, dcheng wrote:
> > Can we have a comment on this field that it's not really an origin?
> > 
> > I'm wondering what we can do for documentation here too... there's a lot of
> > fields here, and while it's basically a reflection of
autofill::PasswordForm,
> it
> > seems like it'd be nice to have some of it here. On the flip side...
> duplicating
> > the comments everywhere doesn't seem good either.
> 
> PasswordForm has bloated a bit in the past. It serves at least 3 different
> use-cases, and we are soon going to split it up into more reasonable pieces
> (http://crbug.com/498048). Some of the fields are obsolete and about to be
> removed (http://crbug.com/408757, http://crbug.com/413020). At that point a
> sizeable piece of the documentation currently present in password_form.h will
> disappear (comments such as "this is only used for use-case 1, and not 2 and
> 3"). So I would suggest not to worry with documenting the Mojo version at this
> moment.

Done.

[leonhsl(Using Gerrit), 2016-07-04 04:56:09]

Ping Daniel, thanks

[dcheng, 2016-07-04 07:13:46]

LGTM with a nit.

https://codereview.chromium.org/2101533002/diff/140001/components/autofill/co...
File components/autofill/content/public/interfaces/autofill_types.mojom (right):

https://codereview.chromium.org/2101533002/diff/140001/components/autofill/co...
components/autofill/content/public/interfaces/autofill_types.mojom:142: // the
rest is stripped.
Nit: instead of the comment, maybe we can just call this origin_with_path?
That'd be good enough as documentation for me, and it's obvious from the name
then.

[leonhsl(Using Gerrit), 2016-07-04 09:14:54]

Uploaded ps#9 to address nit and will land now. Thanks all~

https://codereview.chromium.org/2101533002/diff/140001/components/autofill/co...
File components/autofill/content/public/interfaces/autofill_types.mojom (right):

https://codereview.chromium.org/2101533002/diff/140001/components/autofill/co...
components/autofill/content/public/interfaces/autofill_types.mojom:142: // the
rest is stripped.
On 2016/07/04 07:13:46, dcheng wrote:
> Nit: instead of the comment, maybe we can just call this origin_with_path?
> That'd be good enough as documentation for me, and it's obvious from the name
> then.

Done.

[leonhsl(Using Gerrit), 2016-07-04 09:15:02]

The CQ bit was checked by leon.han@intel.com

[leonhsl(Using Gerrit), 2016-07-04 09:15:02]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org,
vabr@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2101533002/#ps160001
(title: "origin --> origin_with_path")

[commit-bot: I haz the power, 2016-07-04 09:15:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-04 10:19:12]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-07-04 10:20:34]

Description was changed from

==========
[Autofill] Add typemap for PasswordForm and PasswordFormGenerationData.

This CL is basis to mojofy IPCs between:
ContentPasswordManagerDriver<-->PasswordAutofillAgent
ContentPasswordManagerDriver<-->PasswordGenerationAgent

TEST=components_unittests --gtest_filter=AutofillTypeTraitsTestImpl*
BUG=582391
==========

to

==========
[Autofill] Add typemap for PasswordForm and PasswordFormGenerationData.

This CL is basis to mojofy IPCs between:
ContentPasswordManagerDriver<-->PasswordAutofillAgent
ContentPasswordManagerDriver<-->PasswordGenerationAgent

TEST=components_unittests --gtest_filter=AutofillTypeTraitsTestImpl*
BUG=582391

Committed: https://crrev.com/4638cbe08ea8775a448e0537545f2f7e5f9ad336
Cr-Commit-Position: refs/heads/master@{#403658}
==========

[commit-bot: I haz the power, 2016-07-04 10:20:37]

Patchset 9 (id:??) landed as
https://crrev.com/4638cbe08ea8775a448e0537545f2f7e5f9ad336
Cr-Commit-Position: refs/heads/master@{#403658}

[Finnur, 2016-07-04 11:09:33]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/2123483002/ by finnur@chromium.org.

The reason for reverting is: Caused compile failure:
https://build.chromium.org/p/chromium/buildstatus?builder=Mac&number=17333.