Bug 31574 - Crashing bug when removing <ruby> element
Summary: Crashing bug when removing <ruby> element
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Roland Steiner
Depends on:
Reported: 2009-11-16 20:18 PST by Roland Steiner
Modified: 2009-12-02 19:26 PST (History)
2 users (show)

See Also:

patch - fix crash (5.85 KB, patch)
2009-11-16 21:01 PST, Roland Steiner
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Steiner 2009-11-16 20:18:54 PST
Found by Shinichiro Hamaji.

The renderer crashes if a <ruby> tag is being deleted via JavaScript. The example given was:

<div id="t"><ruby>f</div>
<script>t.innerHTML = '';</script>
Comment 1 Roland Steiner 2009-11-16 21:01:05 PST
Created attachment 43341 [details]
patch - fix crash

Cause of the bug:

  1.) RenderBlock::destroy() of the RenderRubyRun called destroyLeftoverChildren()
  2.) that called destroy() of the RenderRubyBase(), which in RenderObject::destroy() calls remove()
  3.) remove() is being redirected as parent()->removeChild() in RenderObject.h
  4.) this triggers the special handling of child removal in RenderRubyRun that causes it to destroy itself
  5.) On returning from all this the renderer crashes when accessing a member
      or virtual function on this now illegal object.

I therefore added a flag that tracks if the ruby run is being destroyed. If so, avoid doing the special handling in removeChild that caused this. It's not the most elegant solution, but the easiest to implement without touching unrelated code. Also, it's self-documenting.
Comment 2 Roland Steiner 2009-11-17 22:36:06 PST
The bug may also be triggered by the code that handles invalid HTML markup rather than just JavaScript.

Updated to P1 as it's a crashing bug.
Comment 3 Eric Seidel (no email) 2009-11-18 13:39:39 PST
Comment on attachment 43341 [details]
patch - fix crash

I'm not well enough versed with the ruby code to understand why the ruby classes need this special memory management.  How does RenderRubyRun relate to RenderInline for instance?  Does RenderInline use a similar m_beingDestroyed method?

I'm missing enough context to understand why this fix is the right fix.
Comment 4 Darin Adler 2009-11-18 15:20:54 PST
Comment on attachment 43341 [details]
patch - fix crash

I agree that there are probably more elegant fixes. But this should do, at least for now.

Comment 5 Roland Steiner 2009-11-18 20:51:33 PST
committed in r51169
Comment 6 Eric Seidel (no email) 2009-12-02 12:00:30 PST
There are several tools which can auto-close the bugs for you.

bugzilla-tool land-diff is one of them. :)
mark-bug-fixed is another.
Comment 7 Roland Steiner 2009-12-02 19:26:03 PST
(In reply to comment #6)

Thanks, good to know! I assumed that bugs needed verification that they are indeed fixed.