Use base::RefCountedBytes in user_manager::UserImage

Use base::RefCountedBytes in user_manager::UserImage

To avoid data copies in many places.

BUG=593251
TEST=user image and wallpaper stuff works as before

Committed: https://crrev.com/41b4a8a89fc643c58f791f850230651f70a734b8
Cr-Commit-Position: refs/heads/master@{#434918}

[satorux1, 2016-11-22 04:58:27]

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

[commit-bot: I haz the power, 2016-11-22 04:58:41]

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

[satorux1, 2016-11-22 04:59:36]

satorux@chromium.org changed reviewers:
+ achuith@chromium.org

[commit-bot: I haz the power, 2016-11-22 05:46:32]

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

[commit-bot: I haz the power, 2016-11-22 05:46:33]

Dry run: This issue passed the CQ dry run.

[achuithb, 2016-11-22 21:16:55]

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/avatar/user_image_loader.cc (right):

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/avatar/user_image_loader.cc:75: if
(!encoded)
Does this early return still make sense?

Why not:
if (encoded)
  bitmap->swap(final_image);
return encoded;

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc
(right):

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc:156:
reinterpret_cast<const char*>(image_bytes->front()),
Do you still need the reinterpret_cast?

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/image_source.cc (right):

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/image_source.cc:44:
got_data_callback.Run(NULL);
nullptr?

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
File components/user_manager/user_image/user_image.cc (right):

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
components/user_manager/user_image/user_image.cc:57: : is_safe_format_(false) {
Move this to header since you're here?

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
components/user_manager/user_image/user_image.cc:69: is_safe_format_(false) {
ditto

[satorux1, 2016-11-24 02:13:41]

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/avatar/user_image_loader.cc (right):

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/avatar/user_image_loader.cc:75: if
(!encoded)
On 2016/11/22 21:16:54, achuithb wrote:
> Does this early return still make sense?
> 
> Why not:
> if (encoded)
>   bitmap->swap(final_image);
> return encoded;

good idea. done!

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc
(right):

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc:156:
reinterpret_cast<const char*>(image_bytes->front()),
On 2016/11/22 21:16:54, achuithb wrote:
> Do you still need the reinterpret_cast?

Yes, because RefCountedByte::front() is const unsigned char* whereas WriteFile
accepts const char*

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/image_source.cc (right):

https://codereview.chromium.org/2517053004/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/image_source.cc:44:
got_data_callback.Run(NULL);
On 2016/11/22 21:16:54, achuithb wrote:
> nullptr?

Fixed here and elsewhere in the file

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
File components/user_manager/user_image/user_image.cc (right):

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
components/user_manager/user_image/user_image.cc:57: : is_safe_format_(false) {
On 2016/11/22 21:16:54, achuithb wrote:
> Move this to header since you're here?

Maybe it's short enough to be in .h file, but I'd rather keep this in .cc file
in favor of not exposing implementation details of the constructor. I guess it's
a matter of taste. :)

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
components/user_manager/user_image/user_image.cc:69: is_safe_format_(false) {
On 2016/11/22 21:16:54, achuithb wrote:
> ditto

ditto

[satorux1, 2016-11-24 06:08:18]

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

[commit-bot: I haz the power, 2016-11-24 06:08:34]

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-11-24 08:09:19]

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

[commit-bot: I haz the power, 2016-11-24 08:09:20]

Dry run: Try jobs failed on following builders:
  blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[achuithb, 2016-11-28 09:01:10]

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
File components/user_manager/user_image/user_image.cc (right):

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
components/user_manager/user_image/user_image.cc:57: : is_safe_format_(false) {
On 2016/11/24 02:13:40, satorux1 wrote:
> On 2016/11/22 21:16:54, achuithb wrote:
> > Move this to header since you're here?
> 
> Maybe it's short enough to be in .h file, but I'd rather keep this in .cc file
> in favor of not exposing implementation details of the constructor. I guess
it's
> a matter of taste. :)

Maybe there's a misunderstanding - I meant the variable is_safe_format_ ought to
be initialized as a part of the class definition of UserImage in the header, in
accordance with the new c++11 guidelines on initialization of member variables:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/zqB-DySA4V0

[satorux1, 2016-11-28 23:09:16]

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
File components/user_manager/user_image/user_image.cc (right):

https://codereview.chromium.org/2517053004/diff/1/components/user_manager/use...
components/user_manager/user_image/user_image.cc:57: : is_safe_format_(false) {
On 2016/11/28 09:01:10, achuithb wrote:
> On 2016/11/24 02:13:40, satorux1 wrote:
> > On 2016/11/22 21:16:54, achuithb wrote:
> > > Move this to header since you're here?
> > 
> > Maybe it's short enough to be in .h file, but I'd rather keep this in .cc
file
> > in favor of not exposing implementation details of the constructor. I guess
> it's
> > a matter of taste. :)
> 
> Maybe there's a misunderstanding - I meant the variable is_safe_format_ ought
to
> be initialized as a part of the class definition of UserImage in the header,
in
> accordance with the new c++11 guidelines on initialization of member
variables:
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/zqB-DySA4V0

My bad. Didn't know about the c++11 guideline. Fixed!

[satorux1, 2016-11-28 23:18:32]

oops. forgot to upload. now uploaded!

[achuithb, 2016-11-28 23:19:22]

lgtm! Thanks!

[satorux1, 2016-11-28 23:21:22]

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

[commit-bot: I haz the power, 2016-11-28 23:22:31]

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-11-29 01:06:59]

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

[commit-bot: I haz the power, 2016-11-29 01:07:04]

Dry run: This issue passed the CQ dry run.

[satorux1, 2016-11-29 05:38:34]

The CQ bit was checked by satorux@chromium.org

[satorux1, 2016-11-29 05:38:34]

The patchset sent to the CQ was uploaded after l-g-t-m from achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/2517053004/#ps50001
(title: "remove unnecessary base::move")

[commit-bot: I haz the power, 2016-11-29 05:38:59]

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

[commit-bot: I haz the power, 2016-11-29 05:39:00]

This CL has an open dependency (Issue 2517413002 Patch 1). Please resolve the
dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[satorux1, 2016-11-29 05:48:49]

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

[commit-bot: I haz the power, 2016-11-29 05:49:06]

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-11-29 06:46:14]

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

[commit-bot: I haz the power, 2016-11-29 06:46:14]

Dry run: This issue passed the CQ dry run.

[satorux1, 2016-11-29 06:46:29]

The CQ bit was checked by satorux@chromium.org

[commit-bot: I haz the power, 2016-11-29 06:46:42]

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-11-29 06:52:32]

CQ is committing da patch.

Bot data: {"patchset_id": 50001, "attempt_start_ts": 1480401989946370,
"parent_rev": "ad4202b79387f43ada35c680ed0cc216802976ba", "commit_rev":
"367eedfced77eda0014904ed11faef6e6e346165"}

[commit-bot: I haz the power, 2016-11-29 06:53:01]

Committed patchset #4 (id:50001)

[commit-bot: I haz the power, 2016-11-29 06:57:01]

Description was changed from

==========
Use base::RefCountedBytes in user_manager::UserImage

To avoid data copies in many places.

BUG=593251
TEST=user image and wallpaper stuff works as before
==========

to

==========
Use base::RefCountedBytes in user_manager::UserImage

To avoid data copies in many places.

BUG=593251
TEST=user image and wallpaper stuff works as before

Committed: https://crrev.com/41b4a8a89fc643c58f791f850230651f70a734b8
Cr-Commit-Position: refs/heads/master@{#434918}
==========

[commit-bot: I haz the power, 2016-11-29 06:57:04]

Patchset 4 (id:??) landed as
https://crrev.com/41b4a8a89fc643c58f791f850230651f70a734b8
Cr-Commit-Position: refs/heads/master@{#434918}