Summary: | JSC wrappers for TextTrack and TextTrackCue should not be collected during event dispatch or when owner is reachable | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anna Cavender <annacc> | ||||||
Component: | Media | Assignee: | Eric Carlson <eric.carlson> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, eric.carlson, ojan, rniwa, tonyg, webkit-bug-importer, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 43668 | ||||||||
Attachments: |
|
Description
Anna Cavender
2011-11-11 13:52:52 PST
*** Bug 72149 has been marked as a duplicate of this bug. *** Wrote up 73865 for the V8 changes. Created attachment 117966 [details]
Proposed patch
Attachment 117966 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/medi..." exit_code: 1
LayoutTests/ChangeLog:8: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
LayoutTests/ChangeLog:10: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 2 in 19 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 117985 [details]
Updated patch
Fix the malformed ChangeLog.
Comment on attachment 117985 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=117985&action=review r=me > Source/WebCore/bindings/js/JSTextTrackCueCustom.cpp:57 > + return visitor.containsOpaqueRoot(root(textTrackCue->track()->mediaElement())); Slightly nicer to call "root(textTrackCue->track())", and put root(TextTrack*) inline in a header. That way, if a text track ever changes its mind about what its root is, we only have to update one function. > Source/WebCore/bindings/js/JSTextTrackCustom.cpp:52 > + return visitor.containsOpaqueRoot(root(textTrack->mediaElement())); This would be "root(textTrack)", if you followed my advice above. > Source/WebCore/html/track/TextTrackList.cpp:93 > + scheduleAddTrackEvent(track); Can this be "track.release()"? > Source/WebCore/html/track/TextTrackList.cpp:96 > +void TextTrackList::remove(PassRefPtr<TextTrack> prpTrack) Seems like prpTrack should just be a raw pointer, since we don't take ownership. Mac rebaseline done in http://trac.webkit.org/changeset/103642. |