Add more support for FreeBSD

Add more support for FreeBSD

BUG=

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

[Paweł Hajdan Jr., 2013-05-29 21:19:26]

Thanks for the patch!

https://codereview.chromium.org/16206002/diff/1/content/browser/browser_main_...
File content/browser/browser_main_loop.cc (right):

https://codereview.chromium.org/16206002/diff/1/content/browser/browser_main_...
content/browser/browser_main_loop.cc:112: !defined(OS_BSD)
Could you explain why? I'd rather get sandbox to work under BSD than disable it.

[George Liaskos, 2013-05-29 21:57:46]

On 2013/05/29 21:19:26, Paweł Hajdan Jr. wrote:
> Thanks for the patch!
> 
>
https://codereview.chromium.org/16206002/diff/1/content/browser/browser_main_...
> File content/browser/browser_main_loop.cc (right):
> 
>
https://codereview.chromium.org/16206002/diff/1/content/browser/browser_main_...
> content/browser/browser_main_loop.cc:112: !defined(OS_BSD)
> Could you explain why? I'd rather get sandbox to work under BSD than disable
it.

You are right, I've updated the patch.

[Paweł Hajdan Jr., 2013-05-29 22:16:25]

LGTM - you'll need some more OWNERS approvals, let's see...

[commit-bot: I haz the power, 2013-05-29 22:16:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Geo.Liaskos@gmail.com/16206002/5001

[commit-bot: I haz the power, 2013-05-29 22:26:58]

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

[Paweł Hajdan Jr., 2013-05-30 17:29:51]

Please review (OWNERS):

Darin: webkit, third_party, chrome, content, base
Nico: ui, skia

Just OS_BSD #ifdefs changes.

[Nico, 2013-05-30 17:37:35]

all the os_openbsd -> os_bsd changes are fine. i'm kinda not feeling the
os_linux -> "os_linux || os_bsd" changes, that seems like it'll just grow and
grow over time. maybe "os_posix && !os_mac" is a bit more scalable? i think this
was discussed a while on chromium_dev but i forgot the conclusion.

https://codereview.chromium.org/16206002/diff/5001/ui/base/l10n/l10n_util.cc
File ui/base/l10n/l10n_util.cc (right):

https://codereview.chromium.org/16206002/diff/5001/ui/base/l10n/l10n_util.cc#...
ui/base/l10n/l10n_util.cc:457: #elif defined(OS_CHROMEOS) || (defined(USE_AURA)
&& !defined(OS_LINUX))
And once we have aura-on-linux (and i suppose bsd), we'll have to change this
too?

[George Liaskos, 2013-05-31 18:32:03]

Please let me know if i should change something to get this committed.

[Paweł Hajdan Jr., 2013-06-19 23:13:35]

On 2013/05/31 18:32:03, George Liaskos wrote:
> Please let me know if i should change something to get this committed.

George, please respond to Nico's comment.

Also see following discussions for reference (note that we now use the new
chromium-dev in chromium.org domain):

https://groups.google.com/d/msg/chromium-dev/bylfVv51om0/CZgA6H0KijAJ
https://groups.google.com/d/msg/chromium-dev/EvklzY9Q06o/iVF9m1oG2gUJ

[George Liaskos, 2013-06-23 11:23:58]

Sorry for the late response

On 2013/05/30 17:37:35, Nico wrote:
> all the os_openbsd -> os_bsd changes are fine. i'm kinda not feeling the
> os_linux -> "os_linux || os_bsd" changes, that seems like it'll just grow and
> grow over time. maybe "os_posix && !os_mac" is a bit more scalable? i think
this
> was discussed a while on chromium_dev but i forgot the conclusion.
> 
> https://codereview.chromium.org/16206002/diff/5001/ui/base/l10n/l10n_util.cc
> File ui/base/l10n/l10n_util.cc (right):

"os_posix && !os_macosx" feels better indeed, I will use it for the new
additions in this file and in the upcoming patches.
I could send a "cleanup" patch later for the existing cases.

>
https://codereview.chromium.org/16206002/diff/5001/ui/base/l10n/l10n_util.cc#...
> ui/base/l10n/l10n_util.cc:457: #elif defined(OS_CHROMEOS) ||
(defined(USE_AURA)
> && !defined(OS_LINUX))
> And once we have aura-on-linux (and i suppose bsd), we'll have to change this
> too?

I am not sure of how this line will change then, "os_chromeos || use_aura"?
If this is the case then the BSDs should be ok too, please correct me if I am
wrong.

[Paweł Hajdan Jr., 2013-07-31 20:59:48]

On 2013/06/23 11:23:58, George Liaskos wrote:
> Sorry for the late response
> 
> On 2013/05/30 17:37:35, Nico wrote:
> > all the os_openbsd -> os_bsd changes are fine. i'm kinda not feeling the
> > os_linux -> "os_linux || os_bsd" changes, that seems like it'll just grow
and
> > grow over time. maybe "os_posix && !os_mac" is a bit more scalable? i think
> this
> > was discussed a while on chromium_dev but i forgot the conclusion.
> > 
> > https://codereview.chromium.org/16206002/diff/5001/ui/base/l10n/l10n_util.cc
> > File ui/base/l10n/l10n_util.cc (right):
> 
> "os_posix && !os_macosx" feels better indeed, I will use it for the new
> additions in this file and in the upcoming patches.
> I could send a "cleanup" patch later for the existing cases.

George, could you update this patch taking the above into account?

>
https://codereview.chromium.org/16206002/diff/5001/ui/base/l10n/l10n_util.cc#...
> > ui/base/l10n/l10n_util.cc:457: #elif defined(OS_CHROMEOS) ||
> (defined(USE_AURA)
> > && !defined(OS_LINUX))
> > And once we have aura-on-linux (and i suppose bsd), we'll have to change
this
> > too?
> 
> I am not sure of how this line will change then, "os_chromeos || use_aura"?
> If this is the case then the BSDs should be ok too, please correct me if I am
> wrong.

Nico, I think this is fine as-is. It can be updated later as needed IMHO.
Alternatively, please just comment on this and recommend an acceptable solution.

[George Liaskos, 2013-08-01 20:55:19]

On 2013/07/31 20:59:48, Paweł Hajdan Jr. wrote:
> On 2013/06/23 11:23:58, George Liaskos wrote:
> > Sorry for the late response
> > 
> > On 2013/05/30 17:37:35, Nico wrote:
> > > all the os_openbsd -> os_bsd changes are fine. i'm kinda not feeling the
> > > os_linux -> "os_linux || os_bsd" changes, that seems like it'll just grow
> and
> > > grow over time. maybe "os_posix && !os_mac" is a bit more scalable? i
think
> > this
> > > was discussed a while on chromium_dev but i forgot the conclusion.
> > > 
> > >
https://codereview.chromium.org/16206002/diff/5001/ui/base/l10n/l10n_util.cc
> > > File ui/base/l10n/l10n_util.cc (right):
> > 
> > "os_posix && !os_macosx" feels better indeed, I will use it for the new
> > additions in this file and in the upcoming patches.
> > I could send a "cleanup" patch later for the existing cases.
> 
> George, could you update this patch taking the above into account?

Done

[Paweł Hajdan Jr., 2013-08-02 18:21:39]

On 2013/08/01 20:55:19, George Liaskos wrote:
> > George, could you update this patch taking the above into account?
> 
> Done

Thanks. Could you do the same update in other files touched by this patch?

I think it's really important to be consistent about this.

[George Liaskos, 2013-08-08 21:28:24]

On 2013/08/02 18:21:39, Paweł Hajdan Jr. wrote:
> Thanks. Could you do the same update in other files touched by this patch?
> 
> I think it's really important to be consistent about this.

Thank you for hanging in there!

Posix also includes android and nacl, so I exclude those platforms too.
Please let me know what do you think, it would be nice if we could use a bot.

Regards

[Paweł Hajdan Jr., 2013-08-26 22:01:39]

https://codereview.chromium.org/16206002/diff/38001/chrome/browser/chrome_con...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/16206002/diff/38001/chrome/browser/chrome_con...
chrome/browser/chrome_content_browser_client.cc:169: #if defined(OS_POSIX) &&
!defined(OS_MACOSX) && !defined(OS_NACL)
nit: Are the OS_NACL guards needed?

[George Liaskos, 2013-08-27 22:16:30]

On 2013/08/26 22:01:39, Paweł Hajdan Jr. wrote:
>
https://codereview.chromium.org/16206002/diff/38001/chrome/browser/chrome_con...
> File chrome/browser/chrome_content_browser_client.cc (right):
> 
>
https://codereview.chromium.org/16206002/diff/38001/chrome/browser/chrome_con...
> chrome/browser/chrome_content_browser_client.cc:169: #if defined(OS_POSIX) &&
> !defined(OS_MACOSX) && !defined(OS_NACL)
> nit: Are the OS_NACL guards needed?

Thank you for explaining, I've removed the os_nacl checks.

[Paweł Hajdan Jr., 2013-08-28 18:03:44]

LGTM

[Nico, 2013-08-28 18:08:45]

lgtm

[commit-bot: I haz the power, 2013-08-28 19:43:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Geo.Liaskos@gmail.com/16206002/46001

[commit-bot: I haz the power, 2013-08-28 21:19:22]

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

[Avi (use Gerrit), 2013-09-13 18:25:36]

LGTM

[jsbell, 2013-09-13 18:43:35]

not lgtm - Note the comment just below the change in
third_party/leveldatabase/env_chromium.cc - this needs to at least exclude
OS_LINUX

(were there try jobs?)

[George Liaskos, 2013-09-13 19:11:32]

On 2013/09/13 18:43:35, jsbell wrote:
> not lgtm - Note the comment just below the change in
> third_party/leveldatabase/env_chromium.cc - this needs to at least exclude
> OS_LINUX
> 
> (were there try jobs?)

Good catch!

[jsbell, 2013-09-19 21:20:31]

https://codereview.chromium.org/16206002/diff/64001/third_party/leveldatabase...
File third_party/leveldatabase/env_chromium.cc (right):

https://codereview.chromium.org/16206002/diff/64001/third_party/leveldatabase...
third_party/leveldatabase/env_chromium.cc:51: #if (defined(OS_POSIX) &&
!defined(OS_LINUX)) || defined(OS_WIN)
I believe that OS_POSIX is basically defined as !OS_WIN. In that case, this
could be simplified to:

#if !defined(OS_LINUX)

... but the intent of the CL is more clear this way. This file lgtm

[commit-bot: I haz the power, 2013-09-19 21:27:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Geo.Liaskos@gmail.com/16206002/64001

[commit-bot: I haz the power, 2013-09-20 02:19:59]

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

[commit-bot: I haz the power, 2013-09-20 19:30:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Geo.Liaskos@gmail.com/16206002/64001

[commit-bot: I haz the power, 2013-09-21 00:18:11]

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

[commit-bot: I haz the power, 2013-09-21 01:44:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Geo.Liaskos@gmail.com/16206002/64001

[commit-bot: I haz the power, 2013-09-21 03:47:47]

Change committed as 224546