Bug 160117 – [Coordinated Graphics] Test fast/fixed-layout/fixed-layout.html crashes in debug

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

The problem is that WebPage has its own m_useFixedLayout that is only updated when changed from the UI process. However, layout tests doing internals.setUseFixedLayout() change the frame view directly, and the WebPage doesn't notice it. The WebPage should use the frame view directly to ensure it's always in sync, instead of having its own m_useFixedLayout.

STDERR: ASSERTION FAILED: m_useFixedLayout
STDERR: ../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp(1374) : void WebKit::WebPage::sendViewportAttributesChanged()
STDERR: 1   0x7fcb3ab2caef /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7fcb3ab2caef]
STDERR: 2   0x7fcb414330a4 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::WebPage::sendViewportAttributesChanged()+0x4c) [0x7fcb414330a4]
STDERR: 3   0x7fcb41432fdb /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::WebPage::setSize(WebCore::IntSize const&)+0xb1) [0x7fcb41432fdb]
STDERR: 4   0x7fcb415dcf46 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::AcceleratedDrawingArea::updateBackingStoreState(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&)+0xee) [0x7fcb415dcf46]
STDERR: 5   0x7fcb415df7d6 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::DrawingAreaImpl::updateBackingStoreState(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&)+0x11a) [0x7fcb415df7d6]
STDERR: 6   0x7fcb416c5150 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(void IPC::callMemberFunctionImpl<WebKit::DrawingArea, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&), std::tuple<unsigned long, bool, float, WebCore::IntSize, WebCore::IntSize>, 0ul, 1ul, 2ul, 3ul, 4ul>(WebKit::DrawingArea*, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&), std::tuple<unsigned long, bool, float, WebCore::IntSize, WebCore::IntSize>&&, std::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul>)+0xf8) [0x7fcb416c5150]
STDERR: 7   0x7fcb416c4cc4 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(void IPC::callMemberFunction<WebKit::DrawingArea, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&), std::tuple<unsigned long, bool, float, WebCore::IntSize, WebCore::IntSize>, std::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul> >(std::tuple<unsigned long, bool, float, WebCore::IntSize, WebCore::IntSize>&&, WebKit::DrawingArea*, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&))+0x41) [0x7fcb416c4cc4]
STDERR: 8   0x7fcb416c4a6b /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(void IPC::handleMessage<Messages::DrawingArea::UpdateBackingStoreState, WebKit::DrawingArea, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&)>(IPC::MessageDecoder&, WebKit::DrawingArea*, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&))+0x9b) [0x7fcb416c4a6b]
STDERR: 9   0x7fcb416c4699 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::DrawingArea::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)+0x9d) [0x7fcb416c4699]
STDERR: 10  0x7fcb41047fff /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::MessageDecoder&)+0x125) [0x7fcb41047fff]
STDERR: 11  0x7fcb412bc290 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)+0x4c) [0x7fcb412bc290]
STDERR: 12  0x7fcb410322fe /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(IPC::Connection::dispatchMessage(IPC::MessageDecoder&)+0x3a) [0x7fcb410322fe]
STDERR: 13  0x7fcb41032461 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(IPC::Connection::dispatchMessage(std::unique_ptr<IPC::MessageDecoder, std::default_delete<IPC::MessageDecoder> >)+0x161) [0x7fcb41032461]
STDERR: 14  0x7fcb41032648 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(IPC::Connection::dispatchOneMessage()+0xc8) [0x7fcb41032648]
STDERR: 15  0x7fcb410321a2 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0x52661a2) [0x7fcb410321a2]
STDERR: 16  0x7fcb4103723c /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0x526b23c) [0x7fcb4103723c]
STDERR: 17  0x7fcb41001c8d /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::Function<void ()>::operator()() const+0x37) [0x7fcb41001c8d]
STDERR: 18  0x7fcb3ab483ba /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTF::RunLoop::performWork()+0xce) [0x7fcb3ab483ba]
STDERR: 19  0x7fcb3ab8b1f4 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x22641f4) [0x7fcb3ab8b1f4]
STDERR: 20  0x7fcb3ab8b219 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x2264219) [0x7fcb3ab8b219]
STDERR: 21  0x7fcb3ab8b194 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x2264194) [0x7fcb3ab8b194]
STDERR: 22  0x7fcb3ab8b1c3 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x22641c3) [0x7fcb3ab8b1c3]
STDERR: 23  0x7fcb3660ca26 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(+0x53a26) [0x7fcb3660ca26]
STDERR: 24  0x7fcb3660d854 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(g_main_context_dispatch+0x33) [0x7fcb3660d854]
STDERR: 25  0x7fcb3660da39 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(+0x54a39) [0x7fcb3660da39]
STDERR: 26  0x7fcb3660de60 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(g_main_loop_run+0x1d7) [0x7fcb3660de60]
STDERR: 27  0x7fcb3ab8b794 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTF::RunLoop::run()+0xac) [0x7fcb3ab8b794]
STDERR: 28  0x7fcb415ea654 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(int WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain>(int, char**)+0x82) [0x7fcb415ea654]
STDERR: 29  0x7fcb415ea4ba /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebProcessMainUnix+0x20) [0x7fcb415ea4ba]
STDERR: 30  0x400cfa /home/slave/webkitgtk/gtk-linux-64-debug-tests/build/WebKitBuild/Debug/bin/WebKitWebProcess(main+0x34) [0x400cfa]
STDERR: 31  0x7fcb33473b45 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7fcb33473b45]

[Carlos Garcia Campos]

Created attachment 284411 [details]
Patch

[Build Bot]

Comment on attachment 284411 [details]
Patch

Attachment 284411 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1739211

New failing tests:
compositing/fixed-with-fixed-layout.html

[Build Bot]

Created attachment 284413 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 284411 [details]
Patch

Attachment 284411 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1739219

New failing tests:
compositing/fixed-with-fixed-layout.html

[Build Bot]

Created attachment 284414 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 284411 [details]
Patch

Attachment 284411 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1739217

New failing tests:
platform/ios-simulator/ios/fast/coordinates/inner-window-sizes.html
platform/ios-simulator/ios/fast/coordinates/inner-window-sizes-quirks.html
fast/css/preserve-user-specified-zoom-level-on-reload.html

[Build Bot]

Created attachment 284415 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.5

[Build Bot]

Comment on attachment 284411 [details]
Patch

Attachment 284411 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1739235

New failing tests:
compositing/fixed-with-fixed-layout.html

[Build Bot]

Created attachment 284416 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Michael Catanzaro]

Comment on attachment 284411 [details]
Patch

Code changes look straightforward, r- only because it breaks the iOS and Mac tests.

This one does need a WK2 owner once you fix the tests.

[Carlos Garcia Campos]

This is weird because I'm only moving the code from wk2 to webcore. If this produces different results it means that we are behaving differently when running tests than when enabling fixed layout from the UI process API and we are not testing the real expected behavior.

[Carlos Garcia Campos]

Created attachment 284441 [details]
Updated patch

This is a more conservative approach, it should fix the crash without affecting the layout tests results, even though I think the other patch was the right one.

[Build Bot]

Comment on attachment 284441 [details]
Updated patch

Attachment 284441 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1744180

New failing tests:
platform/ios-simulator/ios/fast/coordinates/inner-window-sizes.html
platform/ios-simulator/ios/fast/coordinates/inner-window-sizes-quirks.html
fast/css/preserve-user-specified-zoom-level-on-reload.html

[Build Bot]

Created attachment 284443 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.5

[Michael Catanzaro]

And now you have three iOS failures instead of one macOS/iOS failure.

Here is the macOS diff for your original patch. The GraphicsLayer position and bounds just changed to no longer be nice round numbers:

--- /Volumes/Data/EWS/WebKit/WebKitBuild/Debug/layout-test-results/compositing/fixed-with-fixed-layout-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Debug/layout-test-results/compositing/fixed-with-fixed-layout-actual.txt
@@ -10,7 +10,7 @@
       (transform [0.80 0.00 0.00 0.00] [0.00 0.80 0.00 0.00] [0.00 0.00 1.00 0.00] [0.00 0.00 0.00 1.00])
       (children 4
         (GraphicsLayer
-          (bounds 1000.00 100.00)
+          (bounds 982.00 100.00)
           (drawsContent 1)
         )
         (GraphicsLayer
@@ -19,13 +19,13 @@
           (drawsContent 1)
         )
         (GraphicsLayer
-          (position 900.00 200.00)
-          (bounds 100.00 100.00)
+          (position 881.00 200.00)
+          (bounds 101.00 100.00)
           (drawsContent 1)
         )
         (GraphicsLayer
-          (position 0.00 1900.00)
-          (bounds 1000.00 100.00)
+          (position 0.00 650.00)
+          (bounds 982.00 100.00)
           (drawsContent 1)
         )
       )


iOS diffs for the new patch:

--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/layout-test-results/platform/ios-simulator/ios/fast/coordinates/inner-window-sizes-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/layout-test-results/platform/ios-simulator/ios/fast/coordinates/inner-window-sizes-actual.txt
@@ -9,8 +9,8 @@
 scaled and panned
 PASS window.innerWidth is 400
 PASS window.innerHeight is 300
-PASS document.documentElement.clientWidth is 400
-PASS document.documentElement.clientHeight is 300
+FAIL document.documentElement.clientWidth should be 400. Was 800.
+FAIL document.documentElement.clientHeight should be 300. Was 600.
 PASS successfullyParsed is true
 
 TEST COMPLETE


--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/layout-test-results/platform/ios-simulator/ios/fast/coordinates/inner-window-sizes-quirks-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/layout-test-results/platform/ios-simulator/ios/fast/coordinates/inner-window-sizes-quirks-actual.txt
@@ -9,8 +9,8 @@
 scaled and panned
 PASS window.innerWidth is 400
 PASS window.innerHeight is 300
-PASS document.body.clientWidth is 400
-PASS document.body.clientHeight is 300
+FAIL document.body.clientWidth should be 400. Was 800.
+FAIL document.body.clientHeight should be 300. Was 600.
 PASS successfullyParsed is true
 
 TEST COMPLETE


--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/layout-test-results/fast/css/preserve-user-specified-zoom-level-on-reload-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/layout-test-results/fast/css/preserve-user-specified-zoom-level-on-reload-actual.txt
@@ -1,8 +1,10 @@
-layer at (0,0) size 2389x1792
-  RenderView at (0,0) size 2389x1792
-layer at (0,0) size 2389x1792
-  RenderBlock {HTML} at (0,0) size 2389x1792
-    RenderBody {BODY} at (23,23) size 2343x1722
-      RenderBlock {P} at (0,0) size 2342x57
-        RenderText {#text} at (0,1) size 1651x54
-          text run at (0,1) width 1651: "This test ensures that we preserve the user-specified zoom level of the page on reload."
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (23,23) size 754x530
+      RenderBlock {P} at (0,0) size 753x171
+        RenderText {#text} at (0,1) size 718x168
+          text run at (0,1) width 718: "This test ensures that we preserve the"
+          text run at (0,58) width 718: "user-specified zoom level of the page"
+          text run at (0,115) width 192: "on reload."

[Michael Catanzaro]

Comment on attachment 284441 [details]
Updated patch

r- is again only for the layout test failures.

[Carlos Garcia Campos]

(In reply to comment #15)
> And now you have three iOS failures instead of one macOS/iOS failure.

Not instead, those happened with the previous one, see comment #6, but I don't understand why, and I don't have a way to work on them either.

[Carlos Garcia Campos]

Created attachment 284565 [details]
Updated patch

This fixes the assert and shouldn't affect other ports, but again this is even worse solution to the previous one in my opinion, but I don't have a way to fix ios failures

[Michael Catanzaro]

I agree, the first patches are better, but since we don't know what's wrong with iOS port we can't use them. I'm CCing a couple iOS port people just in the off chance they have any clue what might be going on with the test results in comment #15, or comments on the patch.

As we discussed, your new patch only touches CoordinatedGraphics and doesn't require an owner, so sad r=me.

[Carlos Garcia Campos]

Committed r203722: <http://trac.webkit.org/changeset/203722>