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

[bugzilla-daemon at webkit.org]

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


Rob Buis <rwlbuis at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #129160|review?                     |review-
               Flag|                            |




--- Comment #2 from Rob Buis <rwlbuis at gmail.com>  2012-02-27 19:25:52 PST ---
(From update of attachment 129160)
View in context: https://bugs.webkit.org/attachment.cgi?id=129160&action=review

Looks good, but I think it can be cleaned up some more.

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:292
> +    if (bufferLoaded)

CAn combine to if (float bufferLoaded = m_platformPlayer->bufferLoaded())

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:375
> +        if (ro) {

Can be combined to if (RenderObject* ro = client->renderer())

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:379
> +            int newWidth = (rect.width() > PLAYBOOK_MIN_AUDIO_ELEMENT_WIDTH) ? rect.width() : PLAYBOOK_MIN_AUDIO_ELEMENT_WIDTH;

Can use max here.

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:380
> +            int newHeight = (rect.height() > PLAYBOOK_MIN_AUDIO_ELEMENT_HEIGHT) ? rect.height() : PLAYBOOK_MIN_AUDIO_ELEMENT_HEIGHT;

Ditto.

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:395
> +        if (ro) {

Can be combined to if (RenderObject* ro = client->renderer())

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:610
> +    if (element)

Can be combined to if (HTMLMediaElement* element = static_cast<HTMLMediaElement*>(m_webCorePlayer->mediaPlayerClient()))

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:617
> +    if (element)

Ditto.

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h:39
> +#define PLAYBOOK_MIN_AUDIO_ELEMENT_HEIGHT 32

Could be put in the cpp

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h:141
> +    virtual BlackBerry::Platform::Graphics::Window* platformWindow();

Should enter empty line here.

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h:149
> +private:

private can be removed, already is in private section.

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