Turn NULL used as int to 0....

Turn NULL used as int to 0.
(Excluding chrome/browser/...)

BUG=none
TEST=base_unittests & app_unittests

[Jacob Mandelson, 2009-09-11 21:22:51]

(Split off from CL 171028)

Ran tests on Linux. Passed except for ConditionVariableTest.LargeFastTaskTest,
which also fails without patch.

[scherkus (not reviewing), 2009-09-11 21:37:06]

way more manageable to review!

just a tiny nit with the copyright year

http://codereview.chromium.org/195067/diff/1/10
File base/clipboard_linux.cc (right):

http://codereview.chromium.org/195067/diff/1/10#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
copyright year

[Jacob Mandelson, 2009-09-11 21:48:09]

> Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
> copyright year

Changed.

[scherkus (not reviewing), 2009-09-11 22:01:54]

sweet!  lgtm!

[Paweł Hajdan Jr., 2009-09-14 16:29:58]

I'll handle landing for this one. Sent to try servers.

[Paweł Hajdan Jr., 2009-09-14 16:32:24]

+darin

I see there are some changes in webkit/glue. Darin, could you take a look?

[jam, 2009-09-14 20:04:18]

http://codereview.chromium.org/195067/diff/3010/2018
File chrome/common/child_process_host.cc (right):

http://codereview.chromium.org/195067/diff/3010/2018#newcode209
Line 209: set_handle(0);
same as to my comment in webplugin_delegate_proxy.cc

http://codereview.chromium.org/195067/diff/3010/2019
File chrome/renderer/webplugin_delegate_proxy.cc (right):

http://codereview.chromium.org/195067/diff/3010/2019#newcode63
Line 63: notify_needed_(false), notify_data_(0),
This goes against the Google C++ style guide, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#0_and_NULL

http://codereview.chromium.org/195067/diff/3010/2019#newcode164
Line 164: window_(0),
I don't think we should do this, at the very least, not this way.

On Windows, this is an HWND, so it should be initialized to NULL since that's
the standard way we initialize HANDLE/HWND etc.  I realize on POSIX this is an
int, but switching which platform is correct doesn't seem to get us closer to
correctness.

If you want to fix this, then I think the file that defines PluginWindowHandle
should have a function/const that's per platform and returns the "default" value

[Jacob Mandelson, 2009-09-14 20:18:02]

> http://codereview.chromium.org/195067/diff/3010/2019#newcode63
> Line 63: notify_needed_(false), notify_data_(0),
> This goes against the Google C++ style guide, see
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#0_and_NULL

notify_data_ is an intptr_t.  This isn't a pointer, it's an integer large
enough to hold pointer values.


> http://codereview.chromium.org/195067/diff/3010/2019#newcode164
> Line 164: window_(0),
> I don't think we should do this, at the very least, not this way.
> 
> On Windows, this is an HWND, so it should be initialized to NULL since that's
> the standard way we initialize HANDLE/HWND etc.  I realize on POSIX this is an
> int, but switching which platform is correct doesn't seem to get us closer to
> correctness.
> 
> If you want to fix this, then I think the file that defines PluginWindowHandle
> should have a function/const that's per platform and returns the "default"
value


Hmm.  Let me see what I can come up with here.

[Jacob Mandelson, 2009-09-17 22:19:35]

> > http://codereview.chromium.org/195067/diff/3010/2019#newcode164
> > Line 164: window_(0),
> > I don't think we should do this, at the very least, not this way.
> > 
> > On Windows, this is an HWND, so it should be initialized to NULL since
that's
> > the standard way we initialize HANDLE/HWND etc.  I realize on POSIX this is
an
> > int, but switching which platform is correct doesn't seem to get us closer
to
> > correctness.
> > 
> > If you want to fix this, then I think the file that defines
PluginWindowHandle
> > should have a function/const that's per platform and returns the "default"
> value
> 
> 
> Hmm.  Let me see what I can come up with here.


I made a const gfx::kNullPluginWindow in native_widget_types.h which
is set from the type-appropirate zero and changed the usages to that.
(There're only the two, unless they're already using 0 instead of NULL, which my
compile check won't catch.)

[jam, 2009-09-17 22:23:10]

http://codereview.chromium.org/195067/diff/3010/2018
File chrome/common/child_process_host.cc (right):

http://codereview.chromium.org/195067/diff/3010/2018#newcode209
Line 209: set_handle(0);
On 2009/09/14 20:04:18, John Abd-El-Malek wrote:
> same as to my comment in webplugin_delegate_proxy.cc

the latest patch looks good to me but this point is still not addressed.

my objection is that it doesn't seem useful to go from windows correct/linux not
correct to windows not correct/linux correct.  You can add a const for null
ProcessHandle objects just like you did for the window handle.

[Jacob Mandelson, 2009-09-18 00:07:48]

> the latest patch looks good to me but this point is still not addressed.
> 
> my objection is that it doesn't seem useful to go from windows correct/linux
not
> correct to windows not correct/linux correct.  You can add a const for null
> ProcessHandle objects just like you did for the window handle.

Added constants for the null thread and process handles.

[jam, 2009-09-18 03:20:45]

On 2009/09/18 00:07:48, Jacob Mandelson wrote:
> > the latest patch looks good to me but this point is still not addressed.
> > 
> > my objection is that it doesn't seem useful to go from windows correct/linux
> not
> > correct to windows not correct/linux correct.  You can add a const for null
> > ProcessHandle objects just like you did for the window handle.
> 
> Added constants for the null thread and process handles.

thanks, lgtm

[Jacob Mandelson, 2009-09-22 20:17:15]

Is there anything else I should do for this patchset?

[Jacob Mandelson, 2009-09-28 21:35:48]

On 2009/09/22 20:17:15, Jacob Mandelson wrote:
> Is there anything else I should do for this patchset?

I'll need a committer to land this.

[Jacob Mandelson, 2009-10-08 19:28:59]

> I'll need a committer to land this.

Ping.

[Jacob Mandelson, 2009-10-11 22:52:33]

On 2009/10/08 19:28:59, Jacob Mandelson wrote:
> > I'll need a committer to land this.
> 
> Ping.

Pawel, can you do a try run and submit?

[Paweł Hajdan Jr., 2009-10-12 16:30:09]

Sure, I'm submitting it to the try servers now. Please ping me if I don't land
this in a day or so.

[Paweł Hajdan Jr., 2009-10-13 14:03:37]

Landed in http://codereview.chromium.org/267076 . Thanks for the patch.