http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm introduced in 88913
The correct result was changed to the failing result by https://trac.webkit.org/changeset/92554
Created attachment 103460 [details] actual png - failing result expected result: http://trac.webkit.org/browser/trunk/LayoutTests/platform/qt/http/tests/misc/object-embedding-svg-delayed-size-negotiation-2-expected.png
you can easily reproduce it with: Tools/Scripts/old-run-webkit-tests LayoutTests/http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm --wait-for-httpd --iterations 100 --exit-after-n-failures --platform qt -p test was skpped by https://trac.webkit.org/changeset/92761
(In reply to comment #2) > Created an attachment (id=103460) [details] > actual png - failing result > > expected result: > http://trac.webkit.org/browser/trunk/LayoutTests/platform/qt/http/tests/misc/object-embedding-svg-delayed-size-negotiation-2-expected.png This is very convincing that the bug is visible on screen, not just in the DRT dumps, which I assumed so far. I wish we had all bots running pixel tests :( Thanks, Ossy. Would you want to debug this on Qt, or do you know anyone who might want to investigate? I've a hard time reproducing this on mac - I've never been able to do it so far. (In reply to comment #3) > you can easily reproduce it with: > Tools/Scripts/old-run-webkit-tests LayoutTests/http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm --wait-for-httpd --iterations 100 --exit-after-n-failures --platform qt -p You made my day! I had no luck in reproducing any of the svg/zoom/page/* problems on my both macs so far that were visible on gtk/qt/chromium/win (zoom-object* tests are flakey). But using this specific test, I can reproduce the problem, finally! 99 test cases (99%) succeeded 1 test case (1%) had incorrect layout Hopefully I'm now able to find the root issue.
Created attachment 103753 [details] first repro on mac slightly different png
Okay, I found the problem, and will upload a patch soon. I traced this down with QtWebKit/Mac. I must say that's no fun - build-webkit --qt --debug takes 23minutes on my macbook, when editing a single .cpp file and rebuilding :( Glad it's over now :-)
Created attachment 105325 [details] Patch Tried this patch with a from-scratch rebuilt of Qt/Mac debug, and ran the testcase 1000 tests with old-run-webkit-tests --iterations 1000 -p ... Works w/o problems now!
Comment on attachment 105325 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=105325&action=review > Source/WebCore/page/FrameView.cpp:2754 > + HashSet<RefPtr<FrameView> > frameViews; > + collectFrameViewChildren(this, frameViews); Why do you use HashSet<> here? For me it seems a simple Vector would be enough which is much faster and use less sytem resources.
(In reply to comment #8) > (From update of attachment 105325 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=105325&action=review > > > Source/WebCore/page/FrameView.cpp:2754 > > + HashSet<RefPtr<FrameView> > frameViews; > > + collectFrameViewChildren(this, frameViews); > > Why do you use HashSet<> here? For me it seems a simple Vector would be enough which is much faster and use less sytem resources. Yes, you're just right! Uploading a revised version.
Created attachment 105331 [details] Patch v2
Comment on attachment 105331 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=105331&action=review > Source/WebCore/page/FrameView.cpp:119 > + , m_inLayoutParentView(false) Throw a #if ENABLE(SVG) around it. > Source/WebCore/page/FrameView.h:403 > + bool m_inLayoutParentView; Ditto.
(In reply to comment #11) > (From update of attachment 105331 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=105331&action=review > > > Source/WebCore/page/FrameView.cpp:119 > > + , m_inLayoutParentView(false) > > Throw a #if ENABLE(SVG) around it. > > > Source/WebCore/page/FrameView.h:403 > > + bool m_inLayoutParentView; > > Ditto. Ok, shall I upload a new version?
Created attachment 105336 [details] Patch v3 Uploading the final version, if we need to rollout the patch, this makes it easier.
Comment on attachment 105336 [details] Patch v3 Nice patch. r=me
Landed in r93862. Didn't cause any regressions. Unskipped the test in r93864 on Qt, it may need a rebaseline.