Fix gcc 4.7 building problems - cont'

Fix gcc 4.7 building problems.

Narrowing conversion in initiliazation list now raises an 'ill-formed
conversion' warning, which causes error when -Werror is given, fixed
by:

  - adding static_cast for narrowing conversion in simple(short)
    initiliazation lists

  - adding '-Wno-narrowing' flag to compiler for long/bulk
    initilization lists

BUG=None
TEST=Built successfully using GCC-4.7 under linux

[Han, 2012-06-06 21:27:26]

Hi can you take a look at this CL?

Scott - 'chrome/'
Brett - 'content/'
Ami - 'media/'

Thanks,
-Han

[Ami GONE FROM CHROMIUM, 2012-06-06 21:34:08]

https://chromiumcodereview.appspot.com/10537037/diff/1/media/base/simd/conver...
File media/base/simd/convert_rgb_to_yuv_sse2.cc (right):

https://chromiumcodereview.appspot.com/10537037/diff/1/media/base/simd/conver...
media/base/simd/convert_rgb_to_yuv_sse2.cc:27: -INT16_FIX(0.071),
-INT16_FIX(0.368), INT16_FIX(0.439), 0,
nit: add leading space

https://chromiumcodereview.appspot.com/10537037/diff/1/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10537037/diff/1/media/media.gyp#newcode16
media/media.gyp:16: 'cflags_cc+': ['-Wno-narrowing'],
I don't understand why you need this given the INT16_FIX changes you made.

https://chromiumcodereview.appspot.com/10537037/diff/1/media/media.gyp#newcod...
media/media.gyp:625: 'sources': [
nit: remove new space

[brettw, 2012-06-06 21:52:44]

https://chromiumcodereview.appspot.com/10537037/diff/1/content/content.gyp
File content/content.gyp (right):

https://chromiumcodereview.appspot.com/10537037/diff/1/content/content.gyp#ne...
content/content.gyp:82: 'cflags+': ['-Wno-narrowing'],
I'm trying to understand why this is necessary in content and not in other
places like Chrome. Is it that we just haven't gotten that far in the process of
conversion, or is there something about content that makes this more necessary?

[Han, 2012-06-06 22:11:25]

Hi Brett, thanks!

https://chromiumcodereview.appspot.com/10537037/diff/1/content/content.gyp
File content/content.gyp (right):

https://chromiumcodereview.appspot.com/10537037/diff/1/content/content.gyp#ne...
content/content.gyp:82: 'cflags+': ['-Wno-narrowing'],
On 2012/06/06 21:52:44, brettw wrote:
> I'm trying to understand why this is necessary in content and not in other
> places like Chrome. Is it that we just haven't gotten that far in the process
of
> conversion, or is there something about content that makes this more
necessary?

Hi Brett thans! The thing is this -
"content/browser/renderer_host/render_widget_host_view_gtk.cc" includes
"webkit/glue/webcursor_gtk_data.h", which contains a series of "bulk"
initialization lists. I'd rather add '-Wno-narrowing' flag than fix the
narrow-conversion problem for every value inside this webcursor_gtk_data.h file.

So I have to added this "-Wno-narrowing" to those ("content/" and "media/" in
this CL, and some others in my previous CL) that include this head file.

[Peter Kasting, 2012-06-06 22:18:13]

On 2012/06/06 22:11:25, Han wrote:
> Hi Brett thans! The thing is this -
> "content/browser/renderer_host/render_widget_host_view_gtk.cc" includes
> "webkit/glue/webcursor_gtk_data.h", which contains a series of "bulk"
> initialization lists. I'd rather add '-Wno-narrowing' flag than fix the
> narrow-conversion problem for every value inside this webcursor_gtk_data.h
file.

-Wno-narrowing is only a bandaid for this.  Someone should eventually fix all
the values the way you don't currently want to.  If you want to break this into
a second patch that's fine.  If you just don't want to do the work, that's not
an acceptable excuse.  Fix it anyway.

Now, I agree that adding thousands of static_cast<>()s sucks.  So the answer is
presumably to find a better way to do these.  Possibilities include macros,
changing the types of these arrays (to something other than char, e.g. int),
using some different mechanism to declare the values instead of hex constants,
writing initializer functions, pulling in the cursors via external image files,
using built-in cursors from GTK, etc.

[Han, 2012-06-06 22:18:23]

Hi Ami, thanks!

https://chromiumcodereview.appspot.com/10537037/diff/1/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10537037/diff/1/media/media.gyp#newcode16
media/media.gyp:16: 'cflags_cc+': ['-Wno-narrowing'],
On 2012/06/06 21:34:09, Ami Fischman wrote:
> I don't understand why you need this given the INT16_FIX changes you made.

Yup, right. Reverted this file.

https://chromiumcodereview.appspot.com/10537037/diff/1/media/media.gyp#newcod...
media/media.gyp:625: 'sources': [
On 2012/06/06 21:34:09, Ami Fischman wrote:
> nit: remove new space

Yup, right. Reverted this file.

[Ami GONE FROM CHROMIUM, 2012-06-06 22:19:47]

LGTM for media/

[brettw, 2012-06-06 22:21:24]

I agree with Peter, this will have to be done at some point anyway, so we may as
well do it now. These build system tweaks usually never get put back and nobody
remembers what they're for, so I'm very wary of adding any exceptions.

This is just a generated file of some data, so I don't think the static casts
are a problem. Just do search and replace and call it done.

[Han, 2012-06-06 23:04:16]

On 2012/06/06 22:21:24, brettw wrote:
> I agree with Peter, this will have to be done at some point anyway, so we may
as
> well do it now. These build system tweaks usually never get put back and
nobody
> remembers what they're for, so I'm very wary of adding any exceptions.
> 
> This is just a generated file of some data, so I don't think the static casts
> are a problem. Just do search and replace and call it done.

Hi Brett and Peter. Yup, I've reverted "-Wno-narrowing" change in gyp file. And
modified the date file accordingly. Please take a look, thanks!

-Han

[brettw, 2012-06-06 23:05:38]

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
File webkit/glue/webcursor_gtk_data.h (right):

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
webkit/glue/webcursor_gtk_data.h:35: #define CC(X) (X>=0x80 ?
static_cast<char>(X) : X)
Is there any reason not to static_cast all the time?

[Peter Kasting, 2012-06-06 23:07:33]

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
File webkit/glue/webcursor_gtk_data.h (right):

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
webkit/glue/webcursor_gtk_data.h:35: #define CC(X) (X>=0x80 ?
static_cast<char>(X) : X)
On 2012/06/06 23:05:38, brettw wrote:
> Is there any reason not to static_cast all the time?

