[Webkit-unassigned] [Bug 27511] Add WinCE specific platform/graphics files to WebCore

[bugzilla-daemon at webkit.org]

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


George Staikos <staikos at kde.org> changed:

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




--- Comment #85 from George Staikos <staikos at kde.org>  2009-08-10 15:54:30 PDT ---
(From update of attachment 34396)
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 3afc825..2facda8 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,16 @@
> +2009-08-08  Yong Li  <yong.li at torchmobile.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        WINCE PORT: MediaPlayerProxy and MediaPlayerPrivate
> +        https://bugs.webkit.org/show_bug.cgi?id=27511
> +
> +        Written by Crystal Zhang <crystal.zhang at torchmobile.com>
> +
> +        * platform/graphics/wince/MediaPlayerPrivateWince.h: Added.
> +        * platform/graphics/wince/MediaPlayerProxy.cpp: Added.
> +        * platform/graphics/wince/MediaPlayerProxy.h: Added.


  Should probably put Crystal's name in the ChangeLog entry instead.


> +WebMediaPlayerProxy::WebMediaPlayerProxy(MediaPlayer* player)
> +: m_mediaPlayer(player)
> +, m_init(false)
> +, m_hasSentResponseToPlugin(false)

   Need indenting


> +WebMediaPlayerProxy::~WebMediaPlayerProxy()
> +{
> +    m_instance.release();
> +}
> +ScriptInstance WebMediaPlayerProxy::pluginInstance()

  Needs a blank line


> +}
> +void WebMediaPlayerProxy::load(const String& url)
> +{
> +    if (!m_init)
> +        initEngine();
> +    if (m_init)
> +        invokeMethod("play");
> +}
> +void WebMediaPlayerProxy::initEngine()

  More blank lines missing


> +    NamedNodeMap* a = element->attributes();
> +    if (a) {
> +        for (unsigned i = 0; i < a->length(); ++i) {
> +            Attribute* it = a->attributeItem(i);
> +            paramNames.append(it->name().localName().string());
> +            paramValues.append(it->value().string());
> +        }
> +    }

   I guess you should remove the braces around the if, as ugly as it becomes.


> +HTMLMediaElement* WebMediaPlayerProxy::element()
> +{
> +    return static_cast<HTMLMediaElement*>(m_mediaPlayer->mediaPlayerClient());
> +
> +}

   Extra blank line here

> +
> +void WebMediaPlayerProxy::invokeMethod(const String& methodName)
> +{
> +    Frame* frame = element()->document()->frame();
> +    RootObject *root = frame->script()->bindingRootObject();
> +    if (!root)
> +        return;
> +    ExecState *exec = root->globalObject()->globalExec();
> +    Instance* instance = pluginInstance().get();
> +    if (!instance)
> +        return;
> +
> +    instance->begin();
> +    Class *aClass = instance->getClass();
> +    Identifier iden(exec, methodName);
> +    MethodList methodList = aClass->methodsNamed(iden, instance);
> +    ArgList args;
> +    /*JSValuePtr result =*/
> +    instance->invokeMethod(exec, methodList , args);
> +    instance->end();


   This needs a comment to indicate why the result variable is commented out I
think.


   After those minor cleanups it will be good to go.

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