645531 - Content process xpcshell dies if a JS error occurs

Status: RESOLVED WONTFIX

(Core :: IPC, defect)

Description

[Josh Matthews [:jdm]]

From XPCShellEnvironment::EvaluateString:

>  JSScript* script =
>      JS_CompileUCScriptForPrincipals(mCx, global, GetPrincipal(),
>                                      aString.get(), aString.Length(),
>                                      "typein", 0);
>  if (!script) {
>     return false;
>  }

This is probably not ideal.

Comment 1

[Josh Matthews [:jdm]]

I think all this needs is a change from |return false| to |return true|, but it's worth testing to see what the output looks like when this fails.  I'm willing to explain how to do that for anyone who picks this up.

Comment 2

[Josh Matthews [:jdm]]

The way to test this is to take any xpcshell test that runs in the content process (such as anything wrapped by a test in netwerk/test/unit_ipc/ (see netwerk/test/unit/ for the originals)) and introduce a syntax error. If you run the test |SOLO_FILE=test_foo_wrap.js make -C objdir/netwerk/test/ check-one|, you should see a crash right now. Changing the line in linked in the URL field should make this crash go away.

Comment 3

[Brijesh Patel]

I would like to work on this bug....

Comment 4

[Brijesh Patel]

Attachment: patch_1

Comment 5

[Brijesh Patel]

Attachment: patch_2

reverted back the change in test_httpcancel_wrap.js

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 582574 [details] [diff] [review]
patch_2

Review of attachment 582574 [details] [diff] [review]:
-----------------------------------------------------------------

I think what you really want to do is change TestShellChild::RecvExecuteCommand and TestShellChild::RecvPTestShellCommandConstructor to not return false if EvaluateString returns false. XPCShellEnvironment should just do its job and report whether the command succeeded or not.

Comment 7

[Sudheera Palihakkara]

I like to work on this one.

Comment 8

[Josh Matthews [:jdm]]

Actually, this code is being removed very soon, so it's not worth the effort to fix this any longer. Sorry.