Bug 94949

Summary: [Qt][WK2] REGRESSION(r126067): It made qmltests::DoubleTapToZoom::test_basic() fail
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: New BugsAssignee: Simon Hausmann <hausmann>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cmarcelo, gaborb, hausmann, kenneth, loki, menard, noam, oliver, ossy, pvarga, webkit.review.bot, zherczeg, zoltan
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 70236, 94493    
Attachments:
Description Flags
Patch kenneth: review+

Csaba Osztrogonác
Reported 2012-08-24 10:15:32 PDT
After https://trac.webkit.org/changeset/126067 qmltests::DoubleTapToZoom::test_basic() Qt-WK2 API test started to fail regularly. (not always, but in 90% of runs) I got it with manual bisecting. It doesn't fail on r126066, but it fails almost always on r126067. FAIL! : qmltests::DoubleTapToZoom::test_basic() Uncaught exception: Cannot read property 'width' of undefined Loc: [(0)] You can reproduce the bug if you run this binary: $ WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/qmltests/tst_qmltests_WebView
Attachments
Patch (1.80 KB, patch)
2012-08-27 02:03 PDT, Simon Hausmann 		kenneth: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2012-08-24 10:23:36 PDT
cc JSC/QML experts, maybe you know what is it and how to fix it
Chris Dumez
Comment 2 2012-08-24 10:52:45 PDT
It appears this QML attempts to serialize a ClientRect object (unhandled host object), which is not supported according to spec (http://www.whatwg.org/specs/web-apps/current-work/multipage/common-dom-interfaces.html#safe-passing-of-structured-data). As a consequence, a DataCloneError is being thrown. tst_doubleTapToZoom.qml code: function elementRect(id) { resultSpy.clear(); var result; webView.experimental.evaluateJavaScript( "document.getElementById('" + id + "').getBoundingClientRect();", function(rect) { webView.resultReceived(); result = rect }); resultSpy.wait(); return result; }
Kenneth Rohde Christiansen
Comment 3 2012-08-24 10:54:22 PDT
I haven't looked at this in details, but Chris made the serialization more compliant with the specs and thus more strict. getBoundingClientRect(); returns ClientRect object (http://www.w3.org/TR/cssom-view/#clientrect) so it is not serializable and it has to be turned into an object literal (Object object) before serialization. This following line in Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml function(rect) { webView.resultReceived(); result = rect }); needs to be changed to function(rect) { webView.resultReceived(); result = { width: rect.width, height: rect.height } }); or similar
Kenneth Rohde Christiansen
Comment 4 2012-08-24 10:57:54 PDT
Btw you need to do this before serialization! webView.experimental.evaluateJavaScript( "var clientRect = document.getElementById('" + id + "').getBoundingClientRect();" + "return { width: clientRect.width, height: clientRect.height }", function(rect) { webView.resultReceived(); result = rect });
Simon Hausmann
Comment 5 2012-08-27 02:03:38 PDT
Created attachment 160674 [details] Patch
Kenneth Rohde Christiansen
Comment 6 2012-08-27 02:12:05 PDT
Comment on attachment 160674 [details] Patch Funny I thought about this yesterday... I should have mentioned to just use the JSON methods, and here we go :-)
Simon Hausmann
Comment 7 2012-08-27 02:56:55 PDT
I still wonder if for the QML API we should automatically imply a JSON conversion or instead keep the current implementation and document that the return value will be cloned according to the html cloning algo.
Simon Hausmann
Comment 8 2012-08-27 02:59:42 PDT
Committed r126740: <http://trac.webkit.org/changeset/126740>
Note You need to log in before you can comment on or make changes to this bug.