The unit test for snapshots fails on the expected size of the snapshot. ERROR:../../Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:553:void testWebViewSnapshot(SnapshotWebViewTest*, gconstpointer): assertion failed (width <= 50): (200 <= 50)
The problem is that gtk_widget_size_allocate() no longer resizes the widget in WebViewTest::resizeView(). Its not clear to me what the right function is, or if its even possible to resize a widget that's not a window. Options now are 1) find a function that resizes a widget 2) disable the test 3) rewrite the test to avoid resizing (although Claudio felt that this was necessary to test the functionality) What is the best course of action here?
You can try resizing the window instead of the view.
GTK's DumpRenderTree works around the resizing issues by spinning the main loop while the events are pending, simply because resizing is done asynchronously. http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/gtk/DumpRenderTree.cpp#L622 Could you give that a try?
(In reply to comment #3) > GTK's DumpRenderTree works around the resizing issues by spinning the main loop while the events are pending, simply because resizing is done asynchronously. > http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/gtk/DumpRenderTree.cpp#L622 > > Could you give that a try? That probably needs to be added to the resizeView() method (or whatever is called) in WebViewTest.
(In reply to comment #3) > GTK's DumpRenderTree works around the resizing issues by spinning the main loop while the events are pending, simply because resizing is done asynchronously. > http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/gtk/DumpRenderTree.cpp#L622 > > Could you give that a try? Just tried that -> it successfully pumps all the events but the webview doesn't get resized.
(In reply to comment #2) > You can try resizing the window instead of the view. That solves it, but only when used with the event pump! Thanks everyone for the help, patch coming soon...
Created attachment 210125 [details] Patch
Comment on attachment 210125 [details] Patch Rejecting attachment 210125 [details] from commit-queue. brian.holt@samsung.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
(In reply to comment #8) > (From update of attachment 210125 [details]) > Rejecting attachment 210125 [details] from commit-queue. > > brian.holt@samsung.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. > > - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. > > - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights. Sorry, that was a mistake -> wanted to set the cq? flag.
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 210125 [details] Patch While I’m not a GTK expert, I feel I know enough to OK this, especially given that it’s in the test code.
If we don't understand what exactly changed in WK that requires this method to be changed, it seems to me that this is more a workaround than an actual fix.
Comment on attachment 210125 [details] Patch Clearing flags on attachment: 210125 Committed r154899: <http://trac.webkit.org/changeset/154899>
All reviewed patches have been landed. Closing bug.
Comment on attachment 210125 [details] Patch Sorry to be late here (again), but this is not correct. WebViewTest::resizeView() might be used by tests not using a window, so we can't assume m_parentWindow is non-NULL pointer. This also broke the inspector unit tests here that also use resizeView, but I don't know exactly why, because inspector test uses a window.
(In reply to comment #15) > (From update of attachment 210125 [details]) > Sorry to be late here (again), but this is not correct. WebViewTest::resizeView() might be used by tests not using a window, so we can't assume m_parentWindow is non-NULL pointer. This also broke the inspector unit tests here that also use resizeView, but I don't know exactly why, because inspector test uses a window. After discussion with Carlos on IRC it seems we should roll this out. It looks like the patch has exactly the same behaviour as the previous code the snapshot test fails first time under xvfb for both old and new code, but passes on subsequent runs, whereas the test passes every time under X for both old and new. So its an Xvfb issue and this patch doesn't resolve it.
Re-opened since this is blocked by bug 121162
There are at least problems here: 1.- Resizing the web view works, but since we are using a size smaller than the window size, the web view is then resized again to fill the parent container. That's why we need to resize the window instead. 2.- When the WebView is resized, a message is sent to the WebProcess, but since we use injected bundle message to implement snapshots, it can happen that the GetSnapshot message is received before the resize one, and we create the snapshot with the previous size. We should make sure the web view is mapped in the window at the desired size. I also think we should use fixed values for the document size and visible area, setting the doc size using CSS and disabling the scrollbars in CSS as well. I'll rework the test, it has been skipped for too long
Created attachment 244894 [details] Patch
Committed r178644: <http://trac.webkit.org/changeset/178644>