Bug 56969

Summary: Move media controls subtree creation into one method.
Product: WebKit Reporter: Dimitri Glazkov (Google) <dglazkov>
Component: New BugsAssignee: Dimitri Glazkov (Google) <dglazkov>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 53020    
Attachments:
Description Flags
Patch darin: review+

Dimitri Glazkov (Google)
Reported 2011-03-23 15:34:33 PDT
Move media controls subtree creation into one method.
Attachments
Patch (13.26 KB, patch)
2011-03-23 15:37 PDT, Dimitri Glazkov (Google) 		darin: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dimitri Glazkov (Google)
Comment 1 2011-03-23 15:37:57 PDT
Created attachment 86711 [details] Patch
Darin Adler
Comment 2 2011-03-23 16:18:32 PDT
Comment on attachment 86711 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86711&action=review > Source/WebCore/rendering/MediaControlElements.cpp:800 > + setAttribute(precisionAttr, "float"); It’s not a great idea to call functional like this inside the constructor where the object still hasn’t been adopted by its first owner. It can lead to RefCounted assertions if anything ref/derefs. I took these out of the class intentionally a while back, and you are undoing that. The work should be either in a create function or at the call site rather than in the constructor. > Source/WebCore/rendering/MediaControlElements.cpp:864 > + setAttribute(precisionAttr, "float"); > + setAttribute(maxAttr, "1"); > + setAttribute(valueAttr, String::number(mediaElement->volume())); Ditto.
Dimitri Glazkov (Google)
Comment 3 2011-03-23 16:23:33 PDT
(In reply to comment #2) > (From update of attachment 86711 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=86711&action=review > > > Source/WebCore/rendering/MediaControlElements.cpp:800 > > + setAttribute(precisionAttr, "float"); > > It’s not a great idea to call functional like this inside the constructor where the object still hasn’t been adopted by its first owner. It can lead to RefCounted assertions if anything ref/derefs. I took these out of the class intentionally a while back, and you are undoing that. > > The work should be either in a create function or at the call site rather than in the constructor. > > > Source/WebCore/rendering/MediaControlElements.cpp:864 > > + setAttribute(precisionAttr, "float"); > > + setAttribute(maxAttr, "1"); > > + setAttribute(valueAttr, String::number(mediaElement->volume())); > > Ditto. Sure, I'll move these to create func.
Dimitri Glazkov (Google)
Comment 4 2011-03-24 09:58:25 PDT
Committed r81872: <http://trac.webkit.org/changeset/81872>
Note You need to log in before you can comment on or make changes to this bug.