Simplify the macro for this deprecation. g_type_init was deprecated in glib version 2.36. The prev…

Simplify the macro for this deprecation. g_type_init was deprecated in glib version 2.36.  The previous macros were not working for my clang linux build on ubuntu 13.04.

BUG=none

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

[atreat, 2013-10-03 15:49:25]

My linux build was failing to compile with clang and these macros were the
problem.  Not sure why they weren't working with clang, but this patch
simplifies anyway.

[eseidel1, 2013-10-03 16:23:17]

Unfortunately I'm not an owner for this directory. Erg or eroman come to mind,
but you'll need to check the owners file.

[atreat, 2013-10-03 17:36:54]

Added eroman after checking the OWNERS file.  Thanks.

[akalin, 2013-10-04 17:24:48]

https://codereview.chromium.org/25385006/diff/1/net/tools/net_watcher/net_wat...
File net/tools/net_watcher/net_watcher.cc (right):

https://codereview.chromium.org/25385006/diff/1/net/tools/net_watcher/net_wat...
net/tools/net_watcher/net_watcher.cc:139: #if (GLIB_MAJOR_VERSION <= 2 &&
GLIB_MINOR_VERSION < 36)
Shouldn't this be something like:

(GLIB_MAJOR_VERSION < 2) ||
(GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 36)

?

However, it seems better to me to mirror the code in browser_main_loop.cc, which
looks like:

#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
  // g_type_init will be deprecated in 2.36. 2.35 is the development
  // version for 2.36, hence do not call g_type_init starting 2.35.
  //
http://developer.gnome.org/gobject/unstable/gobject-Type-Information.html#g-t...
#if !GLIB_CHECK_VERSION(2, 35, 0)
  // Glib type system initialization. Needed at least for gconf,
  // used in net/proxy/proxy_config_service_linux.cc. Most likely
  // this is superfluous as gtk_init() ought to do this. It's
  // definitely harmless, so retained as a reminder of this
  // requirement for gconf.
  g_type_init();
#endif

[atreat, 2013-10-04 21:06:32]

I've edited the change so that it conforms to the similar code in
browser_main_loop

[akalin, 2013-10-05 06:24:30]

https://codereview.chromium.org/25385006/diff/7001/net/tools/net_watcher/net_...
File net/tools/net_watcher/net_watcher.cc (left):

https://codereview.chromium.org/25385006/diff/7001/net/tools/net_watcher/net_...
net/tools/net_watcher/net_watcher.cc:135: // Needed so ProxyConfigServiceLinux
can use gconf.
can you keep the first two lines?

https://codereview.chromium.org/25385006/diff/7001/net/tools/net_watcher/net_...
File net/tools/net_watcher/net_watcher.cc (right):

https://codereview.chromium.org/25385006/diff/7001/net/tools/net_watcher/net_...
net/tools/net_watcher/net_watcher.cc:134: #if (defined(OS_LINUX) ||
defined(OS_OPENBSD)) && !defined(OS_CHROMEOS)
can you also change this if def to match BrowserMainLoop?

[atreat, 2013-10-07 14:38:59]

Fixed to match the code in browser_main_loop more closely.

[akalin, 2013-10-07 22:27:09]

lgtm

