Bug 97646 - [EFL][WK2] WebProcess keeps on waiting for UIProcess
Summary: [EFL][WK2] WebProcess keeps on waiting for UIProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-26 02:03 PDT by Regina Chung
Modified: 2012-10-11 21:56 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.87 KB, patch)
2012-09-26 02:38 PDT, Regina Chung
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Chung 2012-09-26 02:03:51 PDT
WebKit2 Efl with Coordinated Graphics is not working because WebProcess never gets RenderNextFrame message from UIProcess.
Comment 1 Regina Chung 2012-09-26 02:38:01 PDT
Created attachment 165759 [details]
Patch
Comment 2 Yael 2012-09-28 12:27:02 PDT
Comment on attachment 165759 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=165759&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:533
> +    priv->viewportHandler->setRendererActive(true);

Is this really needed every time we enter AC or only the first time?
As an aside, PageClientImpl::enterAcceleratedCompositingMode() is not implemented in any other port, so we should probably move the initialization code out of it instead of adding to it.
Comment 3 Yael 2012-10-10 15:37:16 PDT
(In reply to comment #2)
> (From update of attachment 165759 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=165759&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:533
> > +    priv->viewportHandler->setRendererActive(true);
> 
> Is this really needed every time we enter AC or only the first time?
> As an aside, PageClientImpl::enterAcceleratedCompositingMode() is not implemented in any other port, so we should probably move the initialization code out of it instead of adding to it.

After http://trac.webkit.org/changeset/130389 ewk_view_accelerated_compositing_mode_enter is called only once.
This patch looks good to me.
Comment 4 Gyuyoung Kim 2012-10-10 20:45:41 PDT
Don't you need to set setRendererActive(false) in ewk_view_accelerated_compositing_mode_exit() ?
Comment 5 Yael 2012-10-11 13:14:33 PDT
(In reply to comment #4)
> Don't you need to set setRendererActive(false) in ewk_view_accelerated_compositing_mode_exit() ?

What is the benefit of doing that?
We need to call setActive(true) to trigger the first renderNextFrame, so that we can start rendering.
Comment 6 Kenneth Rohde Christiansen 2012-10-11 13:33:27 PDT
Comment on attachment 165759 [details]
Patch

OK, r=me then. Anyway we should rename the class to follow current trunk style and inherit from the ViewportPageController, like Qt. But that is not directly related to this, but maybe something for you Yael?
Comment 7 WebKit Review Bot 2012-10-11 13:41:31 PDT
Comment on attachment 165759 [details]
Patch

Clearing flags on attachment: 165759

Committed r131091: <http://trac.webkit.org/changeset/131091>
Comment 8 WebKit Review Bot 2012-10-11 13:41:35 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Yael 2012-10-11 14:49:22 PDT
(In reply to comment #6)
> (From update of attachment 165759 [details])
> OK, r=me then. Anyway we should rename the class to follow current trunk style and inherit from the ViewportPageController, like Qt. But that is not directly related to this, but maybe something for you Yael?

Filed https://bugs.webkit.org/show_bug.cgi?id=99101 for that.
Comment 10 Gyuyoung Kim 2012-10-11 21:56:54 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Don't you need to set setRendererActive(false) in ewk_view_accelerated_compositing_mode_exit() ?
> 
> What is the benefit of doing that?
> We need to call setActive(true) to trigger the first renderNextFrame, so that we can start rendering.

First, I'm not expert for graphics area. To my experience, however, we have been set true/false in a pair of functions generally if there is needs to set specific setting. In this case, ewk_view_accelerated_compositing_mode_exit() might don't need to set setActive(false) because ewk_view_accelerated_compositing_mode_exit() will be called only once when existing program, right ?  If so, we might not call setActive(false) from a functionality perspective. But, IMO, it is not bad to call setActive(false) in _exit() from a pair of functions perspective. If the _exit() will be called many times, isn't it better or safer to call setActive(false) in _exit() ?

Anyway, patch was already landed. I'm just wondering about this.