Create launcher app list for shell windows

Create launcher app list for shell windows


BUG=189727
For launcher_view.cc:
TBR=sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190445

[jeremya, 2013-03-22 00:13:31]

c/b/ui/extensions LGTM

[Mr4D (OOO till 08-26), 2013-03-22 00:45:31]

lgtm

Please address before submitting!

https://codereview.chromium.org/13007002/diff/2001/ash/launcher/launcher_view.cc
File ash/launcher/launcher_view.cc (right):

https://codereview.chromium.org/13007002/diff/2001/ash/launcher/launcher_view...
ash/launcher/launcher_view.cc:1371: break;
That is totally odd. I have fixed this already several times in CL's and somehow
the spacing always comes back again... Hope it stays now!

https://codereview.chromium.org/13007002/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc
(right):

https://codereview.chromium.org/13007002/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc:66:
observed_windows_.Remove(window);
You should possibly check if the removed window is the
"last_active_shell_window_" and if it is either setting it to NULL or find the
next one in line.

https://codereview.chromium.org/13007002/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc:162:
index == 0 /* has_leading_separator */));
You can ignore this - but shouldn't the comments both be of // type to be
consistent?

[stevenjb, 2013-03-22 19:18:33]

https://codereview.chromium.org/13007002/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc
(right):

https://codereview.chromium.org/13007002/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc:66:
observed_windows_.Remove(window);
On 2013/03/22 00:45:31, Mr4D wrote:
> You should possibly check if the removed window is the
> "last_active_shell_window_" and if it is either setting it to NULL or find the
> next one in line.
Good catch, thanks. NULL should be fine.

https://codereview.chromium.org/13007002/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc:162:
index == 0 /* has_leading_separator */));
On 2013/03/22 00:45:31, Mr4D wrote:
> You can ignore this - but shouldn't the comments both be of // type to be
> consistent?
Different types of comments. /* */ is a format that we use specifically for
describing unobvious arguments like "true" or "foo == 0", although that not
longer appears to be in the style guide. I could use // since each is on its own
line here, but since it's the last line it would be less clear that the comment
applies just to that argument.

[commit-bot: I haz the power, 2013-03-22 19:52:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/13007002/9001

[commit-bot: I haz the power, 2013-03-22 19:53:05]

Presubmit check for 13007002-9001 failed and returned exit status 1.

INFO:root:Found 7 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ash/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    ash/launcher/launcher_view.cc

Presubmit checks took 1.2s to calculate.

[commit-bot: I haz the power, 2013-03-22 20:36:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/13007002/9001

[commit-bot: I haz the power, 2013-03-23 16:06:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/13007002/9001

[commit-bot: I haz the power, 2013-03-24 10:09:51]

Commit queue failed due to new patchset.

[commit-bot: I haz the power, 2013-03-24 10:10:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/13007002/29001

[commit-bot: I haz the power, 2013-03-24 11:22:11]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[stevenjb, 2013-03-25 18:36:34]

Committed patchset #5 manually as r190445 (presubmit successful).