This crash appears to be a regression of this changelist http://trac.webkit.org/changeset/53693 for bug https://bugs.webkit.org/show_bug.cgi?id=33150. The crash has been consistently occurring in Chromium reliability test runs. The callstack can be found here. http://build.chromium.org/buildbot/waterfall/builders/Chromium%20Reliability/builds/7979/steps/reliability:%20partial%20result%20of%20current%20build/logs/stdio
Happy to revert the change if we can confirm it's that revision.
I looked at the reliability test runs from Friday. The crashes started occurring from the time we bumped up the DEPS to pull in the corresponding webkit revision. The only change in the revision of webkit which could cause this appears to be this change. We can try reverting it and see if this is indeed the case.
This would be my fault. I am getting Chronium to see what's wrong, I hope I can compile it quickly on Linux.
Created attachment 47403 [details] Unregister the RenderObject in corner cases This patch should fix the problem for the websites listed. The problem is deriving from the change of https://bugs.webkit.org/show_bug.cgi?id=33150 RenderWidget::destroy() is special and do not call ::destroy() on the parent. The code for cleaning the state must be copied in that class (as said in the comments of RenderWidget::destroy()). It's great you have those tests.
Could http://trac.webkit.org/changeset/53797 be reverted until this crash is resolved?
Typo in changelog: + The code to unregister the object with fixed positionning must
Comment on attachment 47403 [details] Unregister the RenderObject in corner cases > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > + The code to unregister the object with fixed positionning must > + be copied to RenderWidget. "positionning" > + Unregistering must also be done when a render widget get a > + transformation. Not just a RenderWidget, but any RenderObject. "gets", not "get". > diff --git a/WebCore/rendering/RenderWidget.cpp b/WebCore/rendering/RenderWidget.cpp > animation()->cancelAnimations(this); > > - if (RenderView* v = view()) > + if (RenderView* v = view()) { > v->removeWidget(this); > + FrameView* frameView = view()->frameView(); > + if (frameView) > + frameView->unregisterFixedPositionedObject(this); > + } Why doesn't this have the "m_style->position() == FixedPosition" check that RenderObject does? Is one of them wrong? This also needs a layout test. r- for that.
Created attachment 47472 [details] Revert 53797 until we land
(In reply to comment #7) > > animation()->cancelAnimations(this); > > > > - if (RenderView* v = view()) > > + if (RenderView* v = view()) { > > v->removeWidget(this); > > + FrameView* frameView = view()->frameView(); > > + if (frameView) > > + frameView->unregisterFixedPositionedObject(this); > > + } > > Why doesn't this have the "m_style->position() == FixedPosition" check that > RenderObject does? Is one of them wrong? Yep, in the hurry I forgot the "m_style->position() == FixedPosition" for this one. (In reply to comment #8) > Created an attachment (id=47472) [details] > Revert 53797 until we land Ok.
Closing the task: The original patch has been reverted. The upgraded version is on https://bugs.webkit.org/show_bug.cgi?id=33150 for review.