Bug 17133

Summary: Should support pausing JavaScript execution without hanging the process
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: ap, beidson, fpizlo, ggaren, mjs, p.jacquemart, xfdbse
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 17134    
Attachments:
Description Flags
[WIP] Make it possible to pause JS timers and block JS event handlers per-Frame
none
[WIP] Make it possible to pause/resume the DatabaseThread
none
[WIP] Make it possible to pause/block JS from plugins (Windows only)
none
[WIP] Make it possible to pause/resume callOnMainThread callbacks (Windows only)
none
[WIP (revised)] Pause/resume of callOnMainThread (Windows only)
none
[WIP] Make more MainThread code cross-platform
none
[WIP (revised 2)] Pause/resume of callOnMainThread
none
[WIP (revised)] Make more MainThread code shareable
none
[WIP] Share most MainThread code across all platforms
none
Make most of callOnMainThread be cross-platform
none
[1/2] Make PausedTimeouts be internal to JSDOMWindowBase
none
[2/2] Remove PausedTimeouts
none
[WIP] Add KJSProxy::[set]JavaScriptExecutionAllowed to pause timeouts and block JS event handlers
none
[WIP] Pause JS timers, block javascript: URIs, block JS event handlers
none
[WIP] Block javascript: URIs and JS event handlers while paused
none
Block JS event handlers/javascript: URIs per-Frame
none
Pause callOnMainThread callbacks
none
Pause/block JS execution from plugins
none
Don't allow newly-scheduled plugin requests to be serviced while JS is paused none

Description Adam Roben (:aroben) 2008-02-01 08:01:25 PST
Currently the only way to pause JavaScript execution is to block the entire process (which is the approach taken by Drosera). It would be much nicer if we could pause JavaScript execution without blocking the process. This would, for example, allow us to build an in-process JavaScript debugger, a la Firebug.
Comment 1 Adam Roben (:aroben) 2008-02-01 08:01:51 PST
See bug 8285 for a previous attempt at reworking the JS interpreter in a way that would have allowed pausing execution.
Comment 2 Adam Roben (:aroben) 2008-02-01 08:09:13 PST
<rdar://problem/5719551>
Comment 3 Adam Roben (:aroben) 2008-03-07 08:32:09 PST
It seems that Firebug, while paused at a breakpoint, basically stops any new JavaScript execution from being initiated. In particular:

1. JavaScript event handlers never run
2. Code scheduled to be executed via setTimeout/setInterval executes immediately when JS is resumed if the timer went off while paused
Comment 4 Adam Roben (:aroben) 2008-03-07 08:33:33 PST
I think we can follow Firebug/Gecko's model here in WebKit.

(In reply to comment #3)
> 2. Code scheduled to be executed via setTimeout/setInterval executes
> immediately when JS is resumed if the timer went off while paused

This doesn't seem like a particularly good behavior. I think it would be better to pause the timers themselves while JS is paused, and resume the timers once you resume.
Comment 5 Adam Roben (:aroben) 2008-03-07 08:34:00 PST
Another thing we'll have to pause are callbacks from the SQL storage API.
Comment 6 Adam Roben (:aroben) 2008-03-07 08:37:32 PST
We'll also want to block plugins from obtaining access to the DOM, and we'll probably want to pause all plugin requests, since they can request javascript: URIs.
Comment 7 Adam Roben (:aroben) 2008-03-07 08:38:42 PST
To actually pause at a breakpoint, I think we'll just need to spin up a nested event loop.
Comment 8 Adam Roben (:aroben) 2008-03-07 09:23:14 PST
Created attachment 19588 [details]
[WIP] Make it possible to pause JS timers and block JS event handlers per-Frame
Comment 9 Adam Roben (:aroben) 2008-03-07 09:23:40 PST
Created attachment 19589 [details]
[WIP] Make it possible to pause/resume the DatabaseThread
Comment 10 Adam Roben (:aroben) 2008-03-07 09:24:05 PST
Created attachment 19590 [details]
[WIP] Make it possible to pause/block JS from plugins (Windows only)
Comment 11 Anders Carlsson 2008-03-07 09:34:11 PST
The JS plugin approach seems good but the patch is insufficient. Since a plug-in can hold on to any NPObject, any NPRuntime call that can result in JS being executed needcs to be handled in the same way as NPN_GetValue.
Comment 12 Adam Roben (:aroben) 2008-03-07 09:51:01 PST
Anders thinks that maybe the current patch, while theoretically insufficient, might be good enough, practically speaking:

<andersca> aroben: I think that all the plug-ins I've encountered always get the window NPObject before calling JS
<andersca> aroben: so for now you might be able to get away with just preventing getvalue
Comment 13 Adam Roben (:aroben) 2008-03-07 09:52:13 PST
(In reply to comment #11)
> The JS plugin approach seems good but the patch is insufficient. Since a
> plug-in can hold on to any NPObject, any NPRuntime call that can result in JS
> being executed needcs to be handled in the same way as NPN_GetValue.

If we decide to address this issue we will have to patch the methods in WebCore/bridge/NP_jsobject.cpp.

Comment 14 Timothy Hatcher 2008-03-07 10:54:26 PST
On Mac we will need to prevent WebScriptObject from making any calls. The evaluate methods and setValue:forKey:. I can look at doing this.
Comment 15 Adam Roben (:aroben) 2008-03-07 12:00:05 PST
(In reply to comment #14)
> On Mac we will need to prevent WebScriptObject from making any calls. The
> evaluate methods and setValue:forKey:. I can look at doing this.

Is this to prevent WebKit clients from calling into JavaScript? Or is it about WebKit plugins? I've made no attempts to block WebKit clients so far.

Comment 16 Timothy Hatcher 2008-03-10 14:06:07 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > On Mac we will need to prevent WebScriptObject from making any calls. The
> > evaluate methods and setValue:forKey:. I can look at doing this.
> 
> Is this to prevent WebKit clients from calling into JavaScript? Or is it about
> WebKit plugins? I've made no attempts to block WebKit clients so far.

Really both.
Comment 17 Adam Roben (:aroben) 2008-03-12 23:34:09 PDT
Comment on attachment 19589 [details]
[WIP] Make it possible to pause/resume the DatabaseThread

After discussing with Alexey, I've realized that this patch is not sufficient. One problem with it is that it does not stop already-queued callbacks from executing.

To fix this completely Alexey and I think we'll need a way to pause all queued actions passed to callOnMainThread. This will require patching each platform's implementation of this function separately.
Comment 18 Adam Roben (:aroben) 2008-03-13 08:33:24 PDT
Created attachment 19725 [details]
[WIP] Make it possible to pause/resume callOnMainThread callbacks (Windows only)

This patch replaces the DatabaseThread patch. It allows you to pause/resume callbacks scheduled through callOnMainThread, which is what the DatabaseThread uses to schedule transaction callbacks. This implementation is Windows-only, just to have a reference point for talking about this concept.
Comment 19 Adam Roben (:aroben) 2008-03-13 08:40:57 PDT
Created attachment 19726 [details]
[WIP (revised)] Pause/resume of callOnMainThread (Windows only)

This revised patch removes the guard of callbacksPaused and adds a comment that setMainThreadCallbacksPaused must be called from the main thread. It also fixes a reversed assignment.
Comment 20 Adam Roben (:aroben) 2008-03-13 09:07:58 PDT
Created attachment 19728 [details]
[WIP] Make more MainThread code cross-platform

This patch makes more of the MainThread code from the Windows implementation cross-platform so that it may be shared with other platforms.
Comment 21 Adam Roben (:aroben) 2008-03-13 09:09:26 PDT
Created attachment 19729 [details]
[WIP (revised 2)] Pause/resume of callOnMainThread

New patch that relies on attachment 19728 [details]
Comment 22 Adam Roben (:aroben) 2008-03-13 09:15:25 PDT
Created attachment 19730 [details]
[WIP (revised)] Make more MainThread code shareable
Comment 23 Adam Roben (:aroben) 2008-03-13 09:49:39 PDT
Created attachment 19731 [details]
[WIP] Share most MainThread code across all platforms

This patch is missing a change to WebCore.xcodeproj to add MainThread.cpp to the project, but other than that (and need a ChangeLog) it should be ready to go.
Comment 24 Adam Roben (:aroben) 2008-03-13 11:49:46 PDT
Comment on attachment 19729 [details]
[WIP (revised 2)] Pause/resume of callOnMainThread

This patch now works cross-platform (note that it depends on attachment 19731 [details]).
Comment 25 Adam Roben (:aroben) 2008-03-13 12:11:15 PDT
Here are the ways I currently know that JavaScript execution can be initiated:

1. JavaScript event handlers
2. Callbacks from window.setTimeout/window.setInterval
3. Callbacks from the client-side database storage API
4. JavaScript access from plugins (both NPAPI and WebKit plugins)
5. Parsing of a <script> tag in a document
6. Loading of a script resource completing
7. Loading of javascript: URIs
8. Calls from WebKit clients
Comment 26 Adam Roben (:aroben) 2008-03-13 12:19:10 PDT
(In reply to comment #25)
> Here are the ways I currently know that JavaScript execution can be initiated:
> 
> 1. JavaScript event handlers
> 2. Callbacks from window.setTimeout/window.setInterval

These are handled by attachment 19588 [details] (except for the FIXME about timers scheduled while paused).

> 3. Callbacks from the client-side database storage API

This is handled by attachment 19729 [details].

> 4. JavaScript access from plugins (both NPAPI and WebKit plugins)

This is handled for Windows by attachment 19590 [details]. Making this work on Mac will involve similar changes to WebBaseNetscapePluginView and blocking calls to WebScriptObject.

> 5. Parsing of a <script> tag in a document
> 6. Loading of a script resource completing

I believe these can be handled by calling Page::setDefersLoading(true).

> 7. Loading of javascript: URIs

I don't yet have a solution for this.

> 8. Calls from WebKit clients

I don't yet plan to try to block this, since for the moment I'm only concerned with getting this working for in-process JavaScript debugging in the Inspector (bug 17134), and I think we can get by without this for those purposes.
Comment 27 Adam Roben (:aroben) 2008-03-13 14:56:14 PDT
Created attachment 19744 [details]
Make most of callOnMainThread be cross-platform
Comment 28 Adam Roben (:aroben) 2008-03-13 21:07:31 PDT
(In reply to comment #26)
> (In reply to comment #25)
> > Here are the ways I currently know that JavaScript execution can be initiated:
> > 7. Loading of javascript: URIs
> 
> I don't yet have a solution for this.

I think I now understand how to deal with this.

Here are the ways a javascript: URI may be evaluated:

A. Parsing of a <frame>/<iframe> tag that has a javascript: URI as the value of its src attribute

   This can be prevented by Page::setDefersLoading(true), just as with parsing of <script> elements.

B. Programmatically changing window.location or the src attribute of a <frame>/<iframe> to a javascript: URI

   This will be prevented simply because we're going to be pausing JavaScript.

C. User activating an <a> that has a javascript: URI as the value of its href attribute
D. User submitting a form that has a javascript: URI as the value of its action attribute

   These we will need to block. Programmatic activation/submission (e.g., anchor.click()/form.submit()) should be allowed.

E. "Navigation" of the main frame to a javascript: URI by the WebKit client

   This falls under the general category of calls from WebKit clients that execute JavaScript (this is item (8) from comment 25). I'm not concerned with this right now.

So, all we need to do is make sure that the link's/form's javascript: URI is not evaluated if the user clicks a link/submits a form.
Comment 29 Adam Roben (:aroben) 2008-03-13 21:09:18 PDT
(In reply to comment #28)
> B. Programmatically changing window.location or the src attribute of a
> <frame>/<iframe> to a javascript: URI
> 
>    This will be prevented simply because we're going to be pausing JavaScript.

I should be slightly clearer here. If the programmatic change is in the code that we are stepping through in the debugger, we will allow this to happen. If the programmatic change is in other code, it won't be executing because JavaScript will be paused.
Comment 30 Alexey Proskuryakov 2008-03-14 09:37:10 PDT
Comment on attachment 19744 [details]
Make most of callOnMainThread be cross-platform

r=me

+				RelativePath="..\platform\MainThreadInternal.h"

I'm not sure if I love the file name "MainThreadInternal" - Internal has all sorts of connotations, which do not apply here :)

I think I'd just keep these functions in the same file, although either way is OK with me.

-    if (processingCustomThreadingMessage)
-        LOG(Threading, "callOnMainThread() called recursively.  Beware of nested PostMessage()s");
+    if (dispatchingCallbacks)
+        LOG(Threading, "callOnMainThread() called recursively.");

I do not understand this check. What's the problem with a thread making a callback when the main thread is working through previous ones (possibly from other threads)? I suggest removing it, if a reason doesn't come up.
Comment 31 Adam Roben (:aroben) 2008-03-14 09:40:16 PDT
(In reply to comment #30)
> (From update of attachment 19744 [details] [edit])
> -    if (processingCustomThreadingMessage)
> -        LOG(Threading, "callOnMainThread() called recursively.  Beware of
> nested PostMessage()s");
> +    if (dispatchingCallbacks)
> +        LOG(Threading, "callOnMainThread() called recursively.");
> 
> I do not understand this check. What's the problem with a thread making a
> callback when the main thread is working through previous ones (possibly from
> other threads)? I suggest removing it, if a reason doesn't come up.

Hopefully Brady can shed some light here, since he wrote the code originally.
Comment 32 Brady Eidson 2008-03-14 10:04:31 PDT
I'm fairly certain I modeled the original code after a public domain "Call a function on the main thread from any other thread in Windows" tutorial.

It's been awhile - I remember the concern of an infinite recursion into PostMessage, but don't remember details beyond that.  

If someone more familiar with the Windows message loop decides the concern is unwarranted in this application, fine by me to remove it!
Comment 33 Adam Roben (:aroben) 2008-03-14 10:12:52 PDT
(In reply to comment #32)
> I'm fairly certain I modeled the original code after a public domain "Call a
> function on the main thread from any other thread in Windows" tutorial.
> 
> It's been awhile - I remember the concern of an infinite recursion into
> PostMessage, but don't remember details beyond that.  
> 
> If someone more familiar with the Windows message loop decides the concern is
> unwarranted in this application, fine by me to remove it!

I don't think there's a danger of recursion beneath PostMessage -- the whole point of PostMessage is that the message won't be dealt with until after we return to the message pump.

Thanks for the explanation!
Comment 34 Adam Roben (:aroben) 2008-03-14 12:28:24 PDT
Created attachment 19768 [details]
[1/2] Make PausedTimeouts be internal to JSDOMWindowBase
Comment 35 Adam Roben (:aroben) 2008-03-14 12:28:41 PDT
Created attachment 19769 [details]
[2/2] Remove PausedTimeouts
Comment 36 Adam Roben (:aroben) 2008-03-14 12:35:50 PDT
Created attachment 19770 [details]
[WIP] Add KJSProxy::[set]JavaScriptExecutionAllowed to pause timeouts and block JS event handlers

This depends on attachment 19769 [details].
Comment 37 Adam Roben (:aroben) 2008-03-14 12:41:24 PDT
Created attachment 19771 [details]
[WIP] Pause JS timers, block javascript: URIs, block JS event handlers

This patch adds KJSProxy::[set]JavaScriptExecutionAllowed. Calling setJavaScriptExecutionAllowed(false) will pause JS timers, block javascript: URIs, and block JS event handlers. This patch depends on attachment 19769 [details].

This takes care of items (1), (2), and (7) from comment 25.
Comment 38 Adam Roben (:aroben) 2008-03-14 12:53:45 PDT
Comment on attachment 19744 [details]
Make most of callOnMainThread be cross-platform

Committed in r31063
Comment 39 Geoffrey Garen 2008-03-14 13:12:44 PDT
Comment on attachment 19771 [details]
[WIP] Pause JS timers, block javascript: URIs, block JS event handlers

I think setPaused / isPaused would be a better naming scheme than setJavaScriptExecutionAllowed  / javaScriptExecutionAllowed. It matches the style of setEnabled / isEnabled, it's more terse, and it's more specific about why JavaScript execution is not allowed, and what will happen to attempts to execute JavaScript.

+void KJSProxy::setJavaScriptExecutionAllowed(bool allowed)
+{
+    if (!m_globalObject)
+        return;
+

This code seems wrong. It means that a proxy that didn't have a global object wouldn't "get the memo" about JavaScript being paused, so it might start up JS execution at a later time.

r=me, with comments
Comment 40 Geoffrey Garen 2008-03-14 13:27:14 PDT
Comment on attachment 19768 [details]
[1/2] Make PausedTimeouts be internal to JSDOMWindowBase

You need to hand off the paused timeouts to the CachedPage, otherwise they can't resume when the CachedPage is displayed again.
Comment 41 Geoffrey Garen 2008-03-14 13:30:43 PDT
Comment on attachment 19769 [details]
[2/2] Remove PausedTimeouts

r=me
Comment 42 Adam Roben (:aroben) 2008-03-14 13:40:12 PDT
(In reply to comment #40)
> (From update of attachment 19768 [details] [edit])
> You need to hand off the paused timeouts to the CachedPage, otherwise they
> can't resume when the CachedPage is displayed again.

A little clarification for those following along at home: the JSDOMWindowBase object is reused for each navigation, and the CachedPage's job is to save off information from JSDOMWindowBase (and other objects) that needs to be restored later.
Comment 43 Adam Roben (:aroben) 2008-03-15 20:54:57 PDT
Comment on attachment 19771 [details]
[WIP] Pause JS timers, block javascript: URIs, block JS event handlers

After some more thinking, I've realized we don't need to worry about new timers being installed while JavaScript is paused. The timers can only be installed programmatically, which can't happen while JavaScript is paused.

So, the PausedTimeouts refactoring is not necessary, and I'll post a newer version of this patch.
Comment 44 Adam Roben (:aroben) 2008-03-15 21:07:09 PDT
Created attachment 19788 [details]
[WIP] Block javascript: URIs and JS event handlers while paused
Comment 45 Adam Roben (:aroben) 2008-03-20 16:55:18 PDT
Created attachment 19915 [details]
Block JS event handlers/javascript: URIs per-Frame
Comment 46 Adam Roben (:aroben) 2008-03-20 16:55:40 PDT
Created attachment 19916 [details]
Pause callOnMainThread callbacks
Comment 47 Adam Roben (:aroben) 2008-03-20 16:56:09 PDT
Created attachment 19917 [details]
Pause/block JS execution from plugins
Comment 48 Adam Roben (:aroben) 2008-03-20 17:15:04 PDT
Comment on attachment 19915 [details]
Block JS event handlers/javascript: URIs per-Frame

Committed in r31197
Comment 49 Adam Roben (:aroben) 2008-03-20 17:15:44 PDT
Comment on attachment 19916 [details]
Pause callOnMainThread callbacks

Committed in r31198
Comment 50 Adam Roben (:aroben) 2008-03-20 17:16:02 PDT
Comment on attachment 19917 [details]
Pause/block JS execution from plugins

Committed in r31199
Comment 51 Adam Roben (:aroben) 2008-03-21 08:03:54 PDT
Created attachment 19926 [details]
Don't allow newly-scheduled plugin requests to be serviced while JS is paused
Comment 52 Adam Roben (:aroben) 2008-03-21 09:29:06 PDT
Comment on attachment 19926 [details]
Don't allow newly-scheduled plugin requests to be serviced while JS is paused

Committed in r31210
Comment 53 Alexey Proskuryakov 2013-10-24 10:06:39 PDT
Is this still something we want (and haven't done yet)?
Comment 54 Adam Roben (:aroben) 2013-10-24 10:30:31 PDT
I'm pretty sure this task was finished (or finished enough to let the Inspector's JS debugger work) when the last patch landed, and I just forgot to close the bug.