[Webkit-unassigned] [Bug 62777] [EFL][WK2] Add RunLoopEfl and WorkQueueEfl

[bugzilla-daemon at webkit.org]

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


Raphael Kubo da Costa <kubo at profusion.mobi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kubo at profusion.mobi




--- Comment #8 from Raphael Kubo da Costa <kubo at profusion.mobi>  2011-06-16 07:06:12 PST ---
Informal r- from my side:

> Source/WebKit2/Platform/RunLoop.h:111
> +        static Eina_Bool timerFired(void* data);

Can you make this method non-static and move it and the attributes to the private section?

> Source/WebKit2/Platform/RunLoop.h:174
> +    static void wakeUpEvent(void* data, void* buf, unsigned int size);

Does it need to be static?

> Source/WebKit2/Platform/WorkQueue.h:224
> +    ThreadIdentifier m_workQueueThread;

Don't you need to initialize some of these attributes to sane values in the constructor?

> Source/WebKit2/Platform/WorkQueue.h:234
> +    static void* workQueueThread(WorkQueue*);

This name/signature is weird. You have m_workQueueThread, so a method called workQueueThread() is usually expected to return m_workQueueThread, not perform some random work and then just return 0 at the end (as you always return 0, I don't see why you return a void* by the way.

> Source/WebKit2/Platform/WorkQueue.h:238
> +    static Eina_Bool timerFired(void*);    

Does this function and the other need to be static?

> Source/WebKit2/Platform/efl/RunLoopEfl.cpp:60
> +    ((RunLoop*)data)->performWork();

C++ cast.

> Source/WebKit2/Platform/efl/RunLoopEfl.cpp:69
> +    : m_timer(0)

It's safer to also initialize m_isRepeating.

> Source/WebKit2/Platform/efl/RunLoopEfl.cpp:80
> +    RunLoop::TimerBase* timer = (RunLoop::TimerBase*)data;

C++ cast.

> Source/WebKit2/Platform/efl/RunLoopEfl.cpp:97
> +    m_timer = ecore_timer_add(fireInterval, timerFired, this);

Is there any risk of start() being called multiple times, thus leaking the previous timers?

> Source/WebKit2/Platform/efl/RunLoopEfl.cpp:109
> +    return m_timer;

You're implicitly casting an Ecore_Timer pointer into a bool. It's better to be explicit and use "return (m_timer != 0);"

> Source/WebKit2/Platform/efl/WorkQueueEfl.cpp:125
> +    WorkItemEfl* item = (WorkItemEfl*)data;

Please use a C++ cast here instead of a C one.

> Source/WebKit2/Platform/efl/WorkQueueEfl.cpp:134
> +    ecore_timer_add(delay, timerFired, (void*)(new WorkItemEfl(item, this)));

Ditto (are you sure you really need to cast here?). Plus, don't you need to remove this timer later?

> Source/WebKit2/Platform/efl/WorkQueueEfl.cpp:137
> +Eina_Bool WorkQueue::exitHandler(void  * data, int type, void* event)

void* data, not void  * data

> Source/WebKit2/Platform/efl/WorkQueueEfl.cpp:139
> +    TermWorkItem* item = (TermWorkItem*)data;

C++ cast.

> Source/WebKit2/Platform/efl/WorkQueueEfl.cpp:140
> +    Ecore_Exe_Event_Del* ev = (Ecore_Exe_Event_Del*)event;

C++ cast.

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