Bug 25597 - In prepartion for enabling VIDEO, fixing some API drift that caused this code to no longer compiler.
Summary: In prepartion for enabling VIDEO, fixing some API drift that caused this code...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: David Levin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-06 14:32 PDT by Albert J. Wong
Modified: 2009-05-06 15:56 PDT (History)
2 users (show)

See Also:


Attachments
Fix some API drift that occurred while this code was ifdefed out. (6.19 KB, patch)
2009-05-06 14:33 PDT, Albert J. Wong
no flags Details | Formatted Diff | Diff
Fix some API drift that occurred while this code was ifdefed out. (7.11 KB, patch)
2009-05-06 14:37 PDT, Albert J. Wong
no flags Details | Formatted Diff | Diff
Fix some API drift that occurred while this code was ifdefed out -- now with correct ChangeLog! (7.36 KB, patch)
2009-05-06 14:52 PDT, Albert J. Wong
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Albert J. Wong 2009-05-06 14:32:09 PDT
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.
Comment 1 Albert J. Wong 2009-05-06 14:33:22 PDT
Created attachment 30065 [details]
Fix some API drift that occurred while this code was ifdefed out.
Comment 2 Albert J. Wong 2009-05-06 14:34:08 PDT
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.
Comment 3 Albert J. Wong 2009-05-06 14:37:05 PDT
Created attachment 30066 [details]
Fix some API drift that occurred while this code was ifdefed out.

This one has the changelog.
Comment 4 Albert J. Wong 2009-05-06 14:38:39 PDT
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 @@
>+2009-05-06  Albert J. Wong  <ajwong@chromium.org>
>+
>+        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  <hclam@chromium.org>
> 
>         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;
> }
Comment 5 Albert J. Wong 2009-05-06 14:52:34 PDT
Created attachment 30068 [details]
Fix some API drift that occurred while this code was ifdefed out -- now with correct ChangeLog!

this time for sure!
Comment 6 David Levin 2009-05-06 15:49:51 PDT
Assigned to levin for landing.
Comment 7 David Levin 2009-05-06 15:56:22 PDT
http://trac.webkit.org/changeset/43329