Issue 8541002: [cros] Initial user image loading and repeated profile image loading fixes.

Issue 8541002: [cros] Initial user image loading and repeated profile image loading fixes. (Closed)

Created:
9 years, 1 month ago by Ivan Korotkov

Modified:
9 years, 1 month ago

Reviewers:
whywhat

CC:
chromium-reviews, arv (Not doing code reviews), stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

[cros] Initial user image loading and repeated profile image loading fixes. BUG=22551 TEST=Manual Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109642

Patch Set 2 : Whitespace #

Created: 9 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+128 lines, -83 lines)			Patch
M	chrome/browser/chromeos/login/default_user_images.h	View		1 chunk	+8 lines, -1 line	0 comments	Download
M	chrome/browser/chromeos/login/default_user_images.cc	View	1	2 chunks	+8 lines, -0 lines	0 comments	Download
M	chrome/browser/chromeos/login/profile_image_downloader.h	View	1	2 chunks	+3 lines, -2 lines	0 comments	Download
M	chrome/browser/chromeos/login/profile_image_downloader.cc	View		2 chunks	+1 line, -3 lines	0 comments	Download
M	chrome/browser/chromeos/login/user_manager.h	View	1	3 chunks	+17 lines, -14 lines	0 comments	Download
M	chrome/browser/chromeos/login/user_manager.cc	View		11 chunks	+53 lines, -34 lines	0 comments	Download
M	chrome/browser/resources/options/chromeos/change_picture_options.js	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/webui/options/chromeos/change_picture_options_handler.h	View	1	2 chunks	+10 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/options/chromeos/change_picture_options_handler.cc	View		8 chunks	+27 lines, -26 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

whywhat

lgtm http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/default_user_images.cc File chrome/browser/chromeos/login/default_user_images.cc (right): http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/default_user_images.cc#newcode14 chrome/browser/chromeos/login/default_user_images.cc:14: include SkBitmap.h? http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/default_user_images.cc#newcode97 chrome/browser/chromeos/login/default_user_images.cc:97: const SkBitmap& GetDefaultImage(int index) ...

9 years, 1 month ago (2011-11-11 15:40:42 UTC) #2

Ivan Korotkov

9 years, 1 month ago (2011-11-11 16:08:00 UTC) #3

http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/d...
File chrome/browser/chromeos/login/default_user_images.cc (right):

http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/d...
chrome/browser/chromeos/login/default_user_images.cc:14: 
On 2011/11/11 15:40:42, whywhat wrote:
> include SkBitmap.h?

Done.

http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/d...
chrome/browser/chromeos/login/default_user_images.cc:97: const SkBitmap&
GetDefaultImage(int index) {
Blessed are we.

http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/p...
File chrome/browser/chromeos/login/profile_image_downloader.h (right):

http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/p...
chrome/browser/chromeos/login/profile_image_downloader.h:38: //
|ProfileImageDownloader| instance may be deleted in any of these handlers.
Rephrased.

http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/p...
chrome/browser/chromeos/login/profile_image_downloader.h:59: // token is
available. Should not be called more than once.
I'd prefer to be consistent with most other loaders that don't start any work
immediately upon creation.

http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/u...
File chrome/browser/chromeos/login/user_manager.h (right):

http://codereview.chromium.org/8541002/diff/1/chrome/browser/chromeos/login/u...
chrome/browser/chromeos/login/user_manager.h:96: // Tries to load user image
from disk; after that, sets it for the user,
On 2011/11/11 15:40:42, whywhat wrote:
> nit: "after that," -> "then" or maybe "if succeeds, sets it for the user,"

Done.

http://codereview.chromium.org/8541002/diff/1/chrome/browser/ui/webui/options...
File chrome/browser/ui/webui/options/chromeos/change_picture_options_handler.h
(right):

http://codereview.chromium.org/8541002/diff/1/chrome/browser/ui/webui/options...
chrome/browser/ui/webui/options/chromeos/change_picture_options_handler.h:45: //
Sends the profile image to the page. If |should_select| is true, then
On 2011/11/11 15:40:42, whywhat wrote:
> No , before then

Done.

Expand Messages | Collapse Messages