Return blank strings for Views Combobox separator items.

Return blank strings for Views Combobox separator items.

Prevents a DCHECK cited in the bug and follows good practice.
(don't ask ComboboxModel impls for separator item names)
Add a unit test that fails without the above change.

BUG=308666
TEST=No DCHECK on debug build for typing "other" then "book" in bookmark bubble folder combobox.
R=sky@chromium.org

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

[msw, 2013-10-17 20:23:07]

Hey Scott, please take a look; thanks!

[sky, 2013-10-17 20:58:54]

https://codereview.chromium.org/26540004/diff/6001/ui/views/controls/combobox...
File ui/views/controls/combobox/combobox_unittest.cc (right):

https://codereview.chromium.org/26540004/diff/6001/ui/views/controls/combobox...
ui/views/controls/combobox/combobox_unittest.cc:319:
EXPECT_TRUE(combobox_->GetTextForRow(i).empty());
Would be nice if the model here returned something other than an empty string
too.

[msw, 2013-10-17 21:25:46]

Hope that's better. Please take another look, thanks!

https://codereview.chromium.org/26540004/diff/6001/ui/views/controls/combobox...
File ui/views/controls/combobox/combobox_unittest.cc (right):

https://codereview.chromium.org/26540004/diff/6001/ui/views/controls/combobox...
ui/views/controls/combobox/combobox_unittest.cc:319:
EXPECT_TRUE(combobox_->GetTextForRow(i).empty());
On 2013/10/17 20:58:54, sky wrote:
> Would be nice if the model here returned something other than an empty string
> too.

Done.

[sky, 2013-10-18 00:35:24]

Much, thanks. LGTM

https://codereview.chromium.org/26540004/diff/18001/ui/views/controls/combobo...
File ui/views/controls/combobox/combobox_unittest.cc (right):

https://codereview.chromium.org/26540004/diff/18001/ui/views/controls/combobo...
ui/views/controls/combobox/combobox_unittest.cc:320:
EXPECT_TRUE(combobox_->GetTextForRow(i).empty());
For loops like this it's best to have << i (or something like that), so that if
there is a failure you know the index.

[msw, 2013-10-18 01:07:58]

Comment addressed; landing.

https://codereview.chromium.org/26540004/diff/18001/ui/views/controls/combobo...
File ui/views/controls/combobox/combobox_unittest.cc (right):

https://codereview.chromium.org/26540004/diff/18001/ui/views/controls/combobo...
ui/views/controls/combobox/combobox_unittest.cc:320:
EXPECT_TRUE(combobox_->GetTextForRow(i).empty());
On 2013/10/18 00:35:24, sky wrote:
> For loops like this it's best to have << i (or something like that), so that
if
> there is a failure you know the index.

Done.

[commit-bot: I haz the power, 2013-10-18 01:15:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/26540004/46001

[commit-bot: I haz the power, 2013-10-18 01:26:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/26540004/46001

[commit-bot: I haz the power, 2013-10-18 02:04:26]

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...

[commit-bot: I haz the power, 2013-10-18 04:56:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/26540004/46001

[commit-bot: I haz the power, 2013-10-18 20:36:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/26540004/408001

[commit-bot: I haz the power, 2013-10-18 21:11:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/26540004/408001

[commit-bot: I haz the power, 2013-10-19 02:14:22]

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

[commit-bot: I haz the power, 2013-10-19 04:37:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/26540004/408001

[commit-bot: I haz the power, 2013-10-19 13:43:10]

Change committed as 229550