Rename avatar to icon in the credential manager in Chromium.

Rename avatar to icon in the credential manager in Chromium.

As specified in https://w3c.github.io/webappsec/specs/credentialmanagement/#credential

The corresponding field in PasswordForm is still named avatar_url, to avoid extensive changes in the backends.

This CL is a part of the three-sided patchset:
1. https://codereview.chromium.org/1209953002/
2. https://codereview.chromium.org/1209363003/ <-- this patch
3. https://codereview.chromium.org/1208193003/

BUG=505445
Committed: https://crrev.com/cde556e2bc6d5095b864e626083a4a37cc1180e5
Cr-Commit-Position: refs/heads/master@{#336774}

[msramek, 2015-06-26 09:55:19]

msramek@chromium.org changed reviewers:
+ mkwst@chromium.org

[msramek, 2015-06-26 09:55:19]

Hi Mike,

PTAL at the second part.

If we want to have a complete renaming, there are 3 layers:
1. Credential Manager (icon->avatar)
2. PasswordForm (avatar_url->icon_url)
3. PasswordStore backends and sync protobufs (avatar_url->icon_url)

Doing all three at once would be a huge change (we change the column name in the
databases, so we have to migrate all the data...).

Originally I prepared a CL that stopped after step 2, but it seemed incorrect.
What we save in the database or send through sync should have 1-to-1
correspondence to the PasswordForm structure.

So this CL just stops after step 1. Let me know if you think we should continue.

Martin

[msramek, 2015-06-26 09:58:36]

+Garrett PTAL as well as the owner of PasswordForm. Also, what's your opinion on
further renaming? (see the previous comment)

[msramek, 2015-06-26 09:59:25]

msramek@chromium.org changed reviewers:
+ gcasto@chromium.org

[msramek, 2015-06-26 09:59:25]

+Garrett, this time in the "Reviewers:" field as well.

[Mike West, 2015-06-29 07:53:33]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-06-29 07:53:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209363003/1

[Mike West, 2015-06-29 07:55:37]

On 2015/06/26 at 09:55:19, msramek wrote:
> Hi Mike,
> 
> PTAL at the second part.
> 
> If we want to have a complete renaming, there are 3 layers:
> 1. Credential Manager (icon->avatar)
> 2. PasswordForm (avatar_url->icon_url)
> 3. PasswordStore backends and sync protobufs (avatar_url->icon_url)
> 
> Doing all three at once would be a huge change (we change the column name in
the databases, so we have to migrate all the data...).
> 
> Originally I prepared a CL that stopped after step 2, but it seemed incorrect.
What we save in the database or send through sync should have 1-to-1
correspondence to the PasswordForm structure.
> 
> So this CL just stops after step 1. Let me know if you think we should
continue.

I'd suggest that we should change everything. Otherwise we're just going to
confuse ourselves. This includes things like renaming the "AvatarFetcher" bits
and pieces.

I certainly wouldn't mind doing that work in more than one patch, but we
shouldn't end up with `icon` at the web-facing layer and `avatar` in the
internals.

[commit-bot: I haz the power, 2015-06-29 08:32:22]

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

[commit-bot: I haz the power, 2015-06-29 08:32:23]

Dry run: This issue passed the CQ dry run.

[msramek, 2015-06-29 14:42:53]

On 2015/06/29 07:55:37, Mike West wrote:
> On 2015/06/26 at 09:55:19, msramek wrote:
> > Hi Mike,
> > 
> > PTAL at the second part.
> > 
> > If we want to have a complete renaming, there are 3 layers:
> > 1. Credential Manager (icon->avatar)
> > 2. PasswordForm (avatar_url->icon_url)
> > 3. PasswordStore backends and sync protobufs (avatar_url->icon_url)
> > 
> > Doing all three at once would be a huge change (we change the column name in
> the databases, so we have to migrate all the data...).
> > 
> > Originally I prepared a CL that stopped after step 2, but it seemed
incorrect.
> What we save in the database or send through sync should have 1-to-1
> correspondence to the PasswordForm structure.
> > 
> > So this CL just stops after step 1. Let me know if you think we should
> continue.
> 
> I'd suggest that we should change everything. Otherwise we're just going to
> confuse ourselves. This includes things like renaming the "AvatarFetcher" bits
> and pieces.
> 
> I certainly wouldn't mind doing that work in more than one patch, but we
> shouldn't end up with `icon` at the web-facing layer and `avatar` in the
> internals.

Ok then. We'll have a bit more than 3 CLs then, so I created a separate bug
(crbug.com/505445) to keep track. I'll send you the CL for step 2 shortly.

[Mike West, 2015-06-30 06:16:00]

On 2015/06/29 at 14:42:53, msramek wrote:
> On 2015/06/29 07:55:37, Mike West wrote:
> > On 2015/06/26 at 09:55:19, msramek wrote:
> > > Hi Mike,
> > > 
> > > PTAL at the second part.
> > > 
> > > If we want to have a complete renaming, there are 3 layers:
> > > 1. Credential Manager (icon->avatar)
> > > 2. PasswordForm (avatar_url->icon_url)
> > > 3. PasswordStore backends and sync protobufs (avatar_url->icon_url)
> > > 
> > > Doing all three at once would be a huge change (we change the column name
in
> > the databases, so we have to migrate all the data...).
> > > 
> > > Originally I prepared a CL that stopped after step 2, but it seemed
incorrect.
> > What we save in the database or send through sync should have 1-to-1
> > correspondence to the PasswordForm structure.
> > > 
> > > So this CL just stops after step 1. Let me know if you think we should
> > continue.
> > 
> > I'd suggest that we should change everything. Otherwise we're just going to
> > confuse ourselves. This includes things like renaming the "AvatarFetcher"
bits
> > and pieces.
> > 
> > I certainly wouldn't mind doing that work in more than one patch, but we
> > shouldn't end up with `icon` at the web-facing layer and `avatar` in the
> > internals.
> 
> Ok then. We'll have a bit more than 3 CLs then, so I created a separate bug
(crbug.com/505445) to keep track. I'll send you the CL for step 2 shortly.

LGTM if you add some TODOs here. In PasswordForm at least.

[msramek, 2015-06-30 09:30:18]

On 2015/06/30 06:16:00, Mike West wrote:
> On 2015/06/29 at 14:42:53, msramek wrote:
> > On 2015/06/29 07:55:37, Mike West wrote:
> > > On 2015/06/26 at 09:55:19, msramek wrote:
> > > > Hi Mike,
> > > > 
> > > > PTAL at the second part.
> > > > 
> > > > If we want to have a complete renaming, there are 3 layers:
> > > > 1. Credential Manager (icon->avatar)
> > > > 2. PasswordForm (avatar_url->icon_url)
> > > > 3. PasswordStore backends and sync protobufs (avatar_url->icon_url)
> > > > 
> > > > Doing all three at once would be a huge change (we change the column
name
> in
> > > the databases, so we have to migrate all the data...).
> > > > 
> > > > Originally I prepared a CL that stopped after step 2, but it seemed
> incorrect.
> > > What we save in the database or send through sync should have 1-to-1
> > > correspondence to the PasswordForm structure.
> > > > 
> > > > So this CL just stops after step 1. Let me know if you think we should
> > > continue.
> > > 
> > > I'd suggest that we should change everything. Otherwise we're just going
to
> > > confuse ourselves. This includes things like renaming the "AvatarFetcher"
> bits
> > > and pieces.
> > > 
> > > I certainly wouldn't mind doing that work in more than one patch, but we
> > > shouldn't end up with `icon` at the web-facing layer and `avatar` in the
> > > internals.
> > 
> > Ok then. We'll have a bit more than 3 CLs then, so I created a separate bug
> (<crxref style="background-image:
url(chrome-extension://ppnlfijacbhfpjgkafbeiaklmgoljooo/res/type/issue.png);">crbug.com/505445</crxref>)
to keep track. I'll send you the CL for step 2 shortly.
> 
> LGTM if you add some TODOs here. In PasswordForm at least.

Done. I added it to PasswordForm, since that is currently the border between
"avatar" and "icon" naming. The follow-up CL might get more TODOs though.

[msramek, 2015-06-30 12:55:58]

The CQ bit was checked by msramek@chromium.org to run a CQ dry run

[msramek, 2015-06-30 12:55:59]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1209363003/#ps20001
(title: "Added TODO.")

[commit-bot: I haz the power, 2015-06-30 12:56:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209363003/20001

[vabr (Chromium), 2015-06-30 13:12:31]

vabr@chromium.org changed reviewers:
+ vabr@chromium.org

[vabr (Chromium), 2015-06-30 13:12:32]

components/autofill/core/common/password_form.h LGTM

[vabr (Chromium), 2015-06-30 13:15:05]

On 2015/06/30 13:12:32, vabr (OOO until 29 June) wrote:
> components/autofill/core/common/password_form.h LGTM

And reading #2, I personally support the smaller CLs. It is easier for me to
review them, than huge chunks requiring long continuous periods of time just to
read through.

Cheers,
Vaclav

[commit-bot: I haz the power, 2015-06-30 13:38:37]

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

[commit-bot: I haz the power, 2015-06-30 13:38:38]

Dry run: This issue passed the CQ dry run.

[msramek, 2015-06-30 13:40:28]

On 2015/06/30 13:15:05, vabr (OOO until 29 June) wrote:
> On 2015/06/30 13:12:32, vabr (OOO until 29 June) wrote:
> > components/autofill/core/common/password_form.h LGTM
> 
> And reading #2, I personally support the smaller CLs. It is easier for me to
> review them, than huge chunks requiring long continuous periods of time just
to
> read through.
> 
> Cheers,
> Vaclav

Thanks for the drive-by, Václav :)

Of course, the rest of the renaming stuff will be chopped into reasonable
pieces. I'm not going to fix 5 backends at once :)

[msramek, 2015-06-30 13:40:40]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2015-06-30 13:40:54]

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

[commit-bot: I haz the power, 2015-06-30 13:44:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-06-30 13:45:33]

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