WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED INVALID
81143
[Qt][WK2] Implement PageClient::isViewWindowActive()
https://bugs.webkit.org/show_bug.cgi?id=81143
Summary
[Qt][WK2] Implement PageClient::isViewWindowActive()
Jesus Sanchez-Palencia
Reported
2012-03-14 12:34:00 PDT
Since now QQuickCanvas provides API for telling us whether it is active or not, we can implement PageClient::isViewWindowActive.
Attachments
Patch
(4.01 KB, patch)
2012-03-14 15:41 PDT
,
Jesus Sanchez-Palencia
no flags
Details
Formatted Diff
Diff
Patch
(1.67 KB, patch)
2012-03-15 11:18 PDT
,
Jesus Sanchez-Palencia
no flags
Details
Formatted Diff
Diff
Patch
(1.71 KB, patch)
2012-03-20 14:53 PDT
,
Jesus Sanchez-Palencia
no flags
Details
Formatted Diff
Diff
Patch
(3.39 KB, patch)
2012-03-23 16:03 PDT
,
Jesus Sanchez-Palencia
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Jesus Sanchez-Palencia
Comment 1
2012-03-14 15:41:53 PDT
Created
attachment 131942
[details]
Patch
Jesus Sanchez-Palencia
Comment 2
2012-03-14 15:45:29 PDT
(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.
Kenneth Rohde Christiansen
Comment 3
2012-03-15 02:55:09 PDT
Comment on
attachment 131942
[details]
Patch why no url to the Qt bug?
Jesus Sanchez-Palencia
Comment 4
2012-03-15 06:28:55 PDT
(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 :)
Kenneth Rohde Christiansen
Comment 5
2012-03-15 06:38:53 PDT
In the code with the FIXME
Jesus Sanchez-Palencia
Comment 6
2012-03-15 06:45:03 PDT
(In reply to
comment #5
)
> In the code with the FIXME
I can fix when landing. Anything else?
Jesus Sanchez-Palencia
Comment 7
2012-03-15 10:43:51 PDT
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... :)
Jesus Sanchez-Palencia
Comment 8
2012-03-15 11:18:15 PDT
Created
attachment 132081
[details]
Patch
Simon Hausmann
Comment 9
2012-03-15 13:38:22 PDT
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?
Jesus Sanchez-Palencia
Comment 10
2012-03-15 14:04:12 PDT
(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).
Jesus Sanchez-Palencia
Comment 11
2012-03-15 14:05:15 PDT
(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
Jesus Sanchez-Palencia
Comment 12
2012-03-20 14:53:12 PDT
Created
attachment 132903
[details]
Patch
Jesus Sanchez-Palencia
Comment 13
2012-03-20 14:55:46 PDT
(In reply to
comment #12
)
> Created an attachment (id=132903) [details] > Patch
Updated the patch due to a check that was missing.
WebKit Review Bot
Comment 14
2012-03-23 06:27:22 PDT
Comment on
attachment 132903
[details]
Patch Clearing flags on attachment: 132903 Committed
r111855
: <
http://trac.webkit.org/changeset/111855
>
WebKit Review Bot
Comment 15
2012-03-23 06:27:28 PDT
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 16
2012-03-23 07:10:32 PDT
Reopen, because it broke 30+ tests -
http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r111856%20%2821981%29/results.html
Jesus Sanchez-Palencia
Comment 17
2012-03-23 14:10:57 PDT
(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.
Jesus Sanchez-Palencia
Comment 18
2012-03-23 16:03:05 PDT
Created
attachment 133579
[details]
Patch
Jesus Sanchez-Palencia
Comment 19
2012-03-24 08:17:30 PDT
Comment on
attachment 133579
[details]
Patch Let's give it a try...
WebKit Review Bot
Comment 20
2012-03-24 08:24:10 PDT
Comment on
attachment 133579
[details]
Patch Clearing flags on attachment: 133579 Committed
r111993
: <
http://trac.webkit.org/changeset/111993
>
WebKit Review Bot
Comment 21
2012-03-24 08:24:16 PDT
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 22
2012-03-25 22:23:35 PDT
Reopen, because it broke many tests, see
https://bugs.webkit.org/show_bug.cgi?id=82167
for details.
Csaba Osztrogonác
Comment 23
2012-03-26 04:08:14 PDT
Rollout landed in
http://trac.webkit.org/changeset/112073
(rs=darktears)
Jesus Sanchez-Palencia
Comment 24
2012-03-26 05:54:27 PDT
Comment on
attachment 133579
[details]
Patch We need to wait the next round of Qt5 updates on the bots before landing this.
Kenneth Rohde Christiansen
Comment 25
2012-04-10 07:03:14 PDT
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.
Jesus Sanchez-Palencia
Comment 26
2012-04-10 07:07:32 PDT
Comment on
attachment 133579
[details]
Patch CQ has issues, I will land it manually.
Jesus Sanchez-Palencia
Comment 27
2012-04-10 08:15:40 PDT
Committed
r113714
: <
http://trac.webkit.org/changeset/113714
>
Csaba Osztrogonác
Comment 28
2012-04-10 08:55:12 PDT
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?
Csaba Osztrogonác
Comment 29
2012-04-10 09:05:22 PDT
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.
Csaba Osztrogonác
Comment 30
2012-04-10 09:18:39 PDT
And it made 24 new flakey test :(
Jesus Sanchez-Palencia
Comment 31
2012-04-10 10:39:38 PDT
Wow! Ok, I will have a look at it now. I will roll it out if I need more time, Ossy. Thanks!
Jesus Sanchez-Palencia
Comment 32
2012-04-10 12:13:28 PDT
(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. =/
Csaba Osztrogonác
Comment 33
2012-04-10 12:55:48 PDT
(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.
Jesus Sanchez-Palencia
Comment 34
2012-04-10 13:43:17 PDT
(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.
Jesus Sanchez-Palencia
Comment 35
2012-04-10 14:11:51 PDT
(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".
Csaba Osztrogonác
Comment 36
2012-04-11 08:54:04 PDT
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
Csaba Osztrogonác
Comment 37
2012-04-11 09:00:05 PDT
(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.
Csaba Osztrogonác
Comment 38
2012-04-11 09:02:45 PDT
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 ...
Jesus Sanchez-Palencia
Comment 39
2012-04-11 10:35:36 PDT
(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!
Jesus Sanchez-Palencia
Comment 40
2012-04-11 16:10:27 PDT
(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. =/
Csaba Osztrogonác
Comment 41
2012-04-12 06:47:16 PDT
(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! :(
Jesus Sanchez-Palencia
Comment 42
2012-04-12 07:05:46 PDT
(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!
Csaba Osztrogonác
Comment 43
2012-04-12 08:30:27 PDT
(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.
Jocelyn Turcotte
Comment 44
2014-02-03 03:20:22 PST
=== 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.
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