hmm seems external people might be using this?

FWIW - the code is totally untested, and isn't even compiled on any of our bots.

That's a question for Ricardo.

right... the sandbox was supposed to be usable by non-chrome projects, and one of those "features" was the ability to work when the broker and sandboxed app do not share the same executable (which is a relatively strong restriction).

Unit test were working in both modes, but we never automated tests to build stuff with the alternate configuration so I'm pretty much sure something is broken at this point (but I'd also bet the fix is trivial, if we care to fix it).

The stand alone repo was abandoned long ago, and I'm sure every external user already has a complete fork of the code (with some process to cherry-pick CLs?), so dropping the feature is not bound to brake anything (but may make someone's work slightly harder).

Where were/are the unit tests? I see no setting of this #define in any build configuration.

There are no dedicated tests, but all tests should pass when using that define (no need for two different exes, sharing a single image also works with the define).

This mostly affects interception, and being able to pass things from one process to the other (as far as I remember), so interception unit tests and all integration/validation tests would exercise this.

I suppose what I'm saying is that these tests are not run on any of the bots, I don't think anyone has actually set this #define and run the tests in a long time (I certainly haven't), so we have no real assurance that anyone setting this #define is going to get a working sandbox.

Given this, I would prefer to remove the code, or add official tests. 'no dead code in chromium' is the axiom I am going by here.

oh, sure, I was just explaining the current status and reasons.

As I said:
- Probably nobody will complain if the support is removed.
- There are not explicit tests, but building a configuration that tests everything is trivial enough (ignoring the burden of another bot, or at least compiling another exe).
- Most likely something is broken, but by just following what the rest of the code does when writing code (aka, not removing SANDBOX_EXPORTS when copy/pasting) I expect most things to "just work".

So it should be easy to support, dunno if we want to.

a little birdie told me that mozilla might use SANDBOX_EXPORTS - jmathies do you know the story on this?

Yes Mozilla currently relies on SANDBOX_EXPORTS. Our plugin processes (NPAPI and media) rely on a separate exe. This is something we've moved away from with our newer child processes but unfortunately we have 3rd party dependencies that keep plugins in the old exe container.

So obviously for simplicity we'd like to see this code stay in the parent repo. This said we currently have a small list of changes we apply on top of merges and could add this to that list if need be.

Thanks for checking in with us!

You started fixing this bug over two years ago. Are you still working on it? 

In reply to comment 11, we are still using the SANDBOX_EXPORTS code at Mozilla for our NPAPI (flash) and media plugin processes.

Unless this code is difficult to maintain or standing in the way of other changes, we'd really appreciate it if it stayed in the chromium codebase at the moment.

We believe that changing the media process may not present too much of a challenge, so when flash reaches end of life, we can hopefully stop using SANDBOX_EXPORTS as well.