[Webkit-unassigned] [Bug 103006] [EFL][WK2] Missing the routine to check the validation for workqueue

[bugzilla-daemon at webkit.org]

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





--- Comment #28 from Christophe Dumez <christophe.dumez at intel.com>  2012-11-23 03:45:18 PST ---
(From update of attachment 175761)
View in context: https://bugs.webkit.org/attachment.cgi?id=175761&action=review

>>>>> Source/WebKit2/Platform/efl/WorkQueueEfl.cpp:85
>>>>> +        for (size_t i = 0; i < workItemQueue.size(); ++i) {
>>>> 
>>>> Please cache workItemQueue.size() before the loop.
>>> 
>>> Could you explain why it is required?
>> 
>> This is WebKit coding style and this makes sure that the queue size is not retrieved (possibly computed) for every iteration of the loop.
> 
> Is it WebKit coding style? There are so many uses like the style in WebKit.

>From http://www.webkit.org/coding/coding-style.html:
size_t frameViewsCount = frameViews.size();
for (size_t i = i; i < frameViewsCount; ++i)
    frameViews[i]->updateLayoutAndStyleIfNeededRecursive();

>>>>> Source/WebKit2/Platform/efl/WorkQueueEfl.cpp:-179
>>>>> -        workQueue->performTimerWork();
>>>> 
>>>> Why is this removed?
>>> 
>>> This function need not be called when m_isValid is false.
>>> So, I inserted this function into performWork().
>> 
>> I see now. I did not see that you moved it. However, moving it has side effects.
>> With your change, performTimerWork() will not be called if m_workItemQueue is empty. Please make sure this is really what we want.
> 
> Thanks for your comment. But I cannot assume the issue.
> WorkQueue::platformInvalidate() is called when trying to delete WorkQueue. At that time, the work for the items in queue is not required any more.
> Even WorkQueue::platformInvalidate() in WorkQueueMac.cpp deletes m_dispatchQueue which has the dispatched items.

Ok, I try to explain again. Imagine this case:
- platformInvalidate() has NOT been called
- m_workItemQueue is empty but m_timerWorkItems is not

Because you moved performTimerWork() to the end of performWork(), performTimerWork() will never get called (because m_workItemQueue.isEmpty() will be true and performWork() will return early. Therefore, in this scenario, the items in m_workItemQueue will never get processed. This is why I was stating that your change has side effects for cases other than m_isValid is false.

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