[Webkit-unassigned] [Bug 79729] [BlackBerry] upstream MediaPlayerPrivateBlackBerry.[cpp|h]

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=79729


Antonio Gomes <tonikitoo at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #129195|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #4 from Antonio Gomes <tonikitoo at webkit.org>  2012-03-03 17:47:54 PST ---
(From update of attachment 129195)
View in context: https://bugs.webkit.org/attachment.cgi?id=129195&action=review

r=me, but please fix it up before landing.

When you have the revised patch, please re-upload and ask Leo or Charles to cq+ it. No need to r? again :-)

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:53
> +/////////////////////////////////////////////////////////////////////////////

Lets remove this "///*///" line.

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:60
> +// static

remove

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:66
> +// static

ditto

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:75
> +// static

ditto

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:92
> +// static

ditto

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:98
> +// static

ditto

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:104
> +/////////////////////////////////////////////////////////////////////////////

ditto

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:172
> +}
> +
> +
> +void MediaPlayerPrivate::prepareToPlay()

extra line

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:178
> +}
> +
> +
> +void MediaPlayerPrivate::play()

ditto

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:389
> +    // If we have an HTMLVideoElement but the source has no video, then we need to resize the media element.
> +    if (client && client->isVideo() && !hasVideo()) {
> +        if (RenderObject* ro = client->renderer()) {
> +            IntRect rect = ro->enclosingBox()->contentBoxRect();
> +
> +            static const int playbookMinAudioElementWidth = 300;
> +            static const int playbookMinAudioElementHeight = 32;
> +            // If the rect dimensions are less than the allowed minimum, use the minimum instead.
> +            int newWidth = max(rect.width(), playbookMinAudioElementWidth);
> +            int newHeight = max(rect.height(), playbookMinAudioElementHeight);
> +
> +            char attrString[12];
> +
> +            sprintf(attrString, "%d", newWidth);
> +            client->setAttribute(HTMLNames::widthAttr, attrString);
> +
> +            sprintf(attrString, "%d", newHeight);
> +            client->setAttribute(HTMLNames::heightAttr, attrString);
> +        }
> +    }
> +
> +    // If we don't know what the width and height of the video source is, then we need to set it to something sane.
> +    if (client && client->isVideo() && !(m_platformPlayer->sourceWidth() && m_platformPlayer->sourceHeight())) {
> +        if (RenderObject* ro = client->renderer()) {
> +            IntRect rect = ro->enclosingBox()->contentBoxRect();
> +            m_platformPlayer->setSourceDimension(rect.width(), rect.height());
> +        }

maybe it would look cleaner this:

if (!client || !client->isVideo())
    return;

RenderObject* o = client->renderer();
if (!o)
    return;

....

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:419
> +int MediaPlayerPrivate::windowPositionGet(unsigned& x, unsigned& y, unsigned& width, unsigned& height) const

"Get" as the suffix is not common in WebKit. In fact, "get" is only used in cases exactly like this, where you "getting" values passed as references. However, I think it should be "getWindowPosition".

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:424
> +const char* MediaPlayerPrivate::mmrContextNameGet()

rename it to mmrContextName()? i.e. no "get"..

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:532
> +//////////////////////////////////////////////////////////////////

Drop the "//////*////" line.

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:687
> +}
> +
> +
> +#if USE(ACCELERATED_COMPOSITING)
> +static const double BufferingAnimationDelay = 1.0 / 24;

drop extra blank line.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.