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.
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
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.
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.
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.
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
(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.
(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.
(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 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.
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.
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.
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.
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.
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
(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.
(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.
(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 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.
(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.
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!
(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!
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 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 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.
(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 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.
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.
2008-03-07 09:23 PST, Adam Roben (:aroben)
2008-03-07 09:23 PST, Adam Roben (:aroben)
2008-03-07 09:24 PST, Adam Roben (:aroben)
2008-03-13 08:33 PDT, Adam Roben (:aroben)
2008-03-13 08:40 PDT, Adam Roben (:aroben)
2008-03-13 09:07 PDT, Adam Roben (:aroben)
2008-03-13 09:09 PDT, Adam Roben (:aroben)
2008-03-13 09:15 PDT, Adam Roben (:aroben)
2008-03-13 09:49 PDT, Adam Roben (:aroben)
2008-03-13 14:56 PDT, Adam Roben (:aroben)
2008-03-14 12:28 PDT, Adam Roben (:aroben)
2008-03-14 12:28 PDT, Adam Roben (:aroben)
2008-03-14 12:35 PDT, Adam Roben (:aroben)
2008-03-14 12:41 PDT, Adam Roben (:aroben)
2008-03-15 21:07 PDT, Adam Roben (:aroben)
2008-03-20 16:55 PDT, Adam Roben (:aroben)
2008-03-20 16:55 PDT, Adam Roben (:aroben)
2008-03-20 16:56 PDT, Adam Roben (:aroben)
2008-03-21 08:03 PDT, Adam Roben (:aroben)