[Webkit-unassigned] [Bug 77951] Chrome crashes when attempting to add cue to track element

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Feb 12 20:23:38 PST 2012


https://bugs.webkit.org/show_bug.cgi?id=77951





--- Comment #10 from Arun Patole <arun.patole at motorola.com>  2012-02-12 20:23:38 PST ---
(In reply to comment #9)
> (In reply to comment #8)
> > > We are splitting semantic hairs here, but "and set its text track list of cues to an empty list" it seems to me that 0 is a perfectly valid "empty list". The bug is that the current code tries to *use* the nil ("empty") list.
> > 
> > Sorry if i have not understood it correctly, but when I locally reproduced this crash, I found that it was crashing because of null pointer m_cues and not because of the pointer to an empty list. Then I referred spec and found that it should be initialized to an empty list when track is created so i initialized it with TextTrackCueList::create().
> > "if(m_cues)" checks were removed and asserts were added before it use just to make sure that its non-null and it is not accidently made null at any other place. 
> > If you still think its wrong I will abandon this patch.
> 
> I was only trying to point out that the way we represent what the spec calls "an empty list" is an implementation detail. The current code *tries* to avoid allocating the list until it is needed, so in the current code nil represents what the spec calls "an empty list". Clearly the current code is wrong, because it will crash if either addCue or removeCue is called. 
> 
> One way to fix this is to allocate it in the constructor as you have done. Another way would be to allocate the list in addCue and removeCue. The way you have done it is OK, it makes the code slightly simpler at the cost of more memory when we have a track with no cues. A track will probably have cues more times than not, so the memory saved is negligible.
> 
> I don't think the asserts are needed because it is unlikely that the list will be accidentally deleted, and the null checks have been removed so we will now crash immediately. I don't feel strongly about this, you can leave them in if you think they are useful.
Ok thanks, I will upload the modified patch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list