Make building seccomp-bpf a GYP condition

Make building seccomp-bpf a GYP condition

We add a use_seccomp_bpf variable to allow not building
seccomp-bpf.

We make sure that it can't be use by mistake with sanity
checks that the architecture is MIPS.

BUG=267179
TEST=Use "GYP_DEFINES='use_seccomp_bpf=0' and comment out the two #error.
about:sandbox should say that seccomp-bpf is disabled.

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

[jln (very slow on Chromium), 2013-12-18 01:49:11]

Mark: could you please take a look ?

If you think of a better strategy, please, let me know!

[Mark Seaborn, 2013-12-18 08:10:21]

Can you add a TEST= line to say how you tested the change?  I assumed you tried
compiling with use_seccomp_bpf=0 and the #errors disabled.

LGTM

https://chromiumcodereview.appspot.com/117803002/diff/40001/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/build/common.gypi...
build/common.gypi:1919: # seccomp-bpf is only supportd on three architectures
currently.
Typo: "supported".  Maybe say "seccomp-bpf sandbox" to provide a little more
context.

https://chromiumcodereview.appspot.com/117803002/diff/40001/build/common.gypi...
build/common.gypi:1928: }]
Nit: add trailing comma

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl.gyp
File components/nacl.gyp (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl.g...
components/nacl.gyp:225: # Included in both cases
So why not leave it in the unconditional sources list above?  (With a comment if
you want one.)

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl/l...
File components/nacl/loader/nacl_sandbox_linux.cc (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl/l...
components/nacl/loader/nacl_sandbox_linux.cc:160: #endif  //
!!defined(ARCH_CPU_MIPS_FAMILY)
Nit: Just "!defined" (but a closing comment is rather superfluous when the
opening #if is two lines away!)

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl/l...
components/nacl/loader/nacl_sandbox_linux.cc:173: return false;
Hmm, as an aside (which does not affect this change), InitializeBPFSandbox()
returning false isn't treated as a fatal error by BecomeNaClLoader().  That
seems risky.  If the seccomp sandbox broke, would the NaCl tests detect it?

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/common/sa...
File content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/common/sa...
content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc:22: #if
defined(USE_SECCOMP_BPF)
Add empty line after to match closing #endif?

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/common/sa...
content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc:39: #else
Add empty line before

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/content_c...
File content/content_common.gypi (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/content_c...
content/content_common.gypi:644: # These are included in both cases.
Same comment as other file...

[jln (very slow on Chromium), 2013-12-19 00:00:19]

Thanks Mark!

https://chromiumcodereview.appspot.com/117803002/diff/40001/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/build/common.gypi...
build/common.gypi:1919: # seccomp-bpf is only supportd on three architectures
currently.
On 2013/12/18 08:10:21, Mark Seaborn wrote:
> Typo: "supported".  Maybe say "seccomp-bpf sandbox" to provide a little more
> context.

Done.

https://chromiumcodereview.appspot.com/117803002/diff/40001/build/common.gypi...
build/common.gypi:1928: }]
On 2013/12/18 08:10:21, Mark Seaborn wrote:
> Nit: add trailing comma

Done.

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl.gyp
File components/nacl.gyp (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl.g...
components/nacl.gyp:225: # Included in both cases
On 2013/12/18 08:10:21, Mark Seaborn wrote:
> So why not leave it in the unconditional sources list above?  (With a comment
if
> you want one.)

Yeah, I think I misunderstood how "defines" worked.

Done, thanks.

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl/l...
File components/nacl/loader/nacl_sandbox_linux.cc (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl/l...
components/nacl/loader/nacl_sandbox_linux.cc:160: #endif  //
!!defined(ARCH_CPU_MIPS_FAMILY)
On 2013/12/18 08:10:21, Mark Seaborn wrote:
> Nit: Just "!defined" (but a closing comment is rather superfluous when the
> opening #if is two lines away!)

Done.

https://chromiumcodereview.appspot.com/117803002/diff/40001/components/nacl/l...
components/nacl/loader/nacl_sandbox_linux.cc:173: return false;
On 2013/12/18 08:10:21, Mark Seaborn wrote:
> Hmm, as an aside (which does not affect this change), InitializeBPFSandbox()
> returning false isn't treated as a fatal error by BecomeNaClLoader().  That
> seems risky.  If the seccomp sandbox broke, would the NaCl tests detect it?

No, and it was a clear TODO (crbug.com/270914), but I didn't get to it yet.

I need to figure out how to wire reporting the about:sandbox status  to NaCl.

A CHECK() on CHROMEOS might "just work" though.

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/common/sa...
File content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/common/sa...
content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc:22: #if
defined(USE_SECCOMP_BPF)
On 2013/12/18 08:10:21, Mark Seaborn wrote:
> Add empty line after to match closing #endif?

Done.

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/common/sa...
content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc:39: #else
On 2013/12/18 08:10:21, Mark Seaborn wrote:
> Add empty line before

Done.

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/content_c...
File content/content_common.gypi (right):

https://chromiumcodereview.appspot.com/117803002/diff/40001/content/content_c...
content/content_common.gypi:644: # These are included in both cases.
On 2013/12/18 08:10:21, Mark Seaborn wrote:
> Same comment as other file...

Done.

[jln (very slow on Chromium), 2013-12-19 00:02:37]

Charlie: do you mind taking a look at content/content_common.gypi for owner
approval?

[awong, 2013-12-19 00:07:47]

https://chromiumcodereview.appspot.com/117803002/diff/60001/content/content_c...
File content/content_common.gypi (right):

https://chromiumcodereview.appspot.com/117803002/diff/60001/content/content_c...
content/content_common.gypi:630:
'common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc',
Aren't these things more commonly defined as sources! when the flag is false so
the files still show up in an IDE?

[Charlie Reis, 2013-12-19 00:08:03]

Rubber stamp LGTM for content/content_common.gypi.

[Charlie Reis, 2013-12-19 00:08:52]

On 2013/12/19 00:07:47, awong wrote:
>
https://chromiumcodereview.appspot.com/117803002/diff/60001/content/content_c...
> File content/content_common.gypi (right):
> 
>
https://chromiumcodereview.appspot.com/117803002/diff/60001/content/content_c...
> content/content_common.gypi:630:
> 'common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc',
> Aren't these things more commonly defined as sources! when the flag is false
so
> the files still show up in an IDE?

I defer to Albert's approval here.  :)

[jln (very slow on Chromium), 2013-12-19 01:02:12]

I'm a bit worried than listing the files twice will result in the lists not
being in sync, but done!

https://chromiumcodereview.appspot.com/117803002/diff/60001/content/content_c...
File content/content_common.gypi (right):

https://chromiumcodereview.appspot.com/117803002/diff/60001/content/content_c...
content/content_common.gypi:630:
'common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc',
On 2013/12/19 00:07:47, awong wrote:
> Aren't these things more commonly defined as sources! when the flag is false
so
> the files still show up in an IDE?

Done.

[commit-bot: I haz the power, 2013-12-19 19:05:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jln@chromium.org/117803002/80001

[commit-bot: I haz the power, 2013-12-19 21:38:40]

Change committed as 241949