771977 - unresponsive scripts in sandboxes associated with the hidden window are never terminated

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

The FrameWorker component created in bug 762569 creates iframes in the hidden window, and uses them as contexts for sandboxes, which then evaluate scripts retrieved from the social providers.

It's possible for the social provider's script to have bugs that could trigger an infinite loop, or otherwise unresponsive script. Right now, when this happens the browser becomes entirely unresponsive with no recourse - the slow script dialog doesn't appear, presumably because it can't usefully parent itself to the hidden window.

We need some mechanism to ensure that these situations don't result in a totally hosed browser. One way to achieve this may be to just cancel the script automatically if the slow script dialog fails to display, or something like that.

Related note: it would be awesome if we could somehow detect that the script had been killed, so that we can disable the guilty provider and/or notify the user.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #0)
> One way to achieve this may be to just cancel the
> script automatically if the slow script dialog fails to display, or
> something like that.

Hmm, this already seems to be what nsJSContext::DOMOperationCallback does. I'll investigate why this isn't working.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Oh, I was wrong, the case being hit is this one:

  rv = prompt->ConfirmEx(title, msg, buttonFlags, waitButton, stopButton,
                         debugButton, neverShowDlg, &neverShowDlgChk,
                         &buttonPressed);
...
  if (NS_FAILED(rv) || (buttonPressed == 0)) {
    // Allow the script to continue running

NS_FAILED(rv) is true, so we let the script continue running.

Can we just change that? What cases would we fail to prompt and want to continue running unresponsive scripts? This behavior seems to go back all the way to bug 13350, and I don't know that it's ever mattered in practice before now.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: proposed patch

This is the minimal workable solution - terminates the runaway script after the normal prompt timeout.

Ideally, we'd limit these scripts even further than that, and have some way to detect this occurring.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 640143 [details] [diff] [review]
proposed patch

I'm generally fine with this change, it means when unexpected things fails, we terminate scripts, as opposed to what we do now which is to let them run. I don't think we ever had a case where we needed this to work one way or another, it just seemed to be the safer bet, but in this case it's not. Alternatively we could of course write code that would explicitly check if we're running in a sandbox and make a decision based on that, but I don't think that's worth the effort right now.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

https://tbpl.mozilla.org/?tree=Try&rev=3124715357b3

Comment 6

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75b6ec6db468

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/75b6ec6db468