Do not use WebSandboxSupport when sandbox is disabled.

Do not use WebSandboxSupport when sandbox is disabled.


BUG=123977
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158557

[rickyz (Google), 2012-09-23 02:54:56]

Hi, this is my first Chromium patch (trying to fix bug 123977).  Can I get a
review?

[abarth-chromium, 2012-09-23 03:44:15]

I'm not the right person to review this change.  Perhaps look for someone in the
OWNERS file?

[rickyz (Google), 2012-09-23 03:57:56]

On 2012/09/23 03:44:15, abarth wrote:
> I'm not the right person to review this change.  Perhaps look for someone in
the
> OWNERS file?
My mistake, trying jamesr from OWNERS instead - thanks!

[jamesr, 2012-09-23 04:15:36]

Could you explain how this resolves the issue?  I don't see any discussion in
the bug or the description of why this is needed now.

[rickyz (Google), 2012-09-23 04:44:01]

On 2012/09/23 04:15:36, jamesr wrote:
> Could you explain how this resolves the issue?  I don't see any discussion in
> the bug or the description of why this is needed now.
Sure.  I'm not very familiar with this code, so it's possible that this fixes
the issue in a superficial or wrong way, but here's the reasoning behind it:

As somebody else mentioned in the bug, the backtrace showed that calls to
GetRenderStyleForStrike and GetFontFamilyForCharacters were trying to talk to a
nonexistent zygote process, since the browser is being started in single process
mode.

The call to GetRenderStyleForStrike originates comes from 
third_party/WebKit/Source/WebCore/platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.cpp:311,
and the call to GetFontFamilyForCharacters from
third_party/WebKit/Source/WebKit/chromium/src/PlatformSupport.cpp:247.  In both
cases, it uses the sandbox call if WebKit::Platform::current()->sandboxSupport()
is non-NULL.  Otherwise, it makes the same call that the render process would
have made in-process.

The patch makes it so that sandboxSupport() will return NULL if the
sandboxEnabled returns false (which is the case exactly when chrome is run in
single-process mode).

While it sounds like it would have made more sense to check sandboxEnabled in
WebKit code, I didn't do this, because it looks like the sandboxEnabled function
was removed from WebKit code in https://bugs.webkit.org/show_bug.cgi?id=83113.

[jamesr, 2012-09-25 04:36:00]

OK, that makes sense.  Could you put this discussion into the crbug for
posterity?

This means we won't exercise sandboxSupport at all in --single-process, but that
seems OK to me.

[jamesr, 2012-09-25 04:36:04]

lgtm

[commit-bot: I haz the power, 2012-09-25 04:50:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rickyz@google.com/10970057/1

[commit-bot: I haz the power, 2012-09-25 13:06:32]

Change committed as 158557