[commit-bot: I haz the power, 2013-10-07 22:29:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/25385006/12001

[commit-bot: I haz the power, 2013-10-07 23:11:51]

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.

[akalin, 2013-10-07 23:24:54]

https://codereview.chromium.org/25385006/diff/12001/net/tools/net_watcher/net...
File net/tools/net_watcher/net_watcher.cc (right):

https://codereview.chromium.org/25385006/diff/12001/net/tools/net_watcher/net...
net/tools/net_watcher/net_watcher.cc:134: #if defined(OS_POSIX) &&
!defined(OS_MACOSX) && !defined(OS_ANDROID)
looks like you change the guards for the glib-object.h include also

[atreat, 2013-10-08 01:20:27]

On 2013/10/07 23:24:54, akalin wrote:
>
https://codereview.chromium.org/25385006/diff/12001/net/tools/net_watcher/net...
> File net/tools/net_watcher/net_watcher.cc (right):
> 
>
https://codereview.chromium.org/25385006/diff/12001/net/tools/net_watcher/net...
> net/tools/net_watcher/net_watcher.cc:134: #if defined(OS_POSIX) &&
> !defined(OS_MACOSX) && !defined(OS_ANDROID)
> looks like you change the guards for the glib-object.h include also

So it sounds like the proper thing to do is to go back to the linux+openbsd
without chromos guards.  Otherwise this is a behavior change...

    #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)

does not equal

    #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)

Agree?

[akalin, 2013-10-08 01:24:04]

On 2013/10/08 01:20:27, atreat wrote:
> On 2013/10/07 23:24:54, akalin wrote:
> >
>
https://codereview.chromium.org/25385006/diff/12001/net/tools/net_watcher/net...
> > File net/tools/net_watcher/net_watcher.cc (right):
> > 
> >
>
https://codereview.chromium.org/25385006/diff/12001/net/tools/net_watcher/net...
> > net/tools/net_watcher/net_watcher.cc:134: #if defined(OS_POSIX) &&
> > !defined(OS_MACOSX) && !defined(OS_ANDROID)
> > looks like you change the guards for the glib-object.h include also
> 
> So it sounds like the proper thing to do is to go back to the linux+openbsd
> without chromos guards.  Otherwise this is a behavior change...
> 
>     #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
> 
> does not equal
> 
>     #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
> 
> Agree?

I don't understand your message. Previously, the #if for the glib-object.h were
both "(linux || bsd) && !chromeos". I asked you to change it to "posix && !os x
&& !android" to match browser_main_loop.cc, but you didn't update the one for
the include.

Yes, it's a behavior change. I think that's fine.

[atreat, 2013-10-08 02:54:34]

On 2013/10/08 01:24:04, akalin wrote:
> I don't understand your message. Previously, the #if for the glib-object.h
were
> both "(linux || bsd) && !chromeos". I asked you to change it to "posix && !os
x
> && !android" to match browser_main_loop.cc, but you didn't update the one for
> the include.
> 
> Yes, it's a behavior change. I think that's fine.

Ok.  You know this code.  I generally don't like to do behavior changes that
don't fit the original issue, but I'll defer to you.  Patch updated.

[akalin, 2013-10-08 20:24:08]

yeah, this is just a tool executable, so it's not a big deal. still lgtm.

[commit-bot: I haz the power, 2013-10-08 20:25:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/25385006/15001

[commit-bot: I haz the power, 2013-10-08 21:06:39]

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.

[Lei Zhang, 2013-10-08 21:40:55]

+yael FYI

r168690 is titled "Avoid calling into glib on ChromeOS" but doesn't say why.
Does CrOS not use glib? I forget...

[pauljensen, 2013-10-08 21:47:52]

Linux uses ProxyConfigServiceLinux which uses glib.
CrOS uses chromeos::ProxyConfigServiceImpl which does not use glib.
r168690 allowed net_watcher to build on CrOS again.

[akalin, 2013-10-08 21:59:05]

On 2013/10/08 21:47:52, pauljensen wrote:
> Linux uses ProxyConfigServiceLinux which uses glib.
> CrOS uses chromeos::ProxyConfigServiceImpl which does not use glib.
> r168690 allowed net_watcher to build on CrOS again.

I see. I guess that part should be put back in.

How does BrowserMainLoop compile on ChromeOS if it doesn't ifdef that part out?

[akalin, 2013-10-08 21:59:57]

On 2013/10/08 21:59:05, akalin wrote:
> On 2013/10/08 21:47:52, pauljensen wrote:
> > Linux uses ProxyConfigServiceLinux which uses glib.
> > CrOS uses chromeos::ProxyConfigServiceImpl which does not use glib.
> > r168690 allowed net_watcher to build on CrOS again.
> 
> I see. I guess that part should be put back in.
> 
> How does BrowserMainLoop compile on ChromeOS if it doesn't ifdef that part
out?

atreat@, mind putting in an !defined(OS_CHROMEOS) clause and adding a comment as
to why? Sorry this is taking so long!

[pauljensen, 2013-10-08 22:00:13]

To more directly answer your question, I don't think CrOS uses glib.  glib is
from GTK+ which Gnome is based on, and I don't think CrOS uses Gnome.
You'll probably need to leave my change in your CL for it to build on CrOS.

[commit-bot: I haz the power, 2013-10-09 14:22:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/25385006/69001

[commit-bot: I haz the power, 2013-10-09 14:32:19]

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

[commit-bot: I haz the power, 2013-10-09 15:02:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/25385006/69001

[commit-bot: I haz the power, 2013-10-09 16:11:20]

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

[commit-bot: I haz the power, 2013-10-09 16:22:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/25385006/69001

[commit-bot: I haz the power, 2013-10-09 16:29:22]

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

[commit-bot: I haz the power, 2013-10-09 16:46:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/25385006/69001

[commit-bot: I haz the power, 2013-10-09 17:23:18]

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

[commit-bot: I haz the power, 2013-10-09 18:40:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/25385006/69001

[commit-bot: I haz the power, 2013-10-09 19:54:20]

Change committed as 227782