179366 2017-11-07 00:35:59 -0800 REGRESSION(r224179): [GTK] Several WebViewEditor tests are failing since r224179 2017-11-07 23:25:49 -0800 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED Gtk, Regression P2 Normal --- 1 cgarcia webkit-unassigned berto bugs-noreply buildbot gustavo mcatanzaro oldest_to_newest 1368950 0 cgarcia 2017-11-07 00:35:59 -0800 Unexpected failures (9) /WebKit2Gtk/TestWebsiteData /webkit2/WebKitWebsiteData/cache /webkit2/WebKitWebsiteData/databases /webkit2/WebKitWebsiteData/resource-load-statistics /webkit2/WebKitWebsiteData/configuration /WebKit2Gtk/TestInspector /webkit2/WebKitWebInspector/default /webkit2/WebKitWebInspector/manual-attach-detach /WebKit2Gtk/TestWebViewEditor /webkit2/WebKitWebView/cut-copy-paste/non-editable /webkit2/WebKitWebView/editable/editable /webkit2/WebKitWebView/cut-copy-paste/editable Unexpected timeouts (3) /WebCore/TestWebCore FileMonitorTest.DetectMultipleChanges FileMonitorTest.DetectChangeAndThenDelete /WebKit2Gtk/TestWebViewEditor /webkit2/WebKitWebView/editor-state/typing-attributes The TestWebViewEditor ones were introduced in r224179 1368953 1 326200 cgarcia 2017-11-07 01:28:18 -0800 Created attachment 326200 Patch 1368954 2 buildbot 2017-11-07 01:29:30 -0800 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 1368984 3 326200 mcatanzaro 2017-11-07 06:37:54 -0800 Comment on attachment 326200 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326200&action=review Hm. Problem #1 is I failed to run all the API tests. I only ran the TestWebViewEditor tests. But I knew this change would break other API tests unless I added calls to flush the editor state. I just forgot. Sorry. Problem #2 is I did not test under X11. It looks like the TestWebViewEditor tests are all currently passing on the Wayland bot. That's a bit surprising, but I guess the editor state flush works differently in Wayland as it is tightly coupled with the drawing code. My original flushEditorState() worked quite reliably for me, but I had tons of failures when I tried to use showInWindowAndWaitUntilMapped, so let's check the Wayland bot after this lands to make sure it's OK. Thanks for cleaning up after me. > Source/WebKit/ChangeLog:11 > + In r224179, webkit_web_view_can_execute_editing_command() was optimized to use the sync path for commands > + supported by the WebViewEditorState, but the state requires a redraw to be up to date. We can't know if > + WebViewEditorState is in sync, when webkit_web_view_can_execute_editing_command() is called, so we always need > + to ask the web process. It should work if we add more calls to flushEditorState() in all the API tests that require it. So it really should not be necessary to remove this. But I'm more comfortable with removing it, so OK. I only added it in the first place because you requested it. > Tools/TestWebKitAPI/Tests/WebKitGtk/TestWebViewEditor.cpp:44 > - return G_SOURCE_REMOVE; > + return FALSE; Oops. > Tools/TestWebKitAPI/Tests/WebKitGtk/TestWebViewEditor.cpp:52 > + g_signal_handler_disconnect(m_webView, signalID); Ooooooooops. 1369010 4 cgarcia 2017-11-07 08:19:22 -0800 (In reply to Michael Catanzaro from comment #3) > Comment on attachment 326200 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=326200&action=review > > Hm. Problem #1 is I failed to run all the API tests. I only ran the > TestWebViewEditor tests. But I knew this change would break other API tests > unless I added calls to flush the editor state. I just forgot. Sorry. I pasted the current bot results, but this patch only fixes TestWebViewEditor, not the other ones, as I said in bug description. > Problem #2 is I did not test under X11. It looks like the TestWebViewEditor > tests are all currently passing on the Wayland bot. That's a bit surprising, > but I guess the editor state flush works differently in Wayland as it is > tightly coupled with the drawing code. My original flushEditorState() worked > quite reliably for me, but I had tons of failures when I tried to use > showInWindowAndWaitUntilMapped, so let's check the Wayland bot after this > lands to make sure it's OK. It worked with native xorg, wayland and weston, but always failed with xvfb, with this patch it passes in all the cases. > Thanks for cleaning up after me. > > > Source/WebKit/ChangeLog:11 > > + In r224179, webkit_web_view_can_execute_editing_command() was optimized to use the sync path for commands > > + supported by the WebViewEditorState, but the state requires a redraw to be up to date. We can't know if > > + WebViewEditorState is in sync, when webkit_web_view_can_execute_editing_command() is called, so we always need > > + to ask the web process. > > It should work if we add more calls to flushEditorState() in all the API > tests that require it. So it really should not be necessary to remove this. No, it's TestWebViewEditor what is failing under xvfb. > But I'm more comfortable with removing it, so OK. I only added it in the > first place because you requested it. > > > Tools/TestWebKitAPI/Tests/WebKitGtk/TestWebViewEditor.cpp:44 > > - return G_SOURCE_REMOVE; > > + return FALSE; > > Oops. > > > Tools/TestWebKitAPI/Tests/WebKitGtk/TestWebViewEditor.cpp:52 > > + g_signal_handler_disconnect(m_webView, signalID); > > Ooooooooops. 1369351 5 cgarcia 2017-11-07 23:25:49 -0800 Committed r224567: <https://trac.webkit.org/changeset/224567> 326200 2017-11-07 01:28:18 -0800 2017-11-07 06:37:54 -0800 Patch wk2-editor-tests.diff text/plain 5379 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9DaGFu Z2VMb2cKaW5kZXggNzE0ZjE2MzY5MWMuLjUzYzY5NTA2NGYwIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0L0NoYW5nZUxvZwpAQCAtMSwzICsx LDE4IEBACisyMDE3LTExLTA3ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlh LmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OKHIyMjQxNzkpOiBbR1RLXSBTZXZlcmFsIFdlYlZp ZXdFZGl0b3IgdGVzdHMgYXJlIGZhaWxpbmcgc2luY2UgcjIyNDE3OQorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc5MzY2CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW4gcjIyNDE3OSwgd2Via2l0X3dlYl92 aWV3X2Nhbl9leGVjdXRlX2VkaXRpbmdfY29tbWFuZCgpIHdhcyBvcHRpbWl6ZWQgdG8gdXNlIHRo ZSBzeW5jIHBhdGggZm9yIGNvbW1hbmRzCisgICAgICAgIHN1cHBvcnRlZCBieSB0aGUgV2ViVmll d0VkaXRvclN0YXRlLCBidXQgdGhlIHN0YXRlIHJlcXVpcmVzIGEgcmVkcmF3IHRvIGJlIHVwIHRv IGRhdGUuIFdlIGNhbid0IGtub3cgaWYKKyAgICAgICAgV2ViVmlld0VkaXRvclN0YXRlIGlzIGlu IHN5bmMsIHdoZW4gd2Via2l0X3dlYl92aWV3X2Nhbl9leGVjdXRlX2VkaXRpbmdfY29tbWFuZCgp IGlzIGNhbGxlZCwgc28gd2UgYWx3YXlzIG5lZWQKKyAgICAgICAgdG8gYXNrIHRoZSB3ZWIgcHJv Y2Vzcy4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvZ2xpYi9XZWJLaXRXZWJWaWV3LmNwcDoK KyAgICAgICAgKHdlYmtpdF93ZWJfdmlld19jYW5fZXhlY3V0ZV9lZGl0aW5nX2NvbW1hbmQpOgor CiAyMDE3LTExLTA3ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4K IAogICAgICAgICBSRUdSRVNTSU9OKHIyMjM3NzMpOiBbR1RLXSBXZWJLaXRXZWJJbnNwZWN0b3Ig YnJpbmctdG8tZnJvbnQgc2lnbmFsIGlzIGVtaXR0ZWQgcmlnaHQgYWZ0ZXIgb3Blbi13aW5kb3cK ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0FQSS9nbGliL1dlYktpdFdlYlZp ZXcuY3BwIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL2dsaWIvV2ViS2l0V2ViVmlldy5j cHAKaW5kZXggNzliOTBkMDIwZDguLmM4Y2I3OGVlY2ZiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi S2l0L1VJUHJvY2Vzcy9BUEkvZ2xpYi9XZWJLaXRXZWJWaWV3LmNwcAorKysgYi9Tb3VyY2UvV2Vi S2l0L1VJUHJvY2Vzcy9BUEkvZ2xpYi9XZWJLaXRXZWJWaWV3LmNwcApAQCAtMzA4NywyNCArMzA4 NywxMCBAQCB2b2lkIHdlYmtpdF93ZWJfdmlld19jYW5fZXhlY3V0ZV9lZGl0aW5nX2NvbW1hbmQo V2ViS2l0V2ViVmlldyogd2ViVmlldywgY29uc3QgYwogICAgIGdfcmV0dXJuX2lmX2ZhaWwoV0VC S0lUX0lTX1dFQl9WSUVXKHdlYlZpZXcpKTsKICAgICBnX3JldHVybl9pZl9mYWlsKGNvbW1hbmQp OwogCi0gICAgR1Rhc2sqIHRhc2sgPSBnX3Rhc2tfbmV3KHdlYlZpZXcsIGNhbmNlbGxhYmxlLCBj YWxsYmFjaywgdXNlckRhdGEpOwotICAgIFdlYktpdEVkaXRvclN0YXRlKiBzdGF0ZSA9IHdlYmtp dF93ZWJfdmlld19nZXRfZWRpdG9yX3N0YXRlKHdlYlZpZXcpOwotCi0gICAgaWYgKCFzdHJjbXAo Y29tbWFuZCwgV0VCS0lUX0VESVRJTkdfQ09NTUFORF9DVVQpKQotICAgICAgICBnX3Rhc2tfcmV0 dXJuX2Jvb2xlYW4oYWRvcHRHUmVmKHRhc2spLmdldCgpLCB3ZWJraXRfZWRpdG9yX3N0YXRlX2lz X2N1dF9hdmFpbGFibGUoc3RhdGUpKTsKLSAgICBlbHNlIGlmICghc3RyY21wKGNvbW1hbmQsIFdF QktJVF9FRElUSU5HX0NPTU1BTkRfQ09QWSkpCi0gICAgICAgIGdfdGFza19yZXR1cm5fYm9vbGVh bihhZG9wdEdSZWYodGFzaykuZ2V0KCksIHdlYmtpdF9lZGl0b3Jfc3RhdGVfaXNfY29weV9hdmFp bGFibGUoc3RhdGUpKTsKLSAgICBlbHNlIGlmICghc3RyY21wKGNvbW1hbmQsIFdFQktJVF9FRElU SU5HX0NPTU1BTkRfUEFTVEUpKQotICAgICAgICBnX3Rhc2tfcmV0dXJuX2Jvb2xlYW4oYWRvcHRH UmVmKHRhc2spLmdldCgpLCB3ZWJraXRfZWRpdG9yX3N0YXRlX2lzX3Bhc3RlX2F2YWlsYWJsZShz dGF0ZSkpOwotICAgIGVsc2UgaWYgKCFzdHJjbXAoY29tbWFuZCwgV0VCS0lUX0VESVRJTkdfQ09N TUFORF9VTkRPKSkKLSAgICAgICAgZ190YXNrX3JldHVybl9ib29sZWFuKGFkb3B0R1JlZih0YXNr KS5nZXQoKSwgd2Via2l0X2VkaXRvcl9zdGF0ZV9pc191bmRvX2F2YWlsYWJsZShzdGF0ZSkpOwot ICAgIGVsc2UgaWYgKCFzdHJjbXAoY29tbWFuZCwgV0VCS0lUX0VESVRJTkdfQ09NTUFORF9SRURP KSkKLSAgICAgICAgZ190YXNrX3JldHVybl9ib29sZWFuKGFkb3B0R1JlZih0YXNrKS5nZXQoKSwg d2Via2l0X2VkaXRvcl9zdGF0ZV9pc19yZWRvX2F2YWlsYWJsZShzdGF0ZSkpOwotICAgIGVsc2Ug ewotICAgICAgICBnZXRQYWdlKHdlYlZpZXcpLnZhbGlkYXRlQ29tbWFuZChTdHJpbmc6OmZyb21V VEY4KGNvbW1hbmQpLCBbdGFza10oY29uc3QgU3RyaW5nJiwgYm9vbCBpc0VuYWJsZWQsIGludDMy X3QsIFdlYktpdDo6Q2FsbGJhY2tCYXNlOjpFcnJvcikgewotICAgICAgICAgICAgZ190YXNrX3Jl dHVybl9ib29sZWFuKGFkb3B0R1JlZih0YXNrKS5nZXQoKSwgaXNFbmFibGVkKTsgICAgICAgIAot ICAgICAgICB9KTsKLSAgICB9CisgICAgR1JlZlB0cjxHVGFzaz4gdGFzayA9IGFkb3B0R1JlZihn X3Rhc2tfbmV3KHdlYlZpZXcsIGNhbmNlbGxhYmxlLCBjYWxsYmFjaywgdXNlckRhdGEpKTsKKyAg ICBnZXRQYWdlKHdlYlZpZXcpLnZhbGlkYXRlQ29tbWFuZChTdHJpbmc6OmZyb21VVEY4KGNvbW1h bmQpLCBbdGFzayA9IFdURk1vdmUodGFzayldKGNvbnN0IFN0cmluZyYsIGJvb2wgaXNFbmFibGVk LCBpbnQzMl90LCBXZWJLaXQ6OkNhbGxiYWNrQmFzZTo6RXJyb3IpIHsKKyAgICAgICAgZ190YXNr X3JldHVybl9ib29sZWFuKHRhc2suZ2V0KCksIGlzRW5hYmxlZCk7CisgICAgfSk7CiB9CiAKIC8q KgpkaWZmIC0tZ2l0IGEvVG9vbHMvQ2hhbmdlTG9nIGIvVG9vbHMvQ2hhbmdlTG9nCmluZGV4IDE3 YzE2NjNlYzhkLi4zZjE3NjUxYmEwMSAxMDA2NDQKLS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBi L1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE3LTExLTA3ICBDYXJsb3MgR2Fy Y2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OKHIy MjQxNzkpOiBbR1RLXSBTZXZlcmFsIFdlYlZpZXdFZGl0b3IgdGVzdHMgYXJlIGZhaWxpbmcgc2lu Y2UgcjIyNDE3OQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTc5MzY2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgVXNlIGFsd2F5cyBhIHRvcGxldmVsIHdpbmRvdyBmb3IgV2ViVmlld0VkaXRvciB0ZXN0cyBp bnN0ZWFkIG9mIGNyZWF0aW5nIGFuZCBkZXN0cm95aW5nIG9uZSBvbmx5IHRvIGZsdXNoIGVkaXRv cgorICAgICAgICBzdGF0ZS4KKworICAgICAgICAqIFRlc3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0 R3RrL1Rlc3RXZWJWaWV3RWRpdG9yLmNwcDoKKwogMjAxNy0xMS0wNiAgV2Vuc29uIEhzaWVoICA8 d2Vuc29uX2hzaWVoQGFwcGxlLmNvbT4KIAogICAgICAgICBSZW1vdmUgc3RyYXkgbG9nZ2luZyBm cm9tIGEgbmV3bHkgYWRkZWQgQVBJIHRlc3QKZGlmZiAtLWdpdCBhL1Rvb2xzL1Rlc3RXZWJLaXRB UEkvVGVzdHMvV2ViS2l0R3RrL1Rlc3RXZWJWaWV3RWRpdG9yLmNwcCBiL1Rvb2xzL1Rlc3RXZWJL aXRBUEkvVGVzdHMvV2ViS2l0R3RrL1Rlc3RXZWJWaWV3RWRpdG9yLmNwcAppbmRleCA4NjJhMjRk N2QwNC4uODg3MTE5NTFkZjAgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMv V2ViS2l0R3RrL1Rlc3RXZWJWaWV3RWRpdG9yLmNwcAorKysgYi9Ub29scy9UZXN0V2ViS2l0QVBJ L1Rlc3RzL1dlYktpdEd0ay9UZXN0V2ViVmlld0VkaXRvci5jcHAKQEAgLTM0LDI4ICszNCwyMiBA QCBwdWJsaWM6CiAgICAgICAgICwgbV90cmllc0NvdW50KDApCiAgICAgICAgICwgbV9lZGl0b3JT dGF0ZShudWxscHRyKQogICAgIHsKKyAgICAgICAgc2hvd0luV2luZG93QW5kV2FpdFVudGlsTWFw cGVkKEdUS19XSU5ET1dfVE9QTEVWRUwpOwogICAgICAgICBndGtfY2xpcGJvYXJkX2NsZWFyKG1f Y2xpcGJvYXJkKTsKICAgICB9CiAKICAgICBzdGF0aWMgZ2Jvb2xlYW4gd2ViVmlld0RyYXdDYWxs YmFjayhHTWFpbkxvb3AqIG1haW5Mb29wKQogICAgIHsKICAgICAgICAgZ19tYWluX2xvb3BfcXVp dChtYWluTG9vcCk7Ci0gICAgICAgIHJldHVybiBHX1NPVVJDRV9SRU1PVkU7CisgICAgICAgIHJl dHVybiBGQUxTRTsKICAgICB9CiAKICAgICB2b2lkIGZsdXNoRWRpdG9yU3RhdGUoKQogICAgIHsK LSAgICAgICAgLy8gRklYTUU6IEl0IHdvdWxkIGJlIGJldHRlciB0byBjYWxsIFdlYlZpZXdUZXN0 OjpzaG93SW5XaW5kb3dBbmRXYWl0VW50aWxNYXBwZWQKLSAgICAgICAgLy8gYXQgdGhlIHN0YXJ0 IG9mIHRoZSB0ZXN0LCByYXRoZXIgdGhhbiBjcmVhdGluZyBhbmQgZGVzdHJveWluZyB0ZW1wb3Jh cnkgd2luZG93cy4KLSAgICAgICAgc2hvd0luV2luZG93KEdUS19XSU5ET1dfVE9QTEVWRUwpOwot Ci0gICAgICAgIGdfc2lnbmFsX2Nvbm5lY3Rfc3dhcHBlZChtX3dlYlZpZXcsICJkcmF3IiwgR19D QUxMQkFDSyh3ZWJWaWV3RHJhd0NhbGxiYWNrKSwgbV9tYWluTG9vcCk7CisgICAgICAgIGF1dG8g c2lnbmFsSUQgPSBnX3NpZ25hbF9jb25uZWN0X3N3YXBwZWQobV93ZWJWaWV3LCAiZHJhdyIsIEdf Q0FMTEJBQ0sod2ViVmlld0RyYXdDYWxsYmFjayksIG1fbWFpbkxvb3ApOwogICAgICAgICBndGtf d2lkZ2V0X3F1ZXVlX2RyYXcoR1RLX1dJREdFVChtX3dlYlZpZXcpKTsKICAgICAgICAgZ19tYWlu X2xvb3BfcnVuKG1fbWFpbkxvb3ApOwotCi0gICAgICAgIGd0a19jb250YWluZXJfcmVtb3ZlKEdU S19DT05UQUlORVIobV9wYXJlbnRXaW5kb3cpLCBHVEtfV0lER0VUKG1fd2ViVmlldykpOwotICAg ICAgICBndGtfd2lkZ2V0X2Rlc3Ryb3kobV9wYXJlbnRXaW5kb3cpOwotICAgICAgICBtX3BhcmVu dFdpbmRvdyA9IG51bGxwdHI7CisgICAgICAgIGdfc2lnbmFsX2hhbmRsZXJfZGlzY29ubmVjdCht X3dlYlZpZXcsIHNpZ25hbElEKTsKICAgICB9CiAKICAgICBzdGF0aWMgdm9pZCBjYW5FeGVjdXRl RWRpdGluZ0NvbW1hbmRSZWFkeUNhbGxiYWNrKEdPYmplY3QqLCBHQXN5bmNSZXN1bHQqIHJlc3Vs dCwgRWRpdG9yVGVzdCogdGVzdCkK