Replace assets for avatar placeholder on Android

Replace assets for avatar placeholder on Android

This CL updates pictures used when there's no user avatar available.
Before, there were two set of PNGs:
1. account_management_no_picture.png (40dp), used in settings (SigninPreference
   and AccountManagementFragment), AutoSigninSnackbarController
   and AccountChooserDialog.
2. fre_placeholder.png (130dp), used in AccountSigninView.
This CL replaces these assets with logo_avatar_anonymous.xml vector drawable,
reducing APK size and removing duplication.
The new asset also looks different than the old ones - it is gray in color
(old ones were blue) and uses different silhouette for avatar.

BUG=722910
Review-Url: https://codereview.chromium.org/2912083003
Cr-Commit-Position: refs/heads/master@{#477301}
Committed: https://chromium.googlesource.com/chromium/src/+/a99122347afb681ff46cf7a78c68a44f64549606

[bsazonov, 2017-05-31 15:00:43]

bsazonov@chromium.org changed reviewers:
+ twellington@chromium.org

[bsazonov, 2017-05-31 15:00:44]

Theresa, please take a look.

[Theresa, 2017-05-31 15:14:44]

This seems like something that is well suited for a vector drawable since the
image is:
 - very simple
 - relatively large (64dp)
 - used at 2 different dp sizes
 - used relatively infrequently (not in core UI)

Since this is a material icon (logo_avatar_anonymous), you can get an Android
compatible XML file for the vector drawable from Android Studio.

For file naming, using the go/icons name is preferred so that it's easier for
folks to find in the future.

[bsazonov, 2017-06-01 19:03:16]

On 2017/05/31 15:14:44, Theresa wrote:
> This seems like something that is well suited for a vector drawable since the
> image is:
>  - very simple
>  - relatively large (64dp)
>  - used at 2 different dp sizes
>  - used relatively infrequently (not in core UI)
> 
> Since this is a material icon (logo_avatar_anonymous), you can get an Android
> compatible XML file for the vector drawable from Android Studio.
> 
> For file naming, using the go/icons name is preferred so that it's easier for
> folks to find in the future.

Theresa, please take a look. I've replaced PNG asset with a vector one as you
requested. This change shaves off 25KB from APK size.

[bsazonov, 2017-06-01 19:03:25]

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

[commit-bot: I haz the power, 2017-06-01 19:04:03]

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, 2017-06-01 20:32:32]

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

[commit-bot: I haz the power, 2017-06-01 20:32:34]

Dry run: This issue passed the CQ dry run.

[Theresa, 2017-06-01 23:22:15]

Thank you for changing it to a vector! Will you please update the commit message
to reflect that?

https://codereview.chromium.org/2912083003/diff/20001/build/android/lint/supp...
File build/android/lint/suppressions.xml (right):

https://codereview.chromium.org/2912083003/diff/20001/build/android/lint/supp...
build/android/lint/suppressions.xml:199: <ignore
regexp="chrome/android/java/res/drawable/logo_avatar_anonymous.xml"/>
optional nit: Instead of adding an ingnore here, you can add
tools:targetApi="21" to the <vector> in the XML file. I don't know that we have
an established pattern (some vectors use tools: some add a rule here), but my
preference is tools:.

https://codereview.chromium.org/2912083003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java
(right):

https://codereview.chromium.org/2912083003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:677:
* @param context
nit: Add a JavaDoc description for context and add JavaDoc for new param in
updateUserNamePictureCache and any other methods that got new params.

https://codereview.chromium.org/2912083003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/smartlockautosignin/AutoSigninSnackbarController.java
(right):

https://codereview.chromium.org/2912083003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/smartlockautosignin/AutoSigninSnackbarController.java:58:
snackbar.setSingleLine(false).setBackgroundColor(backgroundColor).setProfileImage(icon);
It looks like this is the only class that uses the Snackbar profile image.
Rather than converting the drawable to a bitmap, Snackbar's methods should be
updated to work with a Drawable.

[bsazonov, 2017-06-02 11:46:38]

Patchset #3 (id:40001) has been deleted

[bsazonov, 2017-06-02 13:16:25]

Description was changed from

==========
Replace blue avatar placeholder picture with a gray one

This CL replaces the assets used when there's no user avatar available.
Old assets were blue, new ones are gray with a different silhouette.
1. account_management_no_picture.png is used in settings (SigninPreference and
   AccountManagementFragment).
2. fre_placeholder.png is used in AccountSigninView. This CL also changes the
   size of this asset from 130dp to 64dp, because corresponding ImageView is
   always 64dp.

BUG=722910
==========

to

==========
Replace assets for avatar placeholder on Android

This CL updates pictures used when there's no user avatar available.
Before, there were two set of PNGs:
1. account_management_no_picture.png (40dp), used in settings (SigninPreference
   and AccountManagementFragment), AutoSigninSnackbarController
   and AccountChooserDialog.
2. fre_placeholder.png (130dp), used in AccountSigninView.
This CL replaces these assets with logo_avatar_anonymous.xml vector drawable,
reducing APK size and removing duplication.
The new asset also looks different than the old ones - it is gray in color
(old ones were blue) and uses different silhouette for avatar.

BUG=722910
==========

[bsazonov, 2017-06-02 13:17:19]

bsazonov@chromium.org changed reviewers:
+ bauerb@chromium.org

[bsazonov, 2017-06-02 13:17:20]

Theresa, please take a look. I've addressed your comments.

Bernhard, please review changes in:
chrome/android/java/src/org/chromium/chrome/browser/firstrun/ProfileDataCache.java
chrome/android/java/src/org/chromium/chrome/browser/password_manager/AccountChooserDialog.java
chrome/android/java/src/org/chromium/chrome/browser/signin/

https://codereview.chromium.org/2912083003/diff/20001/build/android/lint/supp...
File build/android/lint/suppressions.xml (right):

https://codereview.chromium.org/2912083003/diff/20001/build/android/lint/supp...
build/android/lint/suppressions.xml:199: <ignore
regexp="chrome/android/java/res/drawable/logo_avatar_anonymous.xml"/>
On 2017/06/01 23:22:15, Theresa wrote:
> optional nit: Instead of adding an ingnore here, you can add
> tools:targetApi="21" to the <vector> in the XML file. I don't know that we
have
> an established pattern (some vectors use tools: some add a rule here), but my
> preference is tools:.

Didn't know about this option, thanks! I totally agree that this is way better
than ignoring it suppressions.xml.

https://codereview.chromium.org/2912083003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java
(right):

https://codereview.chromium.org/2912083003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:677:
* @param context
On 2017/06/01 23:22:15, Theresa wrote:
> nit: Add a JavaDoc description for context and add JavaDoc for new param in
> updateUserNamePictureCache and any other methods that got new params.

Done. I've also reordered params in some method to be consistent (Context is
usually the first parameter).

https://codereview.chromium.org/2912083003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/smartlockautosignin/AutoSigninSnackbarController.java
(right):

https://codereview.chromium.org/2912083003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/smartlockautosignin/AutoSigninSnackbarController.java:58:
snackbar.setSingleLine(false).setBackgroundColor(backgroundColor).setProfileImage(icon);
On 2017/06/01 23:22:15, Theresa wrote:
> It looks like this is the only class that uses the Snackbar profile image.
> Rather than converting the drawable to a bitmap, Snackbar's methods should be
> updated to work with a Drawable.

Done.

[bsazonov, 2017-06-02 13:29:34]

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

[commit-bot: I haz the power, 2017-06-02 13:29:55]

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, 2017-06-02 14:32:40]

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

[commit-bot: I haz the power, 2017-06-02 14:32:41]

Dry run: This issue passed the CQ dry run.

[Theresa, 2017-06-02 15:10:42]

lgtm

[Bernhard Bauer, 2017-06-06 12:40:56]

https://codereview.chromium.org/2912083003/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/firstrun/ProfileDataCache.java
(right):

https://codereview.chromium.org/2912083003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/firstrun/ProfileDataCache.java:39:
* Notifies that an account data in the profile has been updated.
Nit: "[...] an account's profile data has been updated"?

https://codereview.chromium.org/2912083003/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java
(right):

https://codereview.chromium.org/2912083003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:702:
private static Bitmap getAvatarPlaceholder(Context context) {
Why do we need this as a bitmap, BTW? Could we return a Drawable? It looks like
in a bunch of places we just put the bitmap into a BitmapDrawable again, so we
could save some memory by directly using the Drawable.

[bsazonov, 2017-06-06 14:10:05]

https://codereview.chromium.org/2912083003/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/firstrun/ProfileDataCache.java
(right):

https://codereview.chromium.org/2912083003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/firstrun/ProfileDataCache.java:39:
* Notifies that an account data in the profile has been updated.
On 2017/06/06 12:40:56, Bernhard Bauer wrote:
> Nit: "[...] an account's profile data has been updated"?

Done, thanks!

https://codereview.chromium.org/2912083003/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java
(right):

https://codereview.chromium.org/2912083003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:702:
private static Bitmap getAvatarPlaceholder(Context context) {
On 2017/06/06 12:40:56, Bernhard Bauer wrote:
> Why do we need this as a bitmap, BTW? Could we return a Drawable? It looks
like
> in a bunch of places we just put the bitmap into a BitmapDrawable again, so we
> could save some memory by directly using the Drawable.

Done. I didn't include these changes in the initial CL in a futile attempt to
keep it small.

[bsazonov, 2017-06-06 14:10:27]

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

[commit-bot: I haz the power, 2017-06-06 14:10:36]

Dry run: CQ is trying da patch.

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

[Bernhard Bauer, 2017-06-06 14:19:34]

Thanks! LGTM.

[bsazonov, 2017-06-06 14:30:28]

The CQ bit was unchecked by bsazonov@chromium.org

[bsazonov, 2017-06-06 14:30:48]

The CQ bit was checked by bsazonov@chromium.org

[bsazonov, 2017-06-06 14:30:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org
Link to the patchset: https://codereview.chromium.org/2912083003/#ps80001
(title: "Address comments")

[commit-bot: I haz the power, 2017-06-06 14:31:07]

CQ is trying da patch.

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

[bsazonov, 2017-06-06 14:47:09]

The CQ bit was unchecked by bsazonov@chromium.org

[bsazonov, 2017-06-06 15:03:15]

The CQ bit was checked by bsazonov@chromium.org

[bsazonov, 2017-06-06 15:03:16]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
twellington@chromium.org
Link to the patchset: https://codereview.chromium.org/2912083003/#ps100001
(title: "Fix makeRoundUserPicture")

[commit-bot: I haz the power, 2017-06-06 15:03:35]

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, 2017-06-06 15:47:33]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1496761395734250,
"parent_rev": "5a5c889a2c718b350d82f666280e8ebff3450550", "commit_rev":
"9585c6dfead3a8d7e7eb59ac59b4ce98f4693cb9"}

[commit-bot: I haz the power, 2017-06-06 15:47:48]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1496761395734250,
"parent_rev": "54316cef205352d84ee18a4fdc67eabc5d9c7a9d", "commit_rev":
"a99122347afb681ff46cf7a78c68a44f64549606"}

[commit-bot: I haz the power, 2017-06-06 15:47:57]

Description was changed from

==========
Replace assets for avatar placeholder on Android

This CL updates pictures used when there's no user avatar available.
Before, there were two set of PNGs:
1. account_management_no_picture.png (40dp), used in settings (SigninPreference
   and AccountManagementFragment), AutoSigninSnackbarController
   and AccountChooserDialog.
2. fre_placeholder.png (130dp), used in AccountSigninView.
This CL replaces these assets with logo_avatar_anonymous.xml vector drawable,
reducing APK size and removing duplication.
The new asset also looks different than the old ones - it is gray in color
(old ones were blue) and uses different silhouette for avatar.

BUG=722910
==========

to

==========
Replace assets for avatar placeholder on Android

This CL updates pictures used when there's no user avatar available.
Before, there were two set of PNGs:
1. account_management_no_picture.png (40dp), used in settings (SigninPreference
   and AccountManagementFragment), AutoSigninSnackbarController
   and AccountChooserDialog.
2. fre_placeholder.png (130dp), used in AccountSigninView.
This CL replaces these assets with logo_avatar_anonymous.xml vector drawable,
reducing APK size and removing duplication.
The new asset also looks different than the old ones - it is gray in color
(old ones were blue) and uses different silhouette for avatar.

BUG=722910

Review-Url: https://codereview.chromium.org/2912083003
Cr-Commit-Position: refs/heads/master@{#477301}
Committed:
https://chromium.googlesource.com/chromium/src/+/a99122347afb681ff46cf7a78c68...
==========

[commit-bot: I haz the power, 2017-06-06 15:47:58]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/a99122347afb681ff46cf7a78c68...