Bug 25597 – In prepartion for enabling VIDEO, fixing some API drift that caused this code to no longer compiler.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Albert J. Wong]

In preparation for enabling the VIDEO tag in chromium for Mac, there were some compilation errors found in the #ifdef'ed segment of code for rendering media controls.  This patch shores up the API drift.

Note that the copying of MediaControllerThemeStyle from RenderThemeMac.mm is a temporary fix until we can settle on the look/feel of the media controls on Mac.  For now, this is just to get us building and displaying.

[Albert J. Wong]

Created attachment 30065 [details]
Fix some API drift that occurred while this code was ifdefed out.

[Albert J. Wong]

Comment on attachment 30065 [details]
Fix some API drift that occurred while this code was ifdefed out.

gha...wrong patch uploaded. uploading the one with the changelog in a sec.

[Albert J. Wong]

Created attachment 30066 [details]
Fix some API drift that occurred while this code was ifdefed out.

This one has the changelog.

[Albert J. Wong]

Comment on attachment 30066 [details]
Fix some API drift that occurred while this code was ifdefed out.

>diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
>index f4cc242..8035487 100644
>--- a/WebCore/ChangeLog
>+++ b/WebCore/ChangeLog
>@@ -1,3 +1,18 @@
>[phone]  Albert J. Wong  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=25597
>+        Fix API drift compile errors that occurred while this was ifdef-ed out.
>+
>+        * rendering/RenderThemeChromiumMac.mm:
>+        (WebCore::RenderThemeChromiumMac::paintMediaFullscreenButton):
>+        (WebCore::RenderThemeChromiumMac::paintMediaMuteButton):
>+        (WebCore::RenderThemeChromiumMac::paintMediaPlayButton):
>+        (WebCore::RenderThemeChromiumMac::paintMediaSeekBackButton):
>+        (WebCore::RenderThemeChromiumMac::paintMediaSeekForwardButton):
>+        (WebCore::RenderThemeChromiumMac::paintMediaSliderTrack):
>+        (WebCore::RenderThemeChromiumMac::paintMediaSliderThumb):
>+
> 2009-05-06  Hin-Chung Lam  <[e-mail]>
> 
>         Reviewed by Darin Fisher.
>diff --git a/WebCore/rendering/RenderThemeChromiumMac.mm b/WebCore/rendering/RenderThemeChromiumMac.mm
>index 61bc9eb..56c07de 100644
>--- a/WebCore/rendering/RenderThemeChromiumMac.mm
>+++ b/WebCore/rendering/RenderThemeChromiumMac.mm
>@@ -1779,15 +1779,23 @@ bool RenderThemeChromiumMac::paintSearchFieldResultsButton(RenderObject* o, cons
>     return false;
> }
> 
>+#if ENABLE(VIDEO)
>+// FIXME: This enum is lifted from RenderThemeMac.mm  We need to decide which theme to use for the default controls, or decide to avoid wkDrawMediaUIPart and render our own.
>+typedef enum {
>+    MediaControllerThemeClassic   = 1,
>+    MediaControllerThemeQT        = 2
>+} MediaControllerThemeStyle;
>+#endif
>+
> bool RenderThemeChromiumMac::paintMediaFullscreenButton(RenderObject* o, const RenderObject::PaintInfo& paintInfo, const IntRect& r)
> {
> #if ENABLE(VIDEO)
>-    Node* node = o->element();
>+    Node* node = o->node();
>     if (!node)
>         return false;
> 
>     LocalCurrentGraphicsContext localContext(paintInfo.context);
>-    wkDrawMediaUIPart(MediaFullscreenButton, paintInfo.context->platformContext(), r, node->active());
>+    wkDrawMediaUIPart(MediaFullscreenButton, MediaControllerThemeClassic, paintInfo.context->platformContext(), r, node->active());
> #endif
>     return false;
> }
>@@ -1795,7 +1803,7 @@ bool RenderThemeChromiumMac::paintMediaFullscreenButton(RenderObject* o, const R
> bool RenderThemeChromiumMac::paintMediaMuteButton(RenderObject* o, const RenderObject::PaintInfo& paintInfo, const IntRect& r)
> {
> #if ENABLE(VIDEO)
>-    Node* node = o->element();
>+    Node* node = o->node();
>     Node* mediaNode = node ? node->shadowAncestorNode() : 0;
>     if (!mediaNode || (!mediaNode->hasTagName(videoTag) && !mediaNode->hasTagName(audioTag)))
>         return false;
>@@ -1805,7 +1813,7 @@ bool RenderThemeChromiumMac::paintMediaMuteButton(RenderObject* o, const RenderO
>         return false;
>     
>     LocalCurrentGraphicsContext localContext(paintInfo.context);
>-    wkDrawMediaUIPart(mediaElement->muted() ? MediaUnMuteButton : MediaMuteButton, paintInfo.context->platformContext(), r, node->active());
>+    wkDrawMediaUIPart(mediaElement->muted() ? MediaUnMuteButton : MediaMuteButton, MediaControllerThemeClassic, paintInfo.context->platformContext(), r, node->active());
> #endif
>     return false;
> }
>@@ -1813,7 +1821,7 @@ bool RenderThemeChromiumMac::paintMediaMuteButton(RenderObject* o, const RenderO
> bool RenderThemeChromiumMac::paintMediaPlayButton(RenderObject* o, const RenderObject::PaintInfo& paintInfo, const IntRect& r)
> {
> #if ENABLE(VIDEO)
>-    Node* node = o->element();
>+    Node* node = o->node();
>     Node* mediaNode = node ? node->shadowAncestorNode() : 0;
>     if (!mediaNode || (!mediaNode->hasTagName(videoTag) && !mediaNode->hasTagName(audioTag)))
>         return false;
>@@ -1823,7 +1831,7 @@ bool RenderThemeChromiumMac::paintMediaPlayButton(RenderObject* o, const RenderO
>         return false;
> 
>     LocalCurrentGraphicsContext localContext(paintInfo.context);
>-    wkDrawMediaUIPart(mediaElement->canPlay() ? MediaPlayButton : MediaPauseButton, paintInfo.context->platformContext(), r, node->active());
>+    wkDrawMediaUIPart(mediaElement->canPlay() ? MediaPlayButton : MediaPauseButton, MediaControllerThemeClassic, paintInfo.context->platformContext(), r, node->active());
> #endif
>     return false;
> }
>@@ -1831,12 +1839,12 @@ bool RenderThemeChromiumMac::paintMediaPlayButton(RenderObject* o, const RenderO
> bool RenderThemeChromiumMac::paintMediaSeekBackButton(RenderObject* o, const RenderObject::PaintInfo& paintInfo, const IntRect& r)
> {
> #if ENABLE(VIDEO)
>-    Node* node = o->element();
>+    Node* node = o->node();
>     if (!node)
>         return false;
> 
>     LocalCurrentGraphicsContext localContext(paintInfo.context);
>-    wkDrawMediaUIPart(MediaSeekBackButton, paintInfo.context->platformContext(), r, node->active());
>+    wkDrawMediaUIPart(MediaSeekBackButton, MediaControllerThemeClassic, paintInfo.context->platformContext(), r, node->active());
> #endif
>     return false;
> }
>@@ -1844,12 +1852,12 @@ bool RenderThemeChromiumMac::paintMediaSeekBackButton(RenderObject* o, const Ren
> bool RenderThemeChromiumMac::paintMediaSeekForwardButton(RenderObject* o, const RenderObject::PaintInfo& paintInfo, const IntRect& r)
> {
> #if ENABLE(VIDEO)
>-    Node* node = o->element();
>+    Node* node = o->node();
>     if (!node)
>         return false;
> 
>     LocalCurrentGraphicsContext localContext(paintInfo.context);
>-    wkDrawMediaUIPart(MediaSeekForwardButton, paintInfo.context->platformContext(), r, node->active());
>+    wkDrawMediaUIPart(MediaSeekForwardButton, MediaControllerThemeClassic, paintInfo.context->platformContext(), r, node->active());
> #endif
>     return false;
> }
>@@ -1857,7 +1865,7 @@ bool RenderThemeChromiumMac::paintMediaSeekForwardButton(RenderObject* o, const
> bool RenderThemeChromiumMac::paintMediaSliderTrack(RenderObject* o, const RenderObject::PaintInfo& paintInfo, const IntRect& r)
> {
> #if ENABLE(VIDEO)
>-    Node* node = o->element();
>+    Node* node = o->node();
>     Node* mediaNode = node ? node->shadowAncestorNode() : 0;
>     if (!mediaNode || (!mediaNode->hasTagName(videoTag) && !mediaNode->hasTagName(audioTag)))
>         return false;
>@@ -1875,7 +1883,7 @@ bool RenderThemeChromiumMac::paintMediaSliderTrack(RenderObject* o, const Render
>         currentTime = player->currentTime();
>     }
> 
>-    wkDrawMediaSliderTrack(paintInfo.context->platformContext(), r, timeLoaded, currentTime, duration);
>+    wkDrawMediaSliderTrack(MediaControllerThemeClassic, paintInfo.context->platformContext(), r, timeLoaded, currentTime, duration);
> #endif
>     return false;
> }
>@@ -1883,12 +1891,12 @@ bool RenderThemeChromiumMac::paintMediaSliderTrack(RenderObject* o, const Render
> bool RenderThemeChromiumMac::paintMediaSliderThumb(RenderObject* o, const RenderObject::PaintInfo& paintInfo, const IntRect& r)
> {
> #if ENABLE(VIDEO)
>-    Node* node = o->element();
>+    Node* node = o->node();
>     if (!node)
>         return false;
> 
>     LocalCurrentGraphicsContext localContext(paintInfo.context);
>-    wkDrawMediaUIPart(MediaSliderThumb, paintInfo.context->platformContext(), r, node->active());
>+    wkDrawMediaUIPart(MediaSliderThumb, MediaControllerThemeClassic, paintInfo.context->platformContext(), r, node->active());
> #endif
>     return false;
> }

[Albert J. Wong]

Created attachment 30068 [details]
Fix some API drift that occurred while this code was ifdefed out -- now with correct ChangeLog!

this time for sure!

[David Levin]

Assigned to levin for landing.

[David Levin]

http://trac.webkit.org/changeset/43329