Yeah, let's just do this.

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
webkit/glue/webcursor_gtk_data.h:39: CC(0x00), CC(0x00), CC(0x00), CC(0x00),
CC(0x02), CC(0x40), CC(0x00), CC(0x00), CC(0x02), CC(0x40), CC(0x00), CC(0x00),
Probably want to break these lines after 4 entries to avoid 80-col issues.

[Han, 2012-06-06 23:11:40]

Hi Brett, thanks!

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
File webkit/glue/webcursor_gtk_data.h (right):

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
webkit/glue/webcursor_gtk_data.h:35: #define CC(X) (X>=0x80 ?
static_cast<char>(X) : X)
On 2012/06/06 23:05:38, brettw wrote:
> Is there any reason not to static_cast all the time?
Hi Brett,

char a = 0x10; // 0x10 is within char range, no cast needed, no warning
char a = 0x90; // 0x90 is beyond char range, needs a cast from int (or short) to
char.

Adding static_cast to all is ok, but to be clearer, I make it this way.

[Han, 2012-06-06 23:15:19]

Hi Peter, thanks!

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
File webkit/glue/webcursor_gtk_data.h (right):

https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
webkit/glue/webcursor_gtk_data.h:35: #define CC(X) (X>=0x80 ?
static_cast<char>(X) : X)
On 2012/06/06 23:07:33, Peter Kasting wrote:
> On 2012/06/06 23:05:38, brettw wrote:
> > Is there any reason not to static_cast all the time?
> 
> Yeah, let's just do this.

Sorry, but you mean "apply static_cast to all" or "keep this CC macro".

[Peter Kasting, 2012-06-06 23:17:06]

On 2012/06/06 23:15:19, Han wrote:
> Hi Peter, thanks!
> 
>
https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
> File webkit/glue/webcursor_gtk_data.h (right):
> 
>
https://chromiumcodereview.appspot.com/10537037/diff/3003/webkit/glue/webcurs...
> webkit/glue/webcursor_gtk_data.h:35: #define CC(X) (X>=0x80 ?
> static_cast<char>(X) : X)
> On 2012/06/06 23:07:33, Peter Kasting wrote:
> > On 2012/06/06 23:05:38, brettw wrote:
> > > Is there any reason not to static_cast all the time?
> > 
> > Yeah, let's just do this.
> 
> Sorry, but you mean "apply static_cast to all" or "keep this CC macro".

Just static_cast<>() all the time.

[brettw, 2012-06-06 23:17:15]

Just use static_cast(). It does what you want. Separating it out implies to me
that you specifically don't want to static_cast in that case, which is then
confusing to me since static_cast has a well-defined meaning which is exactly
what you want here.

[Peter Kasting, 2012-06-06 23:19:11]

Also did you try using unsigned char (or uint8, or whatever) instead?  Seems
like then all the values would fit in the type and we wouldn't need any casting.

[Peter Kasting, 2012-06-06 23:22:23]

On 2012/06/06 23:19:11, Peter Kasting wrote:
> Also did you try using unsigned char (or uint8, or whatever) instead?  Seems
> like then all the values would fit in the type and we wouldn't need any
casting.

(BTW, if this doesn't work straight off, adding a trailing U to each constant
would probably make it work, but I wouldn't do this unless you have to.)

[Han, 2012-06-06 23:34:15]

On 2012/06/06 23:22:23, Peter Kasting wrote:
> On 2012/06/06 23:19:11, Peter Kasting wrote:
> > Also did you try using unsigned char (or uint8, or whatever) instead?  Seems
> > like then all the values would fit in the type and we wouldn't need any
> casting.
> 
> (BTW, if this doesn't work straight off, adding a trailing U to each constant
> would probably make it work, but I wouldn't do this unless you have to.)

Yeah, I changed those data type to "unsigned char", and did a re-build  to see
if it breaks anything else.

[Han, 2012-06-06 23:42:21]

On 2012/06/06 23:34:15, Han wrote:
> On 2012/06/06 23:22:23, Peter Kasting wrote:
> > On 2012/06/06 23:19:11, Peter Kasting wrote:
> > > Also did you try using unsigned char (or uint8, or whatever) instead? 
Seems
> > > like then all the values would fit in the type and we wouldn't need any
> > casting.
> > 
> > (BTW, if this doesn't work straight off, adding a trailing U to each
constant
> > would probably make it work, but I wouldn't do this unless you have to.)
> 
> Yeah, I changed those data type to "unsigned char", and did a re-build  to see
> if it breaks anything else.

No luck, raises lots of type error. I'll stick to static_cast....

[Peter Kasting, 2012-06-07 00:13:07]

On 2012/06/06 23:42:21, Han wrote:
> On 2012/06/06 23:34:15, Han wrote:
> > On 2012/06/06 23:22:23, Peter Kasting wrote:
> > > On 2012/06/06 23:19:11, Peter Kasting wrote:
> > > > Also did you try using unsigned char (or uint8, or whatever) instead? 
> Seems
> > > > like then all the values would fit in the type and we wouldn't need any
> > > casting.
> > > 
> > > (BTW, if this doesn't work straight off, adding a trailing U to each
> constant
> > > would probably make it work, but I wouldn't do this unless you have to.)
> > 
> > Yeah, I changed those data type to "unsigned char", and did a re-build  to
see
> > if it breaks anything else.
> 
> No luck, raises lots of type error. I'll stick to static_cast....

What are the errors?  Without knowing what they are I can't respond.

[Han, 2012-06-07 00:51:29]

On 2012/06/07 00:13:07, Peter Kasting wrote:
> On 2012/06/06 23:42:21, Han wrote:
> > On 2012/06/06 23:34:15, Han wrote:
> > > On 2012/06/06 23:22:23, Peter Kasting wrote:
> > > > On 2012/06/06 23:19:11, Peter Kasting wrote:
> > > > > Also did you try using unsigned char (or uint8, or whatever) instead? 
> > Seems
> > > > > like then all the values would fit in the type and we wouldn't need
any
> > > > casting.
> > > > 
> > > > (BTW, if this doesn't work straight off, adding a trailing U to each
> > constant
> > > > would probably make it work, but I wouldn't do this unless you have to.)
> > > 
> > > Yeah, I changed those data type to "unsigned char", and did a re-build  to
> see
> > > if it breaks anything else.
> > 
> > No luck, raises lots of type error. I'll stick to static_cast....
> 
> What are the errors?  Without knowing what they are I can't respond.

Hi Peter, I changed the data type to "static const unsigned char", the error was
- 
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
./webkit/glue/webcursor_gtk_data.h:279:1: error: invalid conversion from
‘unsigned char const*’ to ‘char const*’ [-fpermissive]
content/browser/renderer_host/render_widget_host_view_gtk.cc: In function
‘GdkCursor* (anonymous namespace)::GetMozSpinningCursor()’:
content/browser/renderer_host/render_widget_host_view_gtk.cc:86:68: error:
invalid conversion from ‘unsigned char const*’ to ‘const gchar* {aka char
const*}’ [-fperm
issive]
In file included from /usr/include/gtk-2.0/gdk/gdk.h:49:0,
                 from
./content/browser/renderer_host/render_widget_host_view_gtk.h:9,
                 from
content/browser/renderer_host/render_widget_host_view_gtk.cc:5:
/usr/include/gtk-2.0/gdk/gdkpixmap.h:73:12: error:   initializing argument 2 of
‘GdkBitmap* gdk_bitmap_create_from_data(GdkDrawable*, const gchar*, gint, gint)’
[-fp
ermissive]
content/browser/renderer_host/render_widget_host_view_gtk.cc:88:73: error:
invalid conversion from ‘unsigned char const*’ to ‘const gchar* {aka char
const*}’ [-fperm
issive]
In file included from /usr/include/gtk-2.0/gdk/gdk.h:49:0,
                 from
./content/browser/renderer_host/render_widget_host_view_gtk.h:9,
                 from
content/browser/renderer_host/render_widget_host_view_gtk.cc:5:
/usr/include/gtk-2.0/gdk/gdkpixmap.h:73:12: error:   initializing argument 2 of
‘GdkBitmap* gdk_bitmap_create_from_data(GdkDrawable*, const gchar*, gint, gint)’
[-fp
ermissive]

[Peter Kasting, 2012-06-07 01:03:37]

On Wed, Jun 6, 2012 at 5:51 PM, <shenhan@google.com> wrote:

> Hi Peter, I changed the data type to "static const unsigned char", the
> error was
> -
> ./webkit/glue/webcursor_gtk_**data.h:279:1: error: invalid conversion from
> ‘unsigned char const*’ to ‘char const*’ [-fpermissive]
>

There are two possible solutions to these.  One is to change the types of
the |bits| and |mask_bits| members of CustomCursor from const char* to
match your new array type.  This may require changes in other code.  The
cruder solution is to add "reinterpret_cast<const char*>()" around the
names of the arrays in the CustomCursors[] initializer block.

content/browser/renderer_host/**render_widget_host_view_gtk.**cc: In
> function
> ‘GdkCursor* (anonymous namespace)::**GetMozSpinningCursor()’:


These remaining errors can be fixed by inserting a couple of
"reinterpret_cast<const gchar*>()" in render_widget_host_view_gtk.cc at
lines 86 and 88.

Incidentally, if you're going to change the type of these arrays anyway,
I'd change to "const uint8*" (for which you need basictypes.h) as "uint8"
is a clearer choice for something that represents a bit pattern as opposed
to a character.

Ultimately, the choice of what to do should be based not on what is the
easiest patch or smallest diff now, but what will result in the clearest
and safest code for the indefinite future.

PK

[Han, 2012-06-07 01:18:45]

On 2012/06/07 01:03:37, Peter Kasting wrote:
> On Wed, Jun 6, 2012 at 5:51 PM, <mailto:shenhan@google.com> wrote:
> 
> > Hi Peter, I changed the data type to "static const unsigned char", the
> > error was
> > -
> > ./webkit/glue/webcursor_gtk_**data.h:279:1: error: invalid conversion from
> > ‘unsigned char const*’ to ‘char const*’ [-fpermissive]
> >
> 
> There are two possible solutions to these.  One is to change the types of
> the |bits| and |mask_bits| members of CustomCursor from const char* to
> match your new array type.  This may require changes in other code.  The
> cruder solution is to add "reinterpret_cast<const char*>()" around the
> names of the arrays in the CustomCursors[] initializer block.
> 
> content/browser/renderer_host/**render_widget_host_view_gtk.**cc: In
> > function
> > ‘GdkCursor* (anonymous namespace)::**GetMozSpinningCursor()’:
> 
> 
> These remaining errors can be fixed by inserting a couple of
> "reinterpret_cast<const gchar*>()" in render_widget_host_view_gtk.cc at
> lines 86 and 88.
> 
> Incidentally, if you're going to change the type of these arrays anyway,
> I'd change to "const uint8*" (for which you need basictypes.h) as "uint8"
> is a clearer choice for something that represents a bit pattern as opposed
> to a character.
> 
> Ultimately, the choice of what to do should be based not on what is the
> easiest patch or smallest diff now, but what will result in the clearest
> and safest code for the indefinite future.
> 
> PK

Thanks, Peter. I'll prepare the CL tomorrow.

[sky, 2012-06-07 03:51:37]

I'm swapped myself with Aaron as he should review extensions changes.

[Aaron Boodman, 2012-06-07 07:40:03]

lgtm

http://codereview.chromium.org/10537037/diff/3003/chrome/browser/extensions/s...
File chrome/browser/extensions/settings/settings_frontend.cc (right):

http://codereview.chromium.org/10537037/diff/3003/chrome/browser/extensions/s...
chrome/browser/extensions/settings/settings_frontend.cc:95:
static_cast<size_t>(UINT_MAX),
std::numeric_limits<size_t>::max() ?

[Han, 2012-06-07 18:02:59]

Hi Aaron, thanks!

http://codereview.chromium.org/10537037/diff/3003/chrome/browser/extensions/s...
File chrome/browser/extensions/settings/settings_frontend.cc (right):

http://codereview.chromium.org/10537037/diff/3003/chrome/browser/extensions/s...
chrome/browser/extensions/settings/settings_frontend.cc:95:
static_cast<size_t>(UINT_MAX),
On 2012/06/07 07:40:03, Aaron Boodman wrote:
> std::numeric_limits<size_t>::max() ?

Good point, done.

[Han, 2012-06-07 18:05:15]

On 2012/06/07 01:03:37, Peter Kasting wrote:
> On Wed, Jun 6, 2012 at 5:51 PM, <mailto:shenhan@google.com> wrote:
> 
> > Hi Peter, I changed the data type to "static const unsigned char", the
> > error was
> > -
> > ./webkit/glue/webcursor_gtk_**data.h:279:1: error: invalid conversion from
> > ‘unsigned char const*’ to ‘char const*’ [-fpermissive]
> >
> 
> There are two possible solutions to these.  One is to change the types of
> the |bits| and |mask_bits| members of CustomCursor from const char* to
> match your new array type.  This may require changes in other code.  The
> cruder solution is to add "reinterpret_cast<const char*>()" around the
> names of the arrays in the CustomCursors[] initializer block.
> 
> content/browser/renderer_host/**render_widget_host_view_gtk.**cc: In
> > function
> > ‘GdkCursor* (anonymous namespace)::**GetMozSpinningCursor()’:
> 
> 
> These remaining errors can be fixed by inserting a couple of
> "reinterpret_cast<const gchar*>()" in render_widget_host_view_gtk.cc at
> lines 86 and 88.
> 
> Incidentally, if you're going to change the type of these arrays anyway,
> I'd change to "const uint8*" (for which you need basictypes.h) as "uint8"
> is a clearer choice for something that represents a bit pattern as opposed
> to a character.
> 
> Ultimately, the choice of what to do should be based not on what is the
> easiest patch or smallest diff now, but what will result in the clearest
> and safest code for the indefinite future.
> 
> PK

Hi Peter, I've changed the data type to "unsigned char" and made other necessary
changes. Please take a look, thanks!

-Han

[Peter Kasting, 2012-06-07 18:09:52]

LGTM

http://codereview.chromium.org/10537037/diff/5007/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_gtk.cc (right):

http://codereview.chromium.org/10537037/diff/5007/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_gtk.cc:86: NULL,
reinterpret_cast<const char*>(moz_spinning_bits), 32, 32);
Nit: Use const gchar* (2 places)

http://codereview.chromium.org/10537037/diff/5007/webkit/glue/webcursor_gtk_d...
File webkit/glue/webcursor_gtk_data.h (right):

http://codereview.chromium.org/10537037/diff/5007/webkit/glue/webcursor_gtk_d...
webkit/glue/webcursor_gtk_data.h:35: static const unsigned char
moz_vertical_text_bits[] = {
Nit: I still think uint8 would be a better type

http://codereview.chromium.org/10537037/diff/5007/webkit/glue/webcursor_gtk_d...
webkit/glue/webcursor_gtk_data.h:263: const char* bits;
Can you make these also be uint8* to avoid the casts?

[Han Shen, 2012-06-07 19:09:09]

Thank, Peter.

http://codereview.chromium.org/10537037/diff/5007/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_gtk.cc (right):

http://codereview.chromium.org/10537037/diff/5007/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_gtk.cc:86: NULL,
reinterpret_cast<const char*>(moz_spinning_bits), 32, 32);
On 2012/06/07 18:09:52, Peter Kasting wrote:
> Nit: Use const gchar* (2 places)

Done.

http://codereview.chromium.org/10537037/diff/5007/webkit/glue/webcursor_gtk_d...
File webkit/glue/webcursor_gtk_data.h (right):

http://codereview.chromium.org/10537037/diff/5007/webkit/glue/webcursor_gtk_d...
webkit/glue/webcursor_gtk_data.h:35: static const unsigned char
moz_vertical_text_bits[] = {
On 2012/06/07 18:09:52, Peter Kasting wrote:
> Nit: I still think uint8 would be a better type

Done.

http://codereview.chromium.org/10537037/diff/5007/webkit/glue/webcursor_gtk_d...
webkit/glue/webcursor_gtk_data.h:263: const char* bits;
On 2012/06/07 18:09:52, Peter Kasting wrote:
> Can you make these also be uint8* to avoid the casts?

Done.

[Peter Kasting, 2012-06-07 19:26:01]

LGTM, thanks!

[commit-bot: I haz the power, 2012-06-07 20:39:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/shenhan@google.com/10537037/5009

[commit-bot: I haz the power, 2012-06-07 20:39:03]

Presubmit check for 10537037-5009 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
  webkit/glue/webcursor_gtk_data.h

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/browser/renderer_host
    webkit

Presubmit checks took 1.1s to calculate.

[Han Shen, 2012-06-07 20:43:46]

On 2012/06/07 20:39:03, I haz the power (commit-bot) wrote:
> Presubmit check for 10537037-5009 failed and returned exit status 1.
> 
> Running presubmit commit checks ...
> 
> ** Presubmit Warnings **
> License must match:
> .*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
> of this source code is governed by a BSD-style license that can be\n.*? found
in
> the LICENSE file\.(?: \*/)?\n
> Found a bad license header in these files:
>   webkit/glue/webcursor_gtk_data.h
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for files in these directories:
>     content/browser/renderer_host
>     webkit
> 
> Presubmit checks took 1.1s to calculate.

Hi Brett, could you give comments/approval for "content" and "webkit", you are
the owner of these 2.

Thanks,
-Han

[brettw, 2012-06-07 21:19:20]

lgtm

[commit-bot: I haz the power, 2012-06-07 21:28:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/shenhan@google.com/10537037/5009

[commit-bot: I haz the power, 2012-06-07 21:28:06]

Presubmit check for 10537037-5009 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
  webkit/glue/webcursor_gtk_data.h

Presubmit checks took 1.3s to calculate.

[Han, 2012-06-07 21:36:46]

On 2012/06/07 21:28:06, I haz the power (commit-bot) wrote:
> Presubmit check for 10537037-5009 failed and returned exit status 1.
> 
> Running presubmit commit checks ...
> 
> ** Presubmit Warnings **
> License must match:
> .*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
> of this source code is governed by a BSD-style license that can be\n.*? found
in
> the LICENSE file\.(?: \*/)?\n
> Found a bad license header in these files:
>   webkit/glue/webcursor_gtk_data.h
> 
> Presubmit checks took 1.3s to calculate.

Hi Peter and Brett, "presubmit queue" complains about missing valid license
header, I copied one and added it to webcursor_gtk_data.h. Let me know if this
is not the right way to go. Thanks,

-Han

[brettw, 2012-06-07 21:39:54]

That's wrong. That file has a different license and you shouldn't put a
different one at the top.

Probably you just can't use commit queue for this patch.

[brettw, 2012-06-07 21:39:54]

That's wrong. That file has a different license and you shouldn't put a
different one at the top.

Probably you just can't use commit queue for this patch.

[Peter Kasting, 2012-06-07 21:41:27]

On 2012/06/07 21:39:54, brettw wrote:
> That's wrong. That file has a different license and you shouldn't put a
> different one at the top.

We did modify the code though; shouldn't it then have both our license and their
license?

I really don't know WTF, someone who's done more of this (maybe mal@?) would
know better.

[Han, 2012-06-07 21:48:19]

On 2012/06/07 21:41:27, Peter Kasting wrote:
> On 2012/06/07 21:39:54, brettw wrote:
> > That's wrong. That file has a different license and you shouldn't put a
> > different one at the top.
> 
> We did modify the code though; shouldn't it then have both our license and
their
> license?
> 
> I really don't know WTF, someone who's done more of this (maybe mal@?) would
> know better.

Hi Mark, what shall I do to this pre-submit check failure - "missing valid
license header" for "webcursor_gtk_data.h"? Add such a header or  submit without
CQ?

Thanks,
-Han

[mal, 2012-06-08 17:59:58]

Putting the Chromium copyright on it is the wrong thing to do.

I don't see any problem with the current license, but I also don't see a way to
make the presubmit check pass.

I think the only option is to bypass the CQ.

On 2012/06/07 21:48:19, Han wrote:
> Hi Mark, what shall I do to this pre-submit check failure - "missing valid
> license header" for "webcursor_gtk_data.h"? Add such a header or  submit
without
> CQ?
> 
> Thanks,
> -Han

[Han, 2012-06-08 18:21:51]

On 2012/06/08 17:59:58, mal wrote:
> Putting the Chromium copyright on it is the wrong thing to do.
> 
> I don't see any problem with the current license, but I also don't see a way
to
> make the presubmit check pass.
> 
> I think the only option is to bypass the CQ.
> 
> On 2012/06/07 21:48:19, Han wrote:
> > Hi Mark, what shall I do to this pre-submit check failure - "missing valid
> > license header" for "webcursor_gtk_data.h"? Add such a header or  submit
> without
> > CQ?
> > 
> > Thanks,
> > -Han

Hi I reverted the chromium license header. Anyone can help land this for me?
Thanks,

-Han

[Peter Kasting, 2012-06-08 18:42:04]

I'll land this for you.

[Peter Kasting, 2012-06-08 19:59:04]

On 2012/06/08 18:42:04, Peter Kasting wrote:
> I'll land this for you.

Now trying the patch on https://chromiumcodereview.appspot.com/10543080 ; will
submit once tryjobs pass.

Closing this CL in favor of that.