[webkit-reviews] review denied: [Bug 26304] [GTK] Add controls for playing html5 video. : [Attachment 44416] rebased the 3 patches in one

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 26304: [GTK] Add controls for playing html5 video.
https://bugs.webkit.org/show_bug.cgi?id=26304

Attachment 44416: rebased the 3 patches in one
https://bugs.webkit.org/attachment.cgi?id=44416&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
I don't like this patch -- the RenderThemeGtk changes just seem bad.

getIconNameForTextDirection leaks the character data underlying the string

All the global resources (panelColor, playButtonIconName, muteButton, etc)
should not be global and instead should be hung off the RenderThemeGtk object

The use of PassRefPtr is (as the name suggests) wrong for storing a reference
to an object -- PassRefPtr will clear itself once the underlying object is
retrieved -- you should use RefPtr.

Honestly I also believe that webkit/gtk would greatly benefit from adding a C++
wrapper for gtk objects that does automatic refcounting so you could more
easily (for instance) just use gstring's everywhere, and also so you can remove
destructors that are simply there to manually deref objects.

--Oliver