Asynchronously load wallpapers when user pod is selected.

Asynchronously load wallpapers when user pod is selected.


On Alex device, loading wallpaper almost takes 1 second to complete and it blocking
UI thread and increase the login time.
This CL load wallpapers in worker pool to unblock Main UI thread and reduce the login
time. Also, issue 124339 wants to display wallpaper in login screen when user pod is
selected. This CL refactored some wallpaper code and hooked up user pod selection to wallpaper
change to support it.

BUG=122855
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=135641

[bshe, 2012-04-25 01:55:50]

Hi Nikita.

Could you please take a look at this CL? Thanks!

[Nikita (slow), 2012-04-25 16:47:47]

Overall looks good. Few nits and one comment on implementation that we should
delay loading of wallpaper when user is selected.

https://chromiumcodereview.appspot.com/10207030/diff/6001/ash/desktop_backgro...
File ash/desktop_background/desktop_background_controller.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/ash/desktop_backgro...
ash/desktop_background/desktop_background_controller.cc:29: wallpaper_ = NULL;
nit: align

https://chromiumcodereview.appspot.com/10207030/diff/6001/ash/desktop_backgro...
ash/desktop_background/desktop_background_controller.cc:38:
base::WorkerPool::PostTaskAndReply(
We should somehow cancel previous wallpaper load if it's in progress.
Or at least make sure that when OnWallpaperLoadCompleted would not be called by
that previous task i.e. preemption strategy is that new task always make sure
that previous task is cancelled and its result is ignored.

https://chromiumcodereview.appspot.com/10207030/diff/6001/ash/desktop_backgro...
ash/desktop_background/desktop_background_controller.cc:45: true);
nit:  /* task_is_slow */

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/chro...
File chrome/browser/chromeos/login/user_manager.h (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/chro...
chrome/browser/chromeos/login/user_manager.h:97: 
nit: add comment.

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/chro...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/chro...
chrome/browser/chromeos/login/user_manager_impl.cc:452: if (email == kGuestUser)
{
At this point we might ignore email == kGuestUser case as Chrome gets restarted
when Guest mode is launched.

We may go back to it when implementing wallpaper at login screen feature.

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/reso...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/reso...
chrome/browser/resources/chromeos/login/user_pod_row.js:650:
chrome.send('userSelected', [podToFocus.user.username]);
See http://crosbug.com/1124339 it mentions one specific issue when we quickly
move through users as sign in screen.

Imagine selecting 3rd user with keyboard. First two would get selected quickly
so we need some timeout after user pod gets focus.

1. User pod focused
2. Set timeout (issue mentions 800ms)
3. When timeout is fired, check that same user pod is still focused and call
chrome.send('userSelectedDelayed') if it is still focused.

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:443:
web_ui()->RegisterMessageCallback("userSelected",
Rename to userSelectedDelayed (per comment in JS).

[bshe, 2012-04-25 23:54:43]

Done. Could you take another look please? Thanks!

https://chromiumcodereview.appspot.com/10207030/diff/6001/ash/desktop_backgro...
File ash/desktop_background/desktop_background_controller.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/ash/desktop_backgro...
ash/desktop_background/desktop_background_controller.cc:29: wallpaper_ = NULL;
On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> nit: align

Done.

https://chromiumcodereview.appspot.com/10207030/diff/6001/ash/desktop_backgro...
ash/desktop_background/desktop_background_controller.cc:38:
base::WorkerPool::PostTaskAndReply(
On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> We should somehow cancel previous wallpaper load if it's in progress.
> Or at least make sure that when OnWallpaperLoadCompleted would not be called
by
> that previous task i.e. preemption strategy is that new task always make sure
> that previous task is cancelled and its result is ignored.

Done. The new wallpaper loading always make sure that previous task is cancelled
and its result is ignored.

https://chromiumcodereview.appspot.com/10207030/diff/6001/ash/desktop_backgro...
ash/desktop_background/desktop_background_controller.cc:45: true);
On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> nit:  /* task_is_slow */

Done.

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/chro...
File chrome/browser/chromeos/login/user_manager.h (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/chro...
chrome/browser/chromeos/login/user_manager.h:97: 
On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> nit: add comment.

Done.

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/chro...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/chro...
chrome/browser/chromeos/login/user_manager_impl.cc:452: if (email == kGuestUser)
{
On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> At this point we might ignore email == kGuestUser case as Chrome gets
restarted
> when Guest mode is launched.
> 
> We may go back to it when implementing wallpaper at login screen feature.

Done.

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/reso...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/reso...
chrome/browser/resources/chromeos/login/user_pod_row.js:650:
chrome.send('userSelected', [podToFocus.user.username]);
On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> See http://crosbug.com/1124339 it mentions one specific issue when we quickly
> move through users as sign in screen.
> 
> Imagine selecting 3rd user with keyboard. First two would get selected quickly
> so we need some timeout after user pod gets focus.
> 
> 1. User pod focused
> 2. Set timeout (issue mentions 800ms)
> 3. When timeout is fired, check that same user pod is still focused and call
> chrome.send('userSelectedDelayed') if it is still focused.

Done. I cleared the previous timeout and setup a new timeout when focused pod
changes. It should do the same (prevent loading wallpapers in between).
Although, 800ms seems a little too long to me, which combined with the 1 second
wallpaper loading time, makes the wallpaper changing sluggish (around 2 secs to
show the wallpaper). 

With the new preemption strategy, We can probably start to load the wallpaper
immediately. It only shows the wallpaper loaded successfully and discard all in
between wallpapers. This way we can reduce the wallpaper changing response time.
What do you think?

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:443:
web_ui()->RegisterMessageCallback("userSelected",
On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> Rename to userSelectedDelayed (per comment in JS).

Done.

[Nikita (slow), 2012-04-26 17:22:03]

lgtm

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/reso...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/reso...
chrome/browser/resources/chromeos/login/user_pod_row.js:650:
chrome.send('userSelected', [podToFocus.user.username]);
On 2012/04/25 23:54:43, bshe wrote:
> On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> > See http://crosbug.com/1124339 it mentions one specific issue when we
quickly
> > move through users as sign in screen.
> > 
> > Imagine selecting 3rd user with keyboard. First two would get selected
quickly
> > so we need some timeout after user pod gets focus.
> > 
> > 1. User pod focused
> > 2. Set timeout (issue mentions 800ms)
> > 3. When timeout is fired, check that same user pod is still focused and call
> > chrome.send('userSelectedDelayed') if it is still focused.
> 
> Done. I cleared the previous timeout and setup a new timeout when focused pod
> changes. It should do the same (prevent loading wallpapers in between).
> Although, 800ms seems a little too long to me, which combined with the 1
second
> wallpaper loading time, makes the wallpaper changing sluggish (around 2 secs
to
> show the wallpaper). 
> 
> With the new preemption strategy, We can probably start to load the wallpaper
> immediately. It only shows the wallpaper loaded successfully and discard all
in
> between wallpapers. This way we can reduce the wallpaper changing response
time.
> What do you think?

I still think that we should have some timeout and maybe experiment and decrease
it. Issue is that most time consuming operation won't be cancelled.

wallpaper_ = ui::ResourceBundle::GetSharedInstance().GetImageNamed(
  GetWallpaperInfo(index_).id).ToSkBitmap();

https://chromiumcodereview.appspot.com/10207030/diff/13001/chrome/browser/res...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/13001/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:14: var
WALLPAPER_LOAD_DELAY = 800;
nit: WALLPAPER_LOAD_DELAY_MS

[bshe, 2012-04-26 20:25:21]

Done. Thanks for reivew!

+OWNERS for owners approval
+ben for ash/*
+jhawkins for chrome/browser/ui/webui/*
and chrome/browser/resources/ 

Thanks!

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/reso...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/6001/chrome/browser/reso...
chrome/browser/resources/chromeos/login/user_pod_row.js:650:
chrome.send('userSelected', [podToFocus.user.username]);
On 2012/04/26 17:22:03, Nikita Kostylev wrote:
> On 2012/04/25 23:54:43, bshe wrote:
> > On 2012/04/25 16:47:48, Nikita Kostylev wrote:
> > > See http://crosbug.com/1124339 it mentions one specific issue when we
> quickly
> > > move through users as sign in screen.
> > > 
> > > Imagine selecting 3rd user with keyboard. First two would get selected
> quickly
> > > so we need some timeout after user pod gets focus.
> > > 
> > > 1. User pod focused
> > > 2. Set timeout (issue mentions 800ms)
> > > 3. When timeout is fired, check that same user pod is still focused and
call
> > > chrome.send('userSelectedDelayed') if it is still focused.
> > 
> > Done. I cleared the previous timeout and setup a new timeout when focused
pod
> > changes. It should do the same (prevent loading wallpapers in between).
> > Although, 800ms seems a little too long to me, which combined with the 1
> second
> > wallpaper loading time, makes the wallpaper changing sluggish (around 2 secs
> to
> > show the wallpaper). 
> > 
> > With the new preemption strategy, We can probably start to load the
wallpaper
> > immediately. It only shows the wallpaper loaded successfully and discard all
> in
> > between wallpapers. This way we can reduce the wallpaper changing response
> time.
> > What do you think?
> 
> I still think that we should have some timeout and maybe experiment and
decrease
> it. Issue is that most time consuming operation won't be cancelled.
> 
> wallpaper_ = ui::ResourceBundle::GetSharedInstance().GetImageNamed(
>   GetWallpaperInfo(index_).id).ToSkBitmap();
> 

Make sense. I will keep it this way and using 800 as a start. We can adjust it
later.

[James Hawkins, 2012-04-26 20:37:37]

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/res...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:11: var POD_WIDTH = 170
+ 2 * (10 + 10);
/** const */ var POD_WIDTH

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:463: // When quickly
move through users at login screen, we need to set a
nit: Grammar is off here, "When quickly move through".

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:659:
chrome.send('userSelectedDelayed', [podToFocus.user.username]);
nit: Indentation is off.

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:761: if
(!delegate_)
When would |delegate_| be NULL?

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h (right):

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h:79: virtual void
UserSelected(const std::string& username) = 0;
nit: Document method and |username|.

[Nikita (slow), 2012-04-27 12:00:47]

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:761: if
(!delegate_)
On 2012/04/26 20:37:37, James Hawkins wrote:
> When would |delegate_| be NULL?

See http://codereview.chromium.org/10213003/

[James Hawkins, 2012-04-27 17:14:18]

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:761: if
(!delegate_)
On 2012/04/27 12:00:49, Nikita Kostylev wrote:
> On 2012/04/26 20:37:37, James Hawkins wrote:
> > When would |delegate_| be NULL?
> 
> See http://codereview.chromium.org/10213003/

OK, then these circumstances need to be documented.

[bshe, 2012-04-27 18:07:21]

Done. Thanks for review!

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/res...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:11: var POD_WIDTH = 170
+ 2 * (10 + 10);
On 2012/04/26 20:37:37, James Hawkins wrote:
> /** const */ var POD_WIDTH

Done.

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:463: // When quickly
move through users at login screen, we need to set a
On 2012/04/26 20:37:37, James Hawkins wrote:
> nit: Grammar is off here, "When quickly move through".

Done.

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:659:
chrome.send('userSelectedDelayed', [podToFocus.user.username]);
On 2012/04/26 20:37:37, James Hawkins wrote:
> nit: Indentation is off.

Done.

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:761: if
(!delegate_)
It seems there are a few places in this file use this check and one of them
(http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/ui/webui/ch...)
has comment about when this delegate_ is null. But I am not entirely sure if
this is the same situation since I am not familiar with the code.
+nikita could you please take a look? 
On 2012/04/27 17:14:18, James Hawkins wrote:
> On 2012/04/27 12:00:49, Nikita Kostylev wrote:
> > On 2012/04/26 20:37:37, James Hawkins wrote:
> > > When would |delegate_| be NULL?
> > 
> > See http://codereview.chromium.org/10213003/
> 
> OK, then these circumstances need to be documented.

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h (right):

https://chromiumcodereview.appspot.com/10207030/diff/19001/chrome/browser/ui/...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h:79: virtual void
UserSelected(const std::string& username) = 0;
On 2012/04/26 20:37:37, James Hawkins wrote:
> nit: Document method and |username|.

Done.

[James Hawkins, 2012-04-27 19:29:03]

OK. LGTM aside from the |delegate_| situation.

https://chromiumcodereview.appspot.com/10207030/diff/14026/chrome/browser/res...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/14026/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:473: // When moving
through users quickly at login screen, we need to set a
nit: Don't use pronouns, e.g. 'we', in comments; pronouns are ambiguous.

[bshe, 2012-04-30 15:10:59]

done. Thanks!

+ben ping?

https://chromiumcodereview.appspot.com/10207030/diff/14026/chrome/browser/res...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://chromiumcodereview.appspot.com/10207030/diff/14026/chrome/browser/res...
chrome/browser/resources/chromeos/login/user_pod_row.js:473: // When moving
through users quickly at login screen, we need to set a
On 2012/04/27 19:29:03, James Hawkins wrote:
> nit: Don't use pronouns, e.g. 'we', in comments; pronouns are ambiguous.

Done.

[bshe, 2012-04-30 22:20:04]

+nikita 

Could you please take another look at patchset 9?
chrome/browser/chromeos/login/user_manager_impl.*
I found that first time logged in user doesn't have a wallpaper after I remove
the dependency to LOGIN notification. Patchset 9 should fix it.

Thanks!

[Ben Goodger (Google), 2012-05-01 19:40:12]

http://codereview.chromium.org/10207030/diff/31002/ash/desktop_background/des...
File ash/desktop_background/desktop_background_controller.cc (right):

http://codereview.chromium.org/10207030/diff/31002/ash/desktop_background/des...
ash/desktop_background/desktop_background_controller.cc:41: void
LoadingWallpaper() {
I think you need to lock when you read/write to data you plan to be used from
multiple threads. See base::AutoLock.

[bshe, 2012-05-01 20:52:02]

Thanks for review!

http://codereview.chromium.org/10207030/diff/31002/ash/desktop_background/des...
File ash/desktop_background/desktop_background_controller.cc (right):

http://codereview.chromium.org/10207030/diff/31002/ash/desktop_background/des...
ash/desktop_background/desktop_background_controller.cc:41: void
LoadingWallpaper() {
I thought the cancel_flag_ is already thread safe from the comment here 
http://code.google.com/searchframe#OAMlx_jo-ck/src/base/synchronization/cance...
But maybe I am wrong. Could you please confirm?

On 2012/05/01 19:40:12, Ben Goodger (Google) wrote:
> I think you need to lock when you read/write to data you plan to be used from
> multiple threads. See base::AutoLock.

[Nikita (slow), 2012-05-02 09:22:22]

lgtm

http://codereview.chromium.org/10207030/diff/31002/ash/desktop_background/des...
File ash/desktop_background/desktop_background_controller.cc (right):

http://codereview.chromium.org/10207030/diff/31002/ash/desktop_background/des...
ash/desktop_background/desktop_background_controller.cc:41: void
LoadingWallpaper() {
On 2012/05/01 20:52:02, bshe wrote:
> I thought the cancel_flag_ is already thread safe from the comment here 
>
http://code.google.com/searchframe#OAMlx_jo-ck/src/base/synchronization/cance...
> But maybe I am wrong. Could you please confirm?
> 
> On 2012/05/01 19:40:12, Ben Goodger (Google) wrote:
> > I think you need to lock when you read/write to data you plan to be used
from
> > multiple threads. See base::AutoLock.
> 

I've also checked that CancellationFlag is thread-safe.

In this case you're in fact accessing
const SkBitmap* wallpaper_;
ImageLayout layout_;

from different threads but that access from UI thread is in fact done only after
WorkerPool task is finished so I think explicit use of AutoLock is not needed.

http://codereview.chromium.org/10207030/diff/31002/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

http://codereview.chromium.org/10207030/diff/31002/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/user_manager_impl.cc:963:
current_user_wallpaper_index_ = ash::GetDefaultWallpaperIndex();
Ideally we might start preloading some random wallpaper when "Add user" flow is
started from sign in screen, right?

In fact it is not that bad as user is shown an avatar selection so wallpaper
load would happen in the background during that time.

[Ben Goodger (Google), 2012-05-02 22:21:35]

LGTM then.

[bshe, 2012-05-03 01:01:59]

+nikita added a todo.

http://codereview.chromium.org/10207030/diff/31002/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

http://codereview.chromium.org/10207030/diff/31002/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/user_manager_impl.cc:963:
current_user_wallpaper_index_ = ash::GetDefaultWallpaperIndex();
On 2012/05/02 09:22:23, Nikita Kostylev wrote:
> Ideally we might start preloading some random wallpaper when "Add user" flow
is
> started from sign in screen, right?
> 
> In fact it is not that bad as user is shown an avatar selection so wallpaper
> load would happen in the background during that time.

I agree, ideally we should start loading wallpaper when "add user" flow started
and revert to last used wallpaper if user hit "cancel". Add a todo here.

[commit-bot: I haz the power, 2012-05-03 01:02:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/10207030/43002

[commit-bot: I haz the power, 2012-05-03 01:02:39]

Can't apply patch for file
ash/desktop_background/desktop_background_controller.cc.
While running patch -p1 --forward --force;
patching file ash/desktop_background/desktop_background_controller.cc
Hunk #2 FAILED at 22.
Hunk #3 succeeded at 59 (offset 4 lines).
1 out of 3 hunks FAILED -- saving rejects to file
ash/desktop_background/desktop_background_controller.cc.rej

[commit-bot: I haz the power, 2012-05-03 02:14:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/10207030/50026

[commit-bot: I haz the power, 2012-05-03 02:15:17]

Presubmit check for 10207030-50026 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

See the JavaScript style guide at
http://www.chromium.org/developers/web-development-style-guide#TOC-JavaScript
and if you have any feedback about the JavaScript PRESUBMIT check, contact
tbreisacher@chromium.org

** Presubmit Warnings **
Found JavaScript style violations in
chrome/browser/resources/chromeos/login/user_pod_row.js:
  line 95: E0001: Extra space after "function"
      this.removeUserButtonElement.addEventListener('mousedown', function (e) {
                                                                         ^
  line 468: E0001: Extra space before ":"
    focusedPod_ : undefined,
               ^
  line 519: E0001: Extra space before ":"
    disabled_ : false,
             ^
  line 720: E0200: Invalid JsDoc tag: public
     * @public
       ^^^^^^^
  line 731: E0200: Invalid JsDoc tag: public
    * @public
      ^^^^^^^
  line 797: E0200: Invalid JsDoc tag: public
     * @public
       ^^^^^^^

Presubmit checks took 3.2s to calculate.

[commit-bot: I haz the power, 2012-05-03 02:20:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/10207030/52026

[commit-bot: I haz the power, 2012-05-03 02:45:33]

Try job failure for 10207030-52026 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-05-07 13:55:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/10207030/65008

[commit-bot: I haz the power, 2012-05-07 15:18:59]

Change committed as 135641