Leftover wstring removal from r70290

Leftover wstring removal from r70290

This should fix compile in the touch buildbot.

BUG=touch buildbot is broken
TEST=see bug

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

[viettrungluu, 2010-12-30 05:21:16]

LGTM with comment (which you can feel free to ignore; certainly your changes
make things no worse).

http://codereview.chromium.org/6038006/diff/1/views/examples/table2_example.h
File views/examples/table2_example.h (right):

http://codereview.chromium.org/6038006/diff/1/views/examples/table2_example.h...
views/examples/table2_example.h:105: virtual int RowCount() {
Arguably, you should include base/compiler_specific.h and mark these with
OVERRIDE. Unfortunately, that only does anything on Windows right now. :(

http://codereview.chromium.org/6038006/diff/1/views/examples/table2_example.h...
views/examples/table2_example.h:110: std::string cells[5][5] = {
Comment: This may as well be made a const char* const cells[5][5], no?

(Making that conversion, it may or may not do the right thing for the last
element of each row. In any case, I find it somewhat disturbing that the rows
only have four entries each....)

http://codereview.chromium.org/6038006/diff/1/views/examples/table2_example.h...
views/examples/table2_example.h:115: { "Cantaloupe", "Orange", "South america",
"$5" },
(And, strictly speaking, C++ doesn't allow the trailing comma, IIRC....)

[viettrungluu, 2010-12-30 05:22:08]

You *should* #include <string>, however.

[sadrul, 2010-12-30 05:33:49]

Thanks for the quick review!

<string> is included via base/string_util.h

http://codereview.chromium.org/6038006/diff/1/views/examples/table2_example.h
File views/examples/table2_example.h (right):

http://codereview.chromium.org/6038006/diff/1/views/examples/table2_example.h...
views/examples/table2_example.h:105: virtual int RowCount() {
On 2010/12/30 05:21:16, viettrungluu wrote:
> Arguably, you should include base/compiler_specific.h and mark these with
> OVERRIDE. Unfortunately, that only does anything on Windows right now. :(

I am not really familiar with OVERRIDE. So I will skip this one for the moment.
:)

http://codereview.chromium.org/6038006/diff/1/views/examples/table2_example.h...
views/examples/table2_example.h:110: std::string cells[5][5] = {
On 2010/12/30 05:21:16, viettrungluu wrote:
> Comment: This may as well be made a const char* const cells[5][5], no?
> 
> (Making that conversion, it may or may not do the right thing for the last
> element of each row. In any case, I find it somewhat disturbing that the rows
> only have four entries each....)

Making it const char* const sounds like a good idea to me. So I have made that
change. I also changed the column-count to 4. It works OK.

http://codereview.chromium.org/6038006/diff/1/views/examples/table2_example.h...
views/examples/table2_example.h:115: { "Cantaloupe", "Orange", "South america",
"$5" },
On 2010/12/30 05:21:16, viettrungluu wrote:
> (And, strictly speaking, C++ doesn't allow the trailing comma, IIRC....)

Fixed.

[viettrungluu, 2010-12-30 05:40:03]

On 2010/12/30 05:33:49, sadrul wrote:
> Thanks for the quick review!
> 
> <string> is included via base/string_util.h

a) You probably shouldn't rely on transitive inclusions (unless all your uses of
std::string involve something from string_util.h).
b) It's not entirely obvious to my why string_util.h is included
c) ASCIIToUTF16() is declared in base/utf_string_conversions.h, whose inclusion
is missing....

[sadrul, 2010-12-30 05:49:37]

On 2010/12/30 05:40:03, viettrungluu wrote:
> On 2010/12/30 05:33:49, sadrul wrote:
> > Thanks for the quick review!
> > 
> > <string> is included via base/string_util.h
> 
> a) You probably shouldn't rely on transitive inclusions (unless all your uses
of
> std::string involve something from string_util.h).
> b) It's not entirely obvious to my why string_util.h is included
> c) ASCIIToUTF16() is declared in base/utf_string_conversions.h, whose
inclusion
> is missing....

Noted. Included <string> and base/utf_string_conversions.h and removed
string_util.h since it doesn't seem to be necessary.

[viettrungluu, 2010-12-30 06:01:33]

Thanks for fixing the #includes.

On Wed, Dec 29, 2010 at 21:49,  <sadrul@chromium.org> wrote:
> On 2010/12/30 05:40:03, viettrungluu wrote:
>>
>> On 2010/12/30 05:33:49, sadrul wrote:
>> > Thanks for the quick review!
>> >
>> > <string> is included via base/string_util.h
>
>> a) You probably shouldn't rely on transitive inclusions (unless all your
>> uses
>
> of
>>
>> std::string involve something from string_util.h).
>> b) It's not entirely obvious to my why string_util.h is included
>> c) ASCIIToUTF16() is declared in base/utf_string_conversions.h, whose
>
> inclusion
>>
>> is missing....
>
> Noted. Included <string> and base/utf_string_conversions.h and removed
> string_util.h since it doesn't seem to be necessary.
>
> http://codereview.chromium.org/6038006/
>