Bug 81143

Created attachment 131942 [details] Patch

(In reply to comment #1) > Created an attachment (id=131942) [details] > Patch This patch has a small workaround due to a segfault in Qt that happens everytime we try QQuickCanvas::isActive() on focusOutEvents. The issue was already reported and a fix for it is being reviewed. After it lands on Qt I can fix this and remove the //FIXME.

Comment on attachment 131942 [details] Patch why no url to the Qt bug?

(In reply to comment #3) > (From update of attachment 131942 [details]) > why no url to the Qt bug? http://codereview.qt-project.org/#change,20129 Sorry :)

In the code with the FIXME

(In reply to comment #5) > In the code with the FIXME I can fix when landing. Anything else?

Comment on attachment 131942 [details] Patch I will upload a new version now that the Qt Bug has been fixed. As soon as it goes through CI and we pick the new hash, I will land it. (after the r+ of course... :)

Created attachment 132081 [details] Patch

Comment on attachment 132081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132081&action=review > Source/WebKit2/UIProcess/qt/QtPageClient.cpp:259 > + return m_webView->canvas()->isActive(); What does it mean for web content if the view window is active? Should this perhaps be tied to the QQuickItem instead of the window?

(In reply to comment #9) > (From update of attachment 132081 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132081&action=review > > > Source/WebKit2/UIProcess/qt/QtPageClient.cpp:259 > > + return m_webView->canvas()->isActive(); > > What does it mean for web content if the view window is active? Should this perhaps be tied to the QQuickItem instead of the window? The PageClient API seems to have been thought for returning the Window state (PageClient::isViewWindowActive) for WebPageProxy (see WebPageProxy::viewStateDidChange). I can see this being tied to the Page Visibility Spec (http://www.w3.org/TR/page-visibility/), for example, in which there are states related to the state of the window itself (QQuickCanvas, in our case) and not directly to the view one. For instance, let's say we have multiple tabs (webviews) but only one is in foreground and, therefore, has focus. This one will considered active while the others will be considered not active even if their top level window (canvas()) still is. I believe that is why we differentiate these states in WebPageProxy as ViewIsFocused and ViewWindowIsActive (besides ViewIsVisible and ViewIsInWindow).

(In reply to comment #10) > The PageClient API seems to have been thought for returning the Window state (PageClient::isViewWindowActive) for WebPageProxy (see WebPageProxy::viewStateDidChange). I can see this being tied to the Page Visibility Spec (http://www.w3.org/TR/page-visibility/), for example, in which there are states related to the state of the window itself (QQuickCanvas, in our case) and not directly to the view one. By the way, such work is being tracked by: https://bugs.webkit.org/show_bug.cgi?id=81154 https://bugs.webkit.org/show_bug.cgi?id=81164 and https://bugs.webkit.org/show_bug.cgi?id=69554

Created attachment 132903 [details] Patch

(In reply to comment #12) > Created an attachment (id=132903) [details] > Patch Updated the patch due to a check that was missing.

Comment on attachment 132903 [details] Patch Clearing flags on attachment: 132903 Committed r111855: <http://trac.webkit.org/changeset/111855>

All reviewed patches have been landed. Closing bug.

Reopen, because it broke 30+ tests - http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r111856%20%2821981%29/results.html

(In reply to comment #16) > Reopen, because it broke 30+ tests - http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r111856%20%2821981%29/results.html I've found what caused the crashes and I'm now looking for a way to tweak the WebKitTestRunner and its WrapperWindow in our favor. I believe the problem is not with the patch itself but with some missing pieces in our PlatformWebView. I will update this bug as soon as I find out the root cause.

Created attachment 133579 [details] Patch

Comment on attachment 133579 [details] Patch Let's give it a try...

Comment on attachment 133579 [details] Patch Clearing flags on attachment: 133579 Committed r111993: <http://trac.webkit.org/changeset/111993>

All reviewed patches have been landed. Closing bug.

Reopen, because it broke many tests, see https://bugs.webkit.org/show_bug.cgi?id=82167 for details.

Rollout landed in http://trac.webkit.org/changeset/112073 (rs=darktears)

Comment on attachment 133579 [details] Patch We need to wait the next round of Qt5 updates on the bots before landing this.

Comment on attachment 133579 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133579&action=review > Tools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:83 > + // QWindow does not create the underlying platform > + // specific bits unless QWindow::setVisible() or QWindow::show() are called, > + // and without it QWindow::isActive() will always return false. > + // For these cases QWindow::create() is provided. // QWindow delays creating of the underlying platform specific bits until ::setVisible() // or ::show() are called. This makes ::isActive() always return false. In order to force // the creation ::create() exists, so we use it here.

Comment on attachment 133579 [details] Patch CQ has issues, I will land it manually.

Committed r113714: <http://trac.webkit.org/changeset/113714>

Reopen, because it broke API tests: FAIL! : qmltests::DesktopWebViewLinkHovered::test_linkHovered() 'wait for signal linkHovered' returned FALSE. () Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/DesktopBehavior/tst_linkHovered.qml(49)] FAIL! : qmltests::DesktopWebViewLinkHovered::test_linkHoveredDoesntEmitRepeated() 'wait for signal linkHovered' returned FALSE. () Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/DesktopBehavior/tst_linkHovered.qml(68)] FAIL! : qmltests::DesktopWebViewLoadHtml::test_baseUrlAfterLoadHtml() 'wait for signal linkHovered' returned FALSE. () Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/DesktopBehavior/tst_loadHtml.qml(48)] Could you check and fix it please?

Additionally it broke a layout test: --- /ramdisk/qt-linux-32-release-webkit2/build/layout-test-results/fast/events/mouseout-on-window-expected.txt +++ /ramdisk/qt-linux-32-release-webkit2/build/layout-test-results/fast/events/mouseout-on-window-actual.txt @@ -1,2 +1,2 @@ This test ensures that mouse out events are sent to the window. -PASS: Received mouseout event. +FAIL: No mouseout was sent.

And it made 24 new flakey test :(

Wow! Ok, I will have a look at it now. I will roll it out if I need more time, Ossy. Thanks!

(In reply to comment #28) > Reopen, because it broke API tests: > > Could you check and fix it please? So, Ossy, I was debugging it with Alexis and we realized that the problem is happening because we run the tests in parallel. If you run "run-qtwebkit-tests -j1" you will see that the tests doesn't fail. Our belief is that running tests in parallel that depend on their window state is a potential mistake, which has now been flagged because this is exactly what this patch fixes. (In reply to comment #29) > Additionally it broke a layout test: This I can't reproduce this locally. =/

(In reply to comment #32) > So, Ossy, I was debugging it with Alexis and we realized that the problem is happening because we run the tests in parallel. If you run "run-qtwebkit-tests -j1" you will see that the tests doesn't fail. > > Our belief is that running tests in parallel that depend on their window state is a potential mistake, which has now been flagged because this is exactly what this patch fixes. I tried it locally, but I got same results as the bot results. Did you run the tests in XFVB as the bots do? xvfb-run -a --server-args="-screen 0 1024x768x24" python ./Tools/Scripts/run-qtwebkit-tests --output-file=qt-unit-tests.html --do-not-open-results --timeout=120 WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/ I got same results with -j1 and without -j1 ... > (In reply to comment #29) > > Additionally it broke a layout test: > > This I can't reproduce this locally. =/ It's hard to believe. :( Did you run all tests with Tools/Scripts/run-webkit-tests on the reference platform? (Ubuntu 11.10) If you do it, fast/events/mouseout-on-window fails and there are 24 new flakey tests after your patch.

(In reply to comment #33) > I tried it locally, but I got same results as the bot results. > Did you run the tests in XFVB as the bots do? Nope. > > > (In reply to comment #29) > > > Additionally it broke a layout test: > > > > This I can't reproduce this locally. =/ > > It's hard to believe. :( Did you run all tests with Tools/Scripts/run-webkit-tests on the reference platform? (Ubuntu 11.10) > If you do it, fast/events/mouseout-on-window fails and there are > 24 new flakey tests after your patch. Hard to believe?! It was tested on two different machines (api tests with -j1 and this layout test specifically) but none of them use Ubuntu. Plus, a fast/event test that is called "mouseout-on-window" shouldn't have any platform-specific issues, right? Anyway, I'm having a deeper look in order to find a workaround for it somehow.

(In reply to comment #34) > It was tested on two different machines (api tests with -j1 and this layout test specifically) but none of them use Ubuntu. Plus, a fast/event test that is called "mouseout-on-window" shouldn't have any platform-specific issues, right? By "platform-specific" here I meant "distro-specific".

Here is the new flakey tests after this patch: ----------------------------------------------- editing/pasteboard/copy-crash.html = TEXT PASS fast/block/positioning/absolute-appended-to-inline.html = TEXT PASS fast/css/hover-affects-child.html = TEXT PASS fast/css/pseudo-any.html = TEXT PASS fast/dom/shadow/shadow-boundary-crossing.html = TEXT PASS fast/dom/shadow/shadow-contents-event.html = TEXT PASS fast/events/dont-loose-last-event.html = TEXT PASS fast/events/event-sender-mouse-moved.html = TEXT PASS fast/events/iframe-onmousemove.html = TEXT PASS fast/events/mouse-moved-remove-frame-crash.html = TEXT PASS fast/events/mousemove-after-drag-over-scrollbar.html = TEXT PASS fast/events/mouseout-dead-node.html = TEXT PASS fast/events/mouseout-dead-subframe.html = TEXT PASS fast/events/mouseover-mouseout.html = TEXT PASS fast/events/mouseover-mouseout2.html = TEXT PASS fast/events/touch/touch-before-pressing-spin-button.html = TEXT PASS fast/forms/autofocus-input-css-style-change.html = TEXT PASS fast/forms/input-step-as-double.html = TEXT PASS fast/forms/number/spin-button-events.html = TEXT PASS fast/forms/number/spin-button-state.html = TEXT PASS fast/forms/number/spin-in-multi-column.html = TEXT PASS fast/forms/range/slider-mouse-events.html = TEXT PASS fast/frames/flattening/iframe-flattening-fixed-width.html = TEXT PASS fast/overflow/overflow-focus-ring.html = TEXT PASS fast/replaced/image-map-2.html = TEXT PASS scrollbars/overflow-custom-scrollbar-crash.html = TEXT PASS scrollbars/scrollbar-iframe-click-does-not-blur-content.html = TEXT PASS svg/custom/foreign-object-skew.svg = TEXT PASS svg/custom/js-update-container2.svg = TEXT PASS svg/custom/pointer-events-invalid-fill.svg = TEXT PASS svg/custom/use-instanceRoot-as-event-target.xhtml = TEXT PASS svg/zoom/page/zoom-foreignObject.svg = TEXT PASS svg/zoom/text/zoom-foreignObject.svg = TEXT PASS And the new failing test: -------------------------- fast/css/hover-update.html = TEXT fast/css/nested-layers-with-hover.html = TEXT fast/events/mouseout-on-window.html = TEXT

(In reply to comment #34) > > > This I can't reproduce this locally. =/ > > > > It's hard to believe. :( Did you run all tests with Tools/Scripts/run-webkit-tests on the reference platform? (Ubuntu 11.10) > > If you do it, fast/events/mouseout-on-window fails and there are > > 24 new flakey tests after your patch. > > Hard to believe?! > It was tested on two different machines (api tests with -j1 and this layout test specifically) but none of them use Ubuntu. Plus, a fast/event test that is called "mouseout-on-window" shouldn't have any platform-specific issues, right? I think it is the main problem. All developers should run tests on the _reference_ platform. And at least you should have at least one Ubuntu to be able reproduce QtWebKit bugs occured on the buildbots. > Anyway, I'm having a deeper look in order to find a workaround for it somehow. Thanks for taking care. To be clarify I won't roll out QtWebKit patches, obviously buggy patches, I don't wan't to be the grumpy bad guy who blocks the rapid developing. But in this case our Qt5-WK2 bots aren't in a good shape. And it's hard to catch new flakey and new failing tests until it is fixed. :-/ But if it's OK for you guys let's leave the bot in this bad shape.

I modified our master.cfg to run API tests with -j1. The runtime was increased from 0.5 minutes to 2.5 minutes, but tests still fail. It's strange, because it worked for me locally in the same environment. But layout tests might break something in the environment and it might break API tests ... I have no idea what happens ...

(In reply to comment #38) > I modified our master.cfg to run API tests with -j1. The runtime was > increased from 0.5 minutes to 2.5 minutes, but tests still fail. > It's strange, because it worked for me locally in the same environment. > But layout tests might break something in the environment and it might > break API tests ... I have no idea what happens ... I will be investigating the issues today and I will update the bug with new comments. Thanks!

(In reply to comment #39) > I will be investigating the issues today and I will update the bug with new comments. An issue with my xvfb version and nvidia driver is preventing to run this as the bots do. I will need a bit more time to investigate and fix this. =/

(In reply to comment #38) > I modified our master.cfg to run API tests with -j1. The runtime was > increased from 0.5 minutes to 2.5 minutes, but tests still fail. > It's strange, because it worked for me locally in the same environment. > But layout tests might break something in the environment and it might > break API tests ... I have no idea what happens ... After -j1 we got very strange results on the 32 bit Qt5-WK2 bot: 31 passed, 3 failed, 0 skipped, 3 crashed http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20WebKit2/builds/22770/steps/API%20tests/logs/stdio CRASHED: WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/qmltests/tst_qmltests_WebView CRASHED: WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview CRASHED: WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/publicapi/tst_publicapi Great! :(

(In reply to comment #41) > Great! :( So remove the -j1 for the time being. But don't you think that not running the tests in parallel shouldn't be causing crashes?!! As I said, Xvfb is not working properly with my nvidia card on arch linux. I'm fixing this and setting up a virtual machine with the "reference platform" so I can debug the issues. Before that there is nothing I can do. If the patch is causing too much trouble on the bots, just roll it out as well and it will come back later when we all have understand what is going on with it. I would do it myself if I had X running right now. (and I would try -j1 without this patch just to see how the bots would behave as well) Other than that, if you really want to help, try to understand what we have explained on irc and on this bug report and help us to debug it as well. There is something way too wrong and not only with the patch if even you said that you are getting different behavior on the bots and on your machine, which is running the "reference platform". thanks!

(In reply to comment #42) > (In reply to comment #41) > > Great! :( > > So remove the -j1 for the time being. But don't you think that not running the tests in parallel shouldn't be causing crashes?!! I removed -j1 and now the runtime is 27 sec again and there isn't any crashes. Very strange. > As I said, Xvfb is not working properly with my nvidia card on arch linux. I'm fixing this and setting up a virtual machine with the "reference platform" so I can debug the issues. Before that there is nothing I can do. If the patch is causing too much trouble on the bots, just roll it out as well and it will come back later when we all have understand what is going on with it. I would do it myself if I had X running right now. (and I would try -j1 without this patch just to see how the bots would behave as well) > > Other than that, if you really want to help, try to understand what we have explained on irc and on this bug report and help us to debug it as well. There is something way too wrong and not only with the patch if even you said that you are getting different behavior on the bots and on your machine, which is running the "reference platform". I'm terrible sorry, but I don't have too much free time for debugging when I'm filing bugs about new regressions, rebase expected files instead of authors, digging which patch broke what in 24/7. :( I agree, something is really wrong with run-qtwebkit-tests script near -j1 option ... I'll check it soon with and without your patch.

=== Bulk closing of Qt bugs === If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary. If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.