Add LauncherItem for TaskManager Dialog

Add LauncherItem for TaskManager Dialog

R=sky@chromium.org
BUG=121242
TEST=visual test

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

[simonhong, 2014-01-10 02:38:51]

Dear sky,
Please take a look.

[tfarina, 2014-01-10 02:45:01]

https://codereview.chromium.org/132543003/diff/30001/chrome/browser/ui/views/...
File chrome/browser/ui/views/task_manager_view.cc (right):

https://codereview.chromium.org/132543003/diff/30001/chrome/browser/ui/views/...
chrome/browser/ui/views/task_manager_view.cc:530:
CreateShelfItemForDialog(IDR_AURA_LAUNCHER_ICON_TASK_MANAGER,
I'm not sure this solo function needs a separate source/header file. Can't you
do it here in an unnamed namespace? Of course, Scott has the final word here.
Just wanted to brought this question to you. So wait for his review first.

[simonhong, 2014-01-10 02:55:11]

Dear tfarina,
Thank you for review!

https://codereview.chromium.org/132543003/diff/30001/chrome/browser/ui/views/...
File chrome/browser/ui/views/task_manager_view.cc (right):

https://codereview.chromium.org/132543003/diff/30001/chrome/browser/ui/views/...
chrome/browser/ui/views/task_manager_view.cc:530:
CreateShelfItemForDialog(IDR_AURA_LAUNCHER_ICON_TASK_MANAGER,
On 2014/01/10 02:45:01, tfarina wrote:
> I'm not sure this solo function needs a separate source/header file. Can't you
> do it here in an unnamed namespace? Of course, Scott has the final word here.
> Just wanted to brought this question to you. So wait for his review first.

I made this function as a separate files for other dialogs.
Maybe, some other dialogs like this task manager can want to make their
ShelfItem.

[sky, 2014-01-10 19:03:41]

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/launcher_item_util.h (right):

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/launcher_item_util.h:14: // Create ShelfItem for
|window|. Its status update and detroying is managed by
Creates an item on the shelf for |window|. The item is destroyed when the window
is destroyed.

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/launcher_item_util.h:17: base::string16 title,
const&.
Also, why do we need a title? Can't you use window->title()?

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/task_manager_view.cc (right):

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/task_manager_view.cc:530:
CreateShelfItemForDialog(IDR_AURA_LAUNCHER_ICON_TASK_MANAGER,
I think this should be in NativeWidgetAura.

[simonhong, 2014-01-10 23:14:58]

Dear sky,
Please check again.

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/launcher_item_util.h (right):

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/launcher_item_util.h:14: // Create ShelfItem for
|window|. Its status update and detroying is managed by
On 2014/01/10 19:03:42, sky wrote:
> Creates an item on the shelf for |window|. The item is destroyed when the
window
> is destroyed.

Done.

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/launcher_item_util.h:17: base::string16 title,
On 2014/01/10 19:03:42, sky wrote:
> const&.
> Also, why do we need a title? Can't you use window->title()?

Done.

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/task_manager_view.cc (right):

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/task_manager_view.cc:530:
CreateShelfItemForDialog(IDR_AURA_LAUNCHER_ICON_TASK_MANAGER,
On 2014/01/10 19:03:42, sky wrote:
> I think this should be in NativeWidgetAura.

I tried but, I found that we can't call ash api
(ash::SetShelfItemDetailsForWindow()) from views(NativeWidgetAura) namespace.
Or, do I mis-understand your suggestion?

[sky, 2014-01-13 14:09:35]

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/task_manager_view.cc (right):

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/task_manager_view.cc:530:
CreateShelfItemForDialog(IDR_AURA_LAUNCHER_ICON_TASK_MANAGER,
On 2014/01/10 23:14:58, simonhong wrote:
> On 2014/01/10 19:03:42, sky wrote:
> > I think this should be in NativeWidgetAura.
> 
> I tried but, I found that we can't call ash api
> (ash::SetShelfItemDetailsForWindow()) from views(NativeWidgetAura) namespace.
> Or, do I mis-understand your suggestion?

The way we do this sort of thing is to put an interface in aura::client that
views uses. That way there isn't a direct dependency on ash. But go ahead and
leave it here for now.

[simonhong, 2014-01-13 21:19:30]

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/task_manager_view.cc (right):

https://codereview.chromium.org/132543003/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/task_manager_view.cc:530:
CreateShelfItemForDialog(IDR_AURA_LAUNCHER_ICON_TASK_MANAGER,
On 2014/01/13 14:09:35, sky wrote:
> On 2014/01/10 23:14:58, simonhong wrote:
> > On 2014/01/10 19:03:42, sky wrote:
> > > I think this should be in NativeWidgetAura.
> > 
> > I tried but, I found that we can't call ash api
> > (ash::SetShelfItemDetailsForWindow()) from views(NativeWidgetAura)
namespace.
> > Or, do I mis-understand your suggestion?
> 
> The way we do this sort of thing is to put an interface in aura::client that
> views uses. That way there isn't a direct dependency on ash. But go ahead and
> leave it here for now.

Thanks, sky
I added TODO comments for this.

[simonhong, 2014-01-16 21:47:15]

Dear sky,
Any more comments for this?

[sky, 2014-01-16 21:59:46]

LGTM

[commit-bot: I haz the power, 2014-01-16 22:03:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhong@chromium.org/132543003/220001

[commit-bot: I haz the power, 2014-01-16 23:14:11]

Retried try job too often on linux_chromeos for step(s) app_list_unittests,
ash_unittests, aura_unittests, base_unittests, browser_tests,
cacheinvalidation_unittests, check_deps, check_deps2git, chromeos_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
dbus_unittests, device_unittests, events_unittests, google_apis_unittests,
gpu_unittests, interactive_ui_tests, ipc_tests, jingle_unittests,
media_unittests, net_unittests, ppapi_unittests, printing_unittests,
sandbox_linux_unittests, sql_unittests, sync_unit_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2014-01-17 10:41:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhong@chromium.org/132543003/220001

[commit-bot: I haz the power, 2014-01-17 12:47:24]

Retried try job too often on win_x64_rel for step(s) base_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[commit-bot: I haz the power, 2014-01-17 13:03:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhong@chromium.org/132543003/220001

[commit-bot: I haz the power, 2014-01-17 14:41:19]

Retried try job too often on win_rel for step(s) net_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-17 19:02:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhong@chromium.org/132543003/220001

[commit-bot: I haz the power, 2014-01-17 20:36:01]

Change committed as 245604