670183 - perhaps DOMOperationCallback should be more ambitious with its script-killing

Status: NEW

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

Description

[Luke Wagner [:luke]]

bug 667915 demonstrated several cases where real websites were locking the browser by OOM'ing the slow script dialog JS.  One aspect of this problem was fixed in bug 667915 (chrome should have more stack than content), but it still raises the question: why did the DOMOperationCallback allow script execution to continue on error?  WebKit has the same stack problem but after throwing up a managled-from-OOM slow-script dialog, they kill script execution.

For this particular issue, the offender is this code in DOMOperationCallback:

  nsCOMPtr<nsIPrompt> prompt = GetPromptFromContext(ctx);
  NS_ENSURE_TRUE(prompt, JS_TRUE);

(returning JS_FALSE will kill script execution).  It definitely seems like this should be fixed, but there are a few other cases where, on error, we default to JS_TRUE.  Perhaps these should also be changed; at least audited.

Comment 1

[Boris Zbarsky [:bzbarsky]]

> For this particular issue, the offender is this code in DOMOperationCallback:

The problem is that this can't tell whether there's no prompt because this context is not tied to a window (e.g. some sandbox) or because of some sort of error....

Maybe we should err on the side of killing content scripts on no prompt and not killing chrome scripts?

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

How can content script run in a sandbox? Or otherwise have script not tied to a window?

I also seem to recall that we don't throw an uncatchable exception when a script was aborted due to the slow-script dialog. If that is indeed the case then we should IMHO fix that.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> How can content script run in a sandbox?

No idea.  Chances are it can't.

> Or otherwise have script not tied to a window?

I don't think it can.  Hence my proposal to kill content script if we can't get a prompt.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Sounds good to me then.

Comment 5

[Luke Wagner [:luke]]

So, can we go turn all those 'return JS_TRUE' to 'report-error + return JS_FALSE' ?

Comment 6

[(dormant account)]

p1 since it's a possible solution to 705695. Assigning to luke since he has a lot of wisdom on this.

Comment 7

[David Mandelin [:dmandelin]]

Luke says this is probably not needed for bug 705695 and is more of a cleanup bug, so it probably doesn't need Snappy:P1.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.