| Summary: | [Qt] Webkit hangs when executing an infinite JavaScript loop | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yael <yael> | ||||||||||
| Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | hausmann, laszlo.gombos, zecke | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 32539 [details]
Add UI asking the user if to stop running the script
Following the model of ChromeClientQt::runJavaScriptConfirm(), added a callback to QWebPage, to display a query to the user.
Comment on attachment 32539 [details]
Add UI asking the user if to stop running the script
Yael, we may not add new virtual methods as this will change the vtable and break ABI to previous versions. One common way is to create a new method with return value and use QMetaObject::invokeMethod. I assume Qt Software has an internal document describing what to do now. Besides that the patch looks fine.
Created attachment 32561 [details] Replace virtual method with signal/slot I followed the instructions from http://techbase.kde.org/Policies/Binary_Compatibility_Examples#Add_new_virtuals_to_a_non-leaf_class. I hope I got it right this time :-) Created attachment 32568 [details]
Replace the signal with QMetaObject::invokeMethod
Third time is a charm :-)
Comment on attachment 32568 [details] Replace the signal with QMetaObject::invokeMethod > Index: WebKit/qt/WebCoreSupport/ChromeClientQt.cpp > =================================================================== > --- WebKit/qt/WebCoreSupport/ChromeClientQt.cpp (revision 45709) > +++ WebKit/qt/WebCoreSupport/ChromeClientQt.cpp (working copy) > @@ -288,8 +288,7 @@ > > bool ChromeClientQt::shouldInterruptJavaScript() > { > - notImplemented(); > - return false; > + return QMetaObject::invokeMethod(m_webPage, "shouldInterruptJavaScript", Qt::DirectConnection); I don't think this is correct, invokeMethod returns a boolean indicating whether the function could be called or not, but the return value itself is placed in the fourth argument of invokeMethod. Also a unit test would be good for this :) An alternate solution would be to use QWebPage::supportsExtension and QWebPage::extension. Holger, do you have any preference? I Created attachment 32576 [details]
Added test case
I am thinking that using the extension is probably an overkill for this feature. It is very similar to JavaScript alert implementation. However, if there is a strong opinion otherwise, I will modify it again.
Comment on attachment 32576 [details] Added test case + JSTestPage(QObject* parent = 0) : QWebPage(parent) {} one style comment, could move the initializer list to the next line in accordance with the style guidelines? > +public slots: please make that public Q_SLOTS... the reason is that it could work with boost and other things. > /*! > + This function is called when a JavaScript program is running for a long period of time. > + > + If the user wanted to stop the JavaScript the implementation should return true; otherwise false. > + > + The default implementation executes the query using QMessageBox::information with QMessageBox::Yes and QMessageBox::No buttons. We need to add a @since 4.6 tag to the documentation. And please add some comments to make that virtual in the future? basically it is r=me when you change the two when landing. Comment on attachment 32576 [details]
Added test case
Yael wants to fix it before landing. So r=+ for me. We might have to discuss on monday if extensions are better...
Landed in 45731. |
When executing an infinite loop in JavaScript, the Browser hangs. That is because the built-in toimeout mechanism is not exposed through the Qt API. The following JavaScript can cause the crash: <html> <head> <script type="text/javascript"> var run = true; var a = 1; function startLoop(){ while(run){ a++; document.forms[0].elements[0].value=a; } } </script> </head><body> Activate the "StartLoop" link below to start an infinite loop by script execution.<br> <a href="#" onclick="startLoop()">Start Loop</a><br> <form><input></form> </body> </html> A patch will be uploaded shortly