[Chromium] fast/frames/frame-limit.html is crashing on debug bots after r67179:r67353 roll
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fframes%2Fframe-limit.html
Created attachment 67789 [details] Proposed patch. Regressed in r67182. In HTMLFrameOwnerElement::willRemove: frame->loader()->frameDetached() causes content frame destruction, then frame->disconnectOwnerElement() crashes.
Comment on attachment 67789 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=67789&action=prettypatch > WebCore/html/HTMLFrameOwnerElement.cpp:57 > + RefPtr<Frame> protect(frame); Aw crap, frameDetached calls FrameLoader::detachFromParent(), which in turn may destroy the frame. Good catch. I see that this is necessary, I just don't like that we have to double-protect it (once here and once in detachFromParent). I also don't like that we end up calling disconnectedOwnerElement twice in such cases.
Comment on attachment 67789 [details] Proposed patch. Rejecting patch 67789 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--quiet']" exit_code: 1 Last 500 characters of output: sts Testing 21299 test cases. websocket/tests/bad-sub-protocol-empty.html -> timed out Sampling process 27542 for 10 seconds with 10 milliseconds of run time between samples Sampling completed, processing symbols... Sample analysis of process 27542 written to file /Users/eseidel/Library/Logs/DumpRenderTree/HangReport.txt Exiting early after 1 failures. 21254 tests run. 562.20s total testing time 21253 test cases (99%) succeeded 1 test case (<1%) timed out 33 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/4042030
Comment on attachment 67789 [details] Proposed patch. I believe this test is just flaky.
Comment on attachment 67789 [details] Proposed patch. Clearing flags on attachment: 67789 Committed r67659: <http://trac.webkit.org/changeset/67659>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/67659 might have broken Qt Linux Release The following changes are on the blame list: http://trac.webkit.org/changeset/67659 http://trac.webkit.org/changeset/67660
Filed bug 45918.
I reordered the two calls in this function in an earlier change. I bet that caused this. I'm not sure using a protect ptr is right here.
*** Bug 45804 has been marked as a duplicate of this bug. ***
I dont' think it's right to add a Protect ptr w/o a comment as to what it's protecting against. In this case, it's not clear from reading the code why it's needed. Clearly my re-ordering of those calls in bug 45365 caused this bug. However before I reordered them the frame counts were off during frameDetached. (WebCore::HTMLFrameOwnerElement::willRemove): - Disconnecting the owner element removes the frame from the frame tree. frameDetached() calls Page::frameCount which expects that the frame is already gone at this point and asserts when it's not. It's unclear how this worked before, except that the frame removal was likely done in the post-attach callback, so the frameCount was wrong (too high) during frameDetached(), but was fixed up in the post-detach callback. Maybe my re-ordering was wrong.
[Chromium] fast/frames/frame-limit.html is slow on debug bots
Pavel, can you please take a look into this. This is a high severity security bug that we dont want to make into v7 (and this is also a regression).
I had a chat with Pavel on this. He dont understand this code path and his attempt was a patch to fix the failure on the bots. It did fix things on the bots and also crash didn't reproduce with my testcase. But someone who understands this code, needs to be check if that is ok and functionally the right thing to do. Eric, i dont know how to process with regards to this. Any advise on the owner for this. Or do you think it is ok to merge this fix for the chromium v7 release.
Updated as always timing out on debug builds in http://trac.webkit.org/changeset/109420
*** Bug 98919 has been marked as a duplicate of this bug. ***
Marking test failures as WontFix. Bug is still accessible and recording in TestExpectations.