WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
71271
[Qt] Web Inspector: local inspector client UI becomes unresponsive on debugger pause during
https://bugs.webkit.org/show_bug.cgi?id=71271
Summary
[Qt] Web Inspector: local inspector client UI becomes unresponsive on debugge...
Yury Semikhatsky
Reported
2011-11-01 01:03:45 PDT
Steps to reproduce: 1. Set a breakpoint. 2. Trigger the breakpoint. Result: Inspector UI is unresponsive.
Attachments
Patch
(2.46 KB, patch)
2012-01-25 09:41 PST
,
Leo Franchi
no flags
Details
Formatted Diff
Diff
Patch
(1.32 KB, patch)
2012-05-17 01:27 PDT
,
Yaroslav Tsarko
no flags
Details
Formatted Diff
Diff
Patch
(2.29 KB, patch)
2012-05-17 03:22 PDT
,
Yaroslav Tsarko
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Mozhaev Grigory
Comment 1
2012-01-15 11:28:19 PST
The bug still exists. I tested it on Windows and Linux platforms. It works fine with webkit from Qt-4.7.4 but doesn't work with webkit from trunk and from Qt-4.8. I did some rough hack - removed isPaused() statement from a couple of the conditions in the following source files: WebCore/bindings/js/JSLazyEventListener.cpp (in ScriptValue ScriptController::executeScriptInWorld() func) WebCore/bindings/js/ScriptController.cpp (in JSLazyEventListener::initializeJSFunction() func) WebCore/bindings/js/JSEventListener.cpp (in JSEventListener::handleEvent() func) Just like that: if (!script->canExecuteScripts(AboutToExecuteScript))// || script->isPaused()) and then GUI becomes partially responsive. After this I did a little bit more research and have got that: when breakpoint paused - nothing changed in GUI, although "Debugger.paused" message sent to frontend page with success, but didn't executed (due to locked JS context). If you clicked on 'workers debug' combobox (or any other GUI part which interacts with backend) and then this method will be executed and GUI will be updated to paused state. Hope it can help somehow. I think that solving of this bug is very important (not just P2) - because no way to make local debug of javascript from inspector while it still there and it's very annoying for js developers who use qt-webkit. Any clues?
Mozhaev Grigory
Comment 2
2012-01-15 11:47:48 PST
(In reply to
comment #1
)
> If you clicked on 'workers debug' combobox
Sorry. Ofc, I meant 'workers debug' checkbox.
Leo Franchi
Comment 3
2012-01-15 14:32:53 PST
I think I have some patches that fix this bug, though they are not ready yet for upstreaming. I will try to find some time next week for the upstreaming. Here they are: 1) Updated Websocket implementation to latest spec to work against a recent chrome/webkit browser:
https://gitorious.org/~lfranchi/webkit/lfranchis-qtwebkit/commit/93a72f2657c2a8bb6863d5d2ffe8ce6faedd4598
2) 2 fixes for hanging on breakpoint (one requires the above commit, but is easy enough to apply against trunk if you read the code)
https://gitorious.org/~lfranchi/webkit/lfranchis-qtwebkit/commit/4e104bc438c21ef63a6a4227a142d36bf2401b2e
https://gitorious.org/~lfranchi/webkit/lfranchis-qtwebkit/commit/1606e202891b856ad0fdae80288ccdbf8d6765e1
The latter two patches I hope should fix your issue. All 3 are also available as a standalone patch against qt 4.8 here:
https://github.com/ariya/phantomjs/blob/master/deploy/qt48_fix_inspector.patch
Mozhaev Grigory
Comment 4
2012-01-15 14:35:16 PST
(In reply to
comment #3
) Thanks for the answer. I saw these patches, they doesn't solve current bug, since these patches are for Remote Debug, but this bug report actually is for Local Debug problem.
Mozhaev Grigory
Comment 5
2012-01-16 08:17:50 PST
(In reply to
comment #1
)
> I did some rough hack - removed isPaused() statement from a couple of the conditions in the following source files: > > WebCore/bindings/js/JSLazyEventListener.cpp (in ScriptValue ScriptController::executeScriptInWorld() func) > WebCore/bindings/js/ScriptController.cpp (in JSLazyEventListener::initializeJSFunction() func) > WebCore/bindings/js/JSEventListener.cpp (in JSEventListener::handleEvent() func) > > Just like that: > if (!script->canExecuteScripts(AboutToExecuteScript))// || script->isPaused()) >
I forgot to mention, what I also commented the following condition part: if (!scriptExecutionContext) // || scriptExecutionContext->isJSExecutionForbidden()) return; in WebCore/bindings/js/JSEventListener.cpp (void JSEventListener::handleEvent(...) func)
Mozhaev Grigory
Comment 6
2012-01-16 08:52:59 PST
The above partial workaround works only on linux, on windows it still freezes the system. Anyone have any clues?
Mozhaev Grigory
Comment 7
2012-01-16 15:52:21 PST
(In reply to
comment #3
) Thank you Leo for patches you made! We are using your patches to make remote debugging works, until local debugging will be fixed.
Leo Franchi
Comment 8
2012-01-17 09:24:31 PST
Glad that works for you partially at least. I remember now also hitting issues with local debugging, but as I only needed remote debugging I didn't spend too much time on that. Next week i'll clean up my patches and see if I can get them committed upstream.
Mozhaev Grigory
Comment 9
2012-01-17 09:27:42 PST
(In reply to
comment #8
)
> Glad that works for you partially at least. I remember now also hitting issues with local debugging, but as I only needed remote debugging I didn't spend too much time on that. > > Next week i'll clean up my patches and see if I can get them committed upstream.
Thanks again. Btw, it works only with last Chrome (with 'm' label in version) and only in Windows Chrome's version. Safari (on Windows and Mac) and Chrome on Mac doesn't work with patched remote debug - got 404 error and empty inspector.
Mozhaev Grigory
Comment 10
2012-01-17 09:30:28 PST
I will gladly help anyone who want to fix current bug, as I'm C++/Qt programmer. Maybe some webkit's guru give any hint or direction to where dig, it will safe a lot time.
Leo Franchi
Comment 11
2012-01-25 09:41:03 PST
Created
attachment 123960
[details]
Patch
Jarred Nicholls
Comment 12
2012-01-25 09:43:40 PST
Comment on
attachment 123960
[details]
Patch This addresses remote debugging only, when this bug is primarily referring to local debugging where the JS context seems to pause for the Inspector client in addition to the Frame that's being inspected. I suggest creating a new ticket for the remote debugger fixes.
Leo Franchi
Comment 13
2012-01-25 10:07:23 PST
(In reply to
comment #12
)
> (From update of
attachment 123960
[details]
) > This addresses remote debugging only, when this bug is primarily referring to local debugging where the JS context seems to pause for the Inspector client in addition to the Frame that's being inspected. I suggest creating a new ticket for the remote debugger fixes.
Thanks, I obsoleted the patch. The new ticket is here:
https://bugs.webkit.org/process_bug.cgi
Leo Franchi
Comment 14
2012-01-25 10:08:02 PST
By that I meant here:
https://bugs.webkit.org/show_bug.cgi?id=77022
Manuel Lazzari
Comment 15
2012-02-15 03:04:40 PST
Not sure that what's happening in our hybrid QtWebKit app with Qt 4.8.0 compiled from source on Windows and Mac OS X is related/linked to this bug, if not please advise. When I put a breakpoint in the javascript debugger and i trigger the breakpoint the javascript execution does stop but the debugger becames unusable/broken: * the debugger does not seems to be in breakpoint (e.g. there are no stepper buttons like the "continue" button). * scrolling through the javascript code causes the code div to scroll but the line numbers do not scroll anymore. * closing the web inspector window will cause the javascript execution to continue: I suppose that the breakpoint was correctly triggered and cleared when exiting the window.
Mozhaev Grigory
Comment 16
2012-02-15 03:07:45 PST
(In reply to
comment #15
)
> Not sure that what's happening in our hybrid QtWebKit app with Qt 4.8.0 compiled from source on Windows and Mac OS X is related/linked to this bug, if not please advise.
This is exactly the problem for which this bugreport was made.
Manuel Lazzari
Comment 17
2012-02-15 03:09:42 PST
(In reply to
comment #16
)
> This is exactly the problem for which this bugreport was made.
Thanks, that's at least comfort me a little :)
Manuel Lazzari
Comment 18
2012-03-05 23:58:24 PST
Is anyone working on this? Is there any patch or workaround? How this "remote debugging" works with Qt 4.8?
Mozhaev Grigory
Comment 19
2012-03-06 04:16:05 PST
(In reply to
comment #18
)
> Is anyone working on this? Is there any patch or workaround? How this "remote debugging" works with Qt 4.8?
"remote debugging" works with Qt 4.8 well (with patched version or webkit), but local debugging still hasn't any workaround.
Saulo
Comment 20
2012-04-03 08:29:15 PDT
It is still broken on Qt-4.8.1.
Yaroslav Tsarko
Comment 21
2012-05-17 01:27:46 PDT
Created
attachment 142441
[details]
Patch Greetings everyone! After 3 days of debugging JavaScript debugger in QtWebkit I have found the reason of Web Inspector GUI lock when debuggee page was paused. The reason itself is quite trivial: Web Inspector front-end (also written in JavaScript) was also paused along with the debuggee page itself. I suppose that this bug has been introduced by commit a2ce7739 (qtwebkit repo) when every QWebPage being created was explicitly added to PageGroup called "Default Group". When debugger pauses page it also pauses all the pages that belong to its page group. So the Web Inspector page was paused too. The applied patch fixes this problem (I`ve tested it with Qt 4.8.1 with QtWebkit 2.2.2).
zalan
Comment 22
2012-05-17 01:59:39 PDT
Comment on
attachment 142441
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=142441&action=review
Good catch! However you are missing changelog entry. Please look at the following page
http://www.webkit.org/coding/contributing.html
and submit your patch accordingly.
> Source/WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:67 > + // This leads that Web Inspector front-end is paused too leading to locked Web Inspector GUI (bugzilla ticket 71271).
No need to mention the bug# in the comment. Changelog entry should have it.
> Source/WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:68 > + handle()->page->setGroupName("__WebInspectorPageGroup__");
I'd rather have it in InspectorClient::openInspectorFrontend() where we actually open the front end, something like m_frontendWebPage->setGroupName("group_name_comes_here");
Yaroslav Tsarko
Comment 23
2012-05-17 03:22:35 PDT
Created
attachment 142448
[details]
Patch Zalan, I have recreated the patch according to your comments. Please review it and commit to the repo if it is ok.
Jarred Nicholls
Comment 24
2012-05-17 05:38:53 PDT
Thanks Tsarko :)
WebKit Review Bot
Comment 25
2012-05-17 06:25:42 PDT
Comment on
attachment 142448
[details]
Patch Clearing flags on attachment: 142448 Committed
r117440
: <
http://trac.webkit.org/changeset/117440
>
WebKit Review Bot
Comment 26
2012-05-17 06:25:50 PDT
All reviewed patches have been landed. Closing bug.
Yaroslav Tsarko
Comment 27
2012-05-17 21:33:45 PDT
(In reply to
comment #26
)
> All reviewed patches have been landed. Closing bug.
Is there any chance that this fix will be merged into qtwebkit (git) repository, branch qtwebkit-2.2?
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug