Bug 61653 - Web Inspector: [REGRESSION r86838] line numbers do not scroll when script paused
Summary: Web Inspector: [REGRESSION r86838] line numbers do not scroll when script paused
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.6
: P2 Normal
Assignee: David Grogan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-05-27 11:24 PDT by Graham Ballantyne
Modified: 2011-06-27 18:01 PDT (History)
16 users (show)

See Also:


Attachments
Patch (8.55 KB, patch)
2011-06-23 14:06 PDT, David Grogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Graham Ballantyne 2011-05-27 11:24:38 PDT
Summary: line numbers in the Web Inspector's script pane do not scroll with the script content when the script is paused.

Steps to reproduce:
1. Open the script pane.
2. Pause script execution, either by clicking the Pause button or by stopping on a breakpoint. Use a sufficently large script or resize the window so that scrollbars appear.
3. Scroll the script content up and down.

Expected behaviour: Line numbers scroll with the content.

Actual behaviour: line numbers are fixed and do not scroll with content. See video here: (14mb http://dl.dropbox.com/u/33854/webinspectorescroll.mov)

Problem occurs in WebKit nightly build r87475. Does not happen in Chrome dev channel (13.0.772.0 dev).
Comment 1 tonistiigi 2011-05-31 13:37:20 PDT
Starts to appear after http://trac.webkit.org/changeset/86838
Comment 2 Pavel Feldman 2011-05-31 21:51:32 PDT
This only affects in-process inspector operation (use WebKit nightly, not Chromium to repro).
Comment 3 David Grogan 2011-06-01 12:02:25 PDT
I will look into this shortly.  I saw it happen once when testing this patch but thought I'd fixed it.
Comment 4 David Grogan 2011-06-14 21:40:38 PDT
I'll need some guidance with this.

The Task containing the dom scroll event is queued in WTF::MainThread's functionQueue, but that queue isn't serviced when the javascript debugger calls setMainThreadCallbacksPaused on pause.  I suppose the old DOMTimer way didn't have this problem.

Can anyone on the cc list recommend someone for me to talk to about what the options might be?
Comment 5 Pavel Feldman 2011-06-22 22:26:02 PDT
I would talk to Yury (yurys). Unless http://trac.webkit.org/changeset/86838 is fixing a regression, I'd roll it out.
Comment 6 Joseph Pecoraro 2011-06-22 22:27:02 PDT
<rdar://problem/9661223>
Comment 7 David Grogan 2011-06-23 14:06:08 PDT
Created attachment 98397 [details]
Patch
Comment 8 Pavel Feldman 2011-06-24 01:24:27 PDT
Comment on attachment 98397 [details]
Patch

Clearing flags on attachment: 98397

Committed r89657: <http://trac.webkit.org/changeset/89657>
Comment 9 Pavel Feldman 2011-06-24 01:24:35 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Dmitry Titov 2011-06-24 09:37:27 PDT
Did we understand why it works with SuspendableTimer and does not work with postTask()?

The logic here sounds counterintuitive - if the idea of suspension main thread tasks is to prevent JS events from being dispatched while debugger has it 'paused', then:

- is it wrong that DOMTimers are still dispatched? (this causes JS callbacks)
- what other functionality of debug page is broken because tasks are not dispatched? postTask is a popular mechanism of doing things in WebKit...
Comment 11 Yury Semikhatsky 2011-06-27 02:39:57 PDT
(In reply to comment #10)
> Did we understand why it works with SuspendableTimer and does not work with postTask()?
> 
> The logic here sounds counterintuitive - if the idea of suspension main thread tasks is to prevent JS events from being dispatched while debugger has it 'paused', then:
> 
> - is it wrong that DOMTimers are still dispatched? (this causes JS callbacks)
> - what other functionality of debug page is broken because tasks are not dispatched? postTask is a popular mechanism of doing things in WebKit...

All active DOM objects inside inspected page are paused(see PageScriptDebugServer::didPause method: http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/js/PageScriptDebugServer.cpp&type=cs ), this means, in particular, that its DOMTimers will not be dispatched.
Comment 12 David Grogan 2011-06-27 14:19:46 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > Did we understand why it works with SuspendableTimer and does not work with postTask()?
> > 
> > The logic here sounds counterintuitive - if the idea of suspension main thread tasks is to prevent JS events from being dispatched while debugger has it 'paused', then:
> > 
> > - is it wrong that DOMTimers are still dispatched? (this causes JS callbacks)
> > - what other functionality of debug page is broken because tasks are not dispatched? postTask is a popular mechanism of doing things in WebKit...
> 
> All active DOM objects inside inspected page are paused(see PageScriptDebugServer::didPause method: http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/js/PageScriptDebugServer.cpp&type=cs ), this means, in particular, that its DOMTimers will not be dispatched.

I am confused.  When we use DOMTimers to fire asynchronous events, the events are fired even when the the web inspector has paused javascript execution.  When we switched to using postTask, the events were no longer fired.

From what you're saying, it seems as if DOMTimer should not work, but we've found it's the only thing that does.
Comment 13 Dmitry Titov 2011-06-27 18:01:17 PDT
(In reply to comment #12)

Actually, what Yury says makes sense. The ActiveDOMObjects are suspended on per-Document basis - in other words, only frames in the page that gets paused are suspended, other SuspendableTimers are continuing to fire - so the debugger page still gets its timers fired. So you may have one page with suspended timers and another with running ones.

That's fine and it addresses my question #1. But it doesn't address #2 - disabling the main thread callbacks wholesale in the browser process will eventually cause issues, and we just found one example here. The problem was not using the Document::postTask() mechanism but the fact that this mechanism can only be paused globally.

For example, suspending the script in a single page will stop all pages in the other tabs of the same single-process browser from doing Workers, Databases, FileAPI, media player and other things which use callOnMainThread().

If it was possible to suspend tasks per-ScriptExecutionContext, then it would be a better fix. The David's change would not regress things then because the main thread queue would be only suspended for the paused page. It would also avoid freezing various internal things that use callOnMainThread() for other, unrelated, pages.