Test for browser sandbox (Issue 4530)

Test for browser sandbox (Issue 4530)

[seano, 2010-07-06 10:47:09]

Couple of comments.

Also could you add a TODO to move this code to a site_ui test and use the actual
login process, once that code is stabilized? Thanks.

http://codereview.chromium.org/2822047/diff/1/2
File client/site_tests/security_RendererSandbox/control (right):

http://codereview.chromium.org/2822047/diff/1/2#newcode3
client/site_tests/security_RendererSandbox/control:3: PURPOSE = "Basic check to
ensure renderer is sandboxed/jailed in Chrome."
Chromium oS

http://codereview.chromium.org/2822047/diff/1/2#newcode11
client/site_tests/security_RendererSandbox/control:11: This test simply checks
if a page is loaded within the sandboxed/jailed environment
Fix this to wrap cleanly at 80chars

http://codereview.chromium.org/2822047/diff/1/2#newcode12
client/site_tests/security_RendererSandbox/control:12: inside Chrome OS.
Chromium OS

http://codereview.chromium.org/2822047/diff/1/3
File client/site_tests/security_RendererSandbox/security_RendererSandbox.py
(right):

http://codereview.chromium.org/2822047/diff/1/3#newcode7
client/site_tests/security_RendererSandbox/security_RendererSandbox.py:7: from
autotest_lib.client.bin import utils, test
Alphebatize the module imports

http://codereview.chromium.org/2822047/diff/1/3#newcode19
client/site_tests/security_RendererSandbox/security_RendererSandbox.py:19:
time.sleep(time_to_wait)
Can you use site_utils.poll_for_condition instead? We try to avoid hardcoded
sleeps.

http://codereview.chromium.org/2822047/diff/1/3#newcode22
client/site_tests/security_RendererSandbox/security_RendererSandbox.py:22:
render_pid = utils.system_output('pgrep -f %s'
Consider choosing the renderer PID to use (Index 0 of the list resulting from
split() ?) here, so the rest of the script is cleaner.

[radi, 2010-07-06 15:03:49]

Addressed comments. Please verify fixes.

On 2010/07/06 10:47:09, seano wrote:
> Couple of comments.
> 
> Also could you add a TODO to move this code to a site_ui test and use the
actual
> login process, once that code is stabilized? Thanks.
> 
> http://codereview.chromium.org/2822047/diff/1/2
> File client/site_tests/security_RendererSandbox/control (right):
> 
> http://codereview.chromium.org/2822047/diff/1/2#newcode3
> client/site_tests/security_RendererSandbox/control:3: PURPOSE = "Basic check
to
> ensure renderer is sandboxed/jailed in Chrome."
> Chromium oS
> 
> http://codereview.chromium.org/2822047/diff/1/2#newcode11
> client/site_tests/security_RendererSandbox/control:11: This test simply checks
> if a page is loaded within the sandboxed/jailed environment
> Fix this to wrap cleanly at 80chars
> 
> http://codereview.chromium.org/2822047/diff/1/2#newcode12
> client/site_tests/security_RendererSandbox/control:12: inside Chrome OS.
> Chromium OS
> 
> http://codereview.chromium.org/2822047/diff/1/3
> File client/site_tests/security_RendererSandbox/security_RendererSandbox.py
> (right):
> 
> http://codereview.chromium.org/2822047/diff/1/3#newcode7
> client/site_tests/security_RendererSandbox/security_RendererSandbox.py:7: from
> autotest_lib.client.bin import utils, test
> Alphebatize the module imports
> 
> http://codereview.chromium.org/2822047/diff/1/3#newcode19
> client/site_tests/security_RendererSandbox/security_RendererSandbox.py:19:
> time.sleep(time_to_wait)
> Can you use site_utils.poll_for_condition instead? We try to avoid hardcoded
> sleeps.
> 
> http://codereview.chromium.org/2822047/diff/1/3#newcode22
> client/site_tests/security_RendererSandbox/security_RendererSandbox.py:22:
> render_pid = utils.system_output('pgrep -f %s'
> Consider choosing the renderer PID to use (Index 0 of the list resulting from
> split() ?) here, so the rest of the script is cleaner.

[seano, 2010-07-06 15:15:03]

LGTM

Small stylistic nits.

http://codereview.chromium.org/2822047/diff/1/2
File client/site_tests/security_RendererSandbox/control (right):

http://codereview.chromium.org/2822047/diff/1/2#newcode3
client/site_tests/security_RendererSandbox/control:3: PURPOSE = "Basic check to
ensure renderer is sandboxed/jailed in Chrome."
On 2010/07/06 10:47:09, seano wrote:
> Chromium oS

Done.

http://codereview.chromium.org/2822047/diff/5001/6001#newcode4
client/site_tests/security_RendererSandbox/control:4: CRITERIA = "This test will
fail if (a) no renderer is found or (b) the requested URL is not
sandboxed/jailed."
Remove extra LF

http://codereview.chromium.org/2822047/diff/5001/6002
File client/site_tests/security_RendererSandbox/security_RendererSandbox.py
(right):

http://codereview.chromium.org/2822047/diff/5001/6002#newcode42
client/site_tests/security_RendererSandbox/security_RendererSandbox.py:42: 
remove extra LF

[radi, 2010-07-06 15:34:43]

http://codereview.chromium.org/2822047/diff/1/2
File client/site_tests/security_RendererSandbox/control (right):

http://codereview.chromium.org/2822047/diff/1/2#newcode11
client/site_tests/security_RendererSandbox/control:11: This test simply checks
if a page is loaded within the sandboxed/jailed environment
On 2010/07/06 10:47:09, seano wrote:
> Fix this to wrap cleanly at 80chars

Done.

http://codereview.chromium.org/2822047/diff/1/2#newcode12
client/site_tests/security_RendererSandbox/control:12: inside Chrome OS.
On 2010/07/06 10:47:09, seano wrote:
> Chromium OS

Done.

http://codereview.chromium.org/2822047/diff/5001/6002
File client/site_tests/security_RendererSandbox/security_RendererSandbox.py
(right):

http://codereview.chromium.org/2822047/diff/5001/6002#newcode42
client/site_tests/security_RendererSandbox/security_RendererSandbox.py:42: 
On 2010/07/06 15:15:03, seano wrote:
> remove extra LF

Done.

[Chris Masone, 2010-07-09 00:12:51]

On 2010/07/06 15:34:45, radi wrote:
> 

Is there any reason this hasn't landed?

[seano, 2010-07-09 06:11:32]

I thought Radi pushed this the other day when the tree opened again... Radi?

On Fri, Jul 9, 2010 at 2:12 AM,  <cmasone@chromium.org> wrote:
> On 2010/07/06 15:34:45, radi wrote:
>
>
> Is there any reason this hasn't landed?
>
> http://codereview.chromium.org/2822047/show
>

[seano, 2010-07-09 08:56:47]

He's pushed it, and we're going to add it to a new
"suite_SecurityFunctionalSuite" and have suite_BuildVerify include
that.

-S

On Fri, Jul 9, 2010 at 8:11 AM, Sean O <seano@chromium.org> wrote:
> I thought Radi pushed this the other day when the tree opened again... Radi?
>
> On Fri, Jul 9, 2010 at 2:12 AM,  <cmasone@chromium.org> wrote:
>> On 2010/07/06 15:34:45, radi wrote:
>>
>>
>> Is there any reason this hasn't landed?
>>
>> http://codereview.chromium.org/2822047/show
>>
>