Change GetBrowserItemIndex() to GetLauncherItemIndexForType()

Change GetBrowserItemIndex() to GetLauncherItemIndexForType()

Makes GetBrowserItemIndex() more general function to get item index.
Move this function to ./ash/launcher/launcher_model_util.

R=sky@chromium.org
BUG=NONE
TEST=browser_tests, ash_unittests

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

[simonhong_, 2013-10-16 14:04:56]

Dear sky,
Please take a look.

[simonhong_, 2013-10-22 07:56:34]

Kindly ping..

[sky, 2013-10-22 20:05:16]

https://codereview.chromium.org/27369004/diff/1/ash/shelf/shelf_util.cc
File ash/shelf/shelf_util.cc (right):

https://codereview.chromium.org/27369004/diff/1/ash/shelf/shelf_util.cc#newco...
ash/shelf/shelf_util.cc:13: DCHECK(type == TYPE_BROWSER_SHORTCUT || type ==
TYPE_APP_LIST);
Why does the type need to be one of these?

[simonhong_, 2013-10-22 20:22:35]

https://codereview.chromium.org/27369004/diff/1/ash/shelf/shelf_util.cc
File ash/shelf/shelf_util.cc (right):

https://codereview.chromium.org/27369004/diff/1/ash/shelf/shelf_util.cc#newco...
ash/shelf/shelf_util.cc:13: DCHECK(type == TYPE_BROWSER_SHORTCUT || type ==
TYPE_APP_LIST);
On 2013/10/22 20:05:17, sky wrote:
> Why does the type need to be one of these?

Because other types can have many items.
Do we need to handle other types, too?
If so, we can return an index of first item in the model.

[sky, 2013-10-22 20:43:57]

First index SG. Be sure and document it.

On Tue, Oct 22, 2013 at 1:22 PM,  <simon.hong81@gmail.com> wrote:
>
> https://codereview.chromium.org/27369004/diff/1/ash/shelf/shelf_util.cc
> File ash/shelf/shelf_util.cc (right):
>
>
https://codereview.chromium.org/27369004/diff/1/ash/shelf/shelf_util.cc#newco...
> ash/shelf/shelf_util.cc:13: DCHECK(type == TYPE_BROWSER_SHORTCUT || type
> == TYPE_APP_LIST);
> On 2013/10/22 20:05:17, sky wrote:
>>
>> Why does the type need to be one of these?
>
>
> Because other types can have many items.
> Do we need to handle other types, too?
> If so, we can return an index of first item in the model.
>
> https://codereview.chromium.org/27369004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[simonhong_, 2013-10-22 21:17:50]

Modified to return first index at patchset #3.
Please check again.

[sky, 2013-10-22 23:33:51]

https://codereview.chromium.org/27369004/diff/85001/ash/shelf/shelf_util.h
File ash/shelf/shelf_util.h (right):

https://codereview.chromium.org/27369004/diff/85001/ash/shelf/shelf_util.h#ne...
ash/shelf/shelf_util.h:9: #include "ash/launcher/launcher_types.h"
Sorry, one more. I think this should be in ash/launcher/launcher_model_util. It
isn't really part of the shelf per say.

[simonhong_, 2013-10-23 00:13:06]

https://codereview.chromium.org/27369004/diff/85001/ash/shelf/shelf_util.h
File ash/shelf/shelf_util.h (right):

https://codereview.chromium.org/27369004/diff/85001/ash/shelf/shelf_util.h#ne...
ash/shelf/shelf_util.h:9: #include "ash/launcher/launcher_types.h"
On 2013/10/22 23:33:52, sky wrote:
> Sorry, one more. I think this should be in ash/launcher/launcher_model_util.
It
> isn't really part of the shelf per say.

Moved from shelf/shelf_util to launcher/launcher_model_util.
Done.

[sky, 2013-10-23 13:26:10]

LGTM with the following change

https://codereview.chromium.org/27369004/diff/145001/chrome/browser/ui/ash/la...
File chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc
(right):

https://codereview.chromium.org/27369004/diff/145001/chrome/browser/ui/ash/la...
chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc:63:
size_t browser_index =
This should be an int.

[simonhong_, 2013-10-23 14:05:24]

https://codereview.chromium.org/27369004/diff/145001/chrome/browser/ui/ash/la...
File chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc
(right):

https://codereview.chromium.org/27369004/diff/145001/chrome/browser/ui/ash/la...
chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc:63:
size_t browser_index =
On 2013/10/23 13:26:10, sky wrote:
> This should be an int.

Done.
Thank you!

[commit-bot: I haz the power, 2013-10-23 14:06:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simon.hong81@gmail.com/27369004/255001

[commit-bot: I haz the power, 2013-10-23 14:43:42]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-23 14:53:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simon.hong81@gmail.com/27369004/445001

[commit-bot: I haz the power, 2013-10-23 17:16:27]

Change committed as 230456