Bug 25575 - Registered mutation event listener crashes HTMLMediaElement
Summary: Registered mutation event listener crashes HTMLMediaElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-05 12:29 PDT by Eric Carlson
Modified: 2009-05-07 18:41 PDT (History)
1 user (show)

See Also:


Attachments
crash log (10.89 KB, text/plain)
2009-05-05 12:31 PDT, Eric Carlson
no flags Details
patch (6.76 KB, patch)
2009-05-07 18:22 PDT, Darin Adler
simon.fraser: review+
Details | Formatted Diff | Diff
patch (6.76 KB, patch)
2009-05-07 18:22 PDT, Darin Adler
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2009-05-05 12:29:55 PDT
The MediaControlTimelineElement constructor calls setAttribute(precisionAttr, "float"). When used in a page that registers for mutation events, this causes a synchronous event to be fired immediately. The event destructor derefs the event target, the MediaControlTimelineElement, which means the element gets destroyed before the constructor returns. Badness ensues.

Stack trace attached
Comment 1 Eric Carlson 2009-05-05 12:31:02 PDT
Created attachment 30030 [details]
crash log
Comment 2 Eric Carlson 2009-05-05 12:42:15 PDT
Firing a mutation event for something in the shadow tree seems wrong.
Comment 3 Eric Carlson 2009-05-06 17:14:00 PDT
rdar://6862908
Comment 4 Darin Adler 2009-05-06 20:49:26 PDT
(In reply to comment #2)
> Firing a mutation event for something in the shadow tree seems wrong.

There are three separate ways to fix this I can think of:

1) Fix it so mutation events don't fire for elements in shadow trees. This is a problem regardless. If JavaScript code in the main page can get a pointer to a node in the shadow tree, we've got trouble.

2) Setting the initial value of the precision attribute needn't be done inside the constructor and probably shouldn't. It can just be done by RenderMedia::createTimeline instead. Generally we should do as little as possible in derived classes and just use HTML classes in any case we can. Moving the setAttribute call is probably the quickest fix to the problem.

3) To set initial attributes in a way more like the way the parser does, you could create a NamedNodeMap and call setAttributeMap instead. I am pretty sure this code path won't fire any DOM mutation events.
Comment 5 Darin Adler 2009-05-07 15:26:52 PDT
(In reply to comment #2)
> Firing a mutation event for something in the shadow tree seems wrong.

It turns out that firing the mutation event isn't necessary to cause the bug. The bug happens because the object gets ref/deref'd during its constructor and event dispatch is not the only code that does this.
Comment 6 Darin Adler 2009-05-07 18:22:18 PDT
Created attachment 30124 [details]
patch
Comment 7 Darin Adler 2009-05-07 18:22:19 PDT
Created attachment 30125 [details]
patch
Comment 8 Darin Adler 2009-05-07 18:41:26 PDT
http://trac.webkit.org/changeset/43381