perhaps DOMOperationCallback should be more ambitious with its script-killing

NEW
Assigned to

Status

()

7 years ago
7 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Snappy:p1])

(Assignee)

Description

7 years ago
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.
> 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?
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.
> 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.
(Assignee)

Comment 5

7 years ago
So, can we go turn all those 'return JS_TRUE' to 'report-error + return JS_FALSE' ?
(Assignee)

Updated

7 years ago
Blocks: 699974
(Assignee)

Updated

7 years ago
Blocks: 705695
(Assignee)

Updated

7 years ago
Whiteboard: [Snappy]

Comment 6

7 years ago
p1 since it's a possible solution to 705695. Assigning to luke since he has a lot of wisdom on this.
Assignee: nobody → luke
Whiteboard: [Snappy] → [Snappy:p1]
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.
No longer blocks: 705695
You need to log in before you can comment on or make changes to this bug.