Bug 138305 – [EFL] Improve previous temporary fix against drawing (0,0) position before rendering content of next web page

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Gyuyoung Kim]

In EFL port, there was to draw current content at (0, 0) scroll position before drawing next page. r170243 fixed it using a flag for the occasion though, we should fix it with more correct way.

[Gyuyoung Kim]

Created attachment 240838 [details]
WIP

[Gyuyoung Kim]

Created attachment 240896 [details]
Patch

[Gyuyoung Kim]

CC'ing Ossy.

[Ryuan Choi]

Created attachment 240905 [details]
another_approach

[Ryuan Choi]

(In reply to comment #4)
> Created attachment 240905 [details]
> another_approach

Dear Gyuyoung,
As mentioned in IRC,

I uplodated my approach based on analysis.

Feel free to check this.
I hope that this is helpfull for you

[Ryuan Choi]

Created attachment 240907 [details]
another_approach2

[Gyuyoung Kim]

Comment on attachment 240907 [details]
another_approach2

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

> Source/WebKit2/ChangeLog:13
> +        rendered the results of first visually not empty layout.

I modify this description a little.

"WebKit2 has freezed layer tree until frame load completion since r101838. In EFL port, we unfreeze it when PageViewportController::pageTransitionViewportReady() is called though, UIProcess on EFL port updates cairo surface during the freezing time of layer tree. Thus UIProcess should not update the layer trees until the layerTreeState is unfrozen."

> Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.h:107
> +    enum class PageState {

I think that it would be nicer if we use variable or enum name based on "layerTreeStateIsFrozen" to improve our code readability. Under my understanding, this state is depended on m_layerTreeStateIsFrozen state.

[Ryuan Choi]

Created attachment 240919 [details]
Patch

[Ryuan Choi]

Comment on attachment 240907 [details]
another_approach2

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

>> Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.h:107
>> +    enum class PageState {
> 
> I think that it would be nicer if we use variable or enum name based on "layerTreeStateIsFrozen" to improve our code readability. Under my understanding, this state is depended on m_layerTreeStateIsFrozen state.

After considered more, boolean looks enough like your original approach.

I changed this as boolean member, m_layerTreeStateIsFrozen

[Gyuyoung Kim]

Comment on attachment 240919 [details]
Patch

LGTM, r=me.

[Gyuyoung Kim]

Before landing, could you check if there is any side effect ? It looks black screen is drawn as soon as the MiniBrowser is shown.

[Gyuyoung Kim]

Comment on attachment 240919 [details]
Patch

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

> Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp:273
> +    if (!m_layerTreeStateIsFrozen)

I'm not sure if we can block during the m_layerTreeStateIsFrozen is true. If we block to update display, can black screen be shown during the period ?

[Ryuan Choi]

(In reply to comment #12)
> Comment on attachment 240919 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240919&action=review
> 
> > Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp:273
> > +    if (!m_layerTreeStateIsFrozen)
> 
> I'm not sure if we can block during the m_layerTreeStateIsFrozen is true. If
> we block to update display, can black screen be shown during the period ?

Well, I can't imagine that scenario.
Are you considering with direct rendering feature?

In that scenario, previous screen will be used because we didn't remove it.

[Gyuyoung Kim]

(In reply to comment #13)
> (In reply to comment #12)
> > Comment on attachment 240919 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=240919&action=review
> > 
> > > Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp:273
> > > +    if (!m_layerTreeStateIsFrozen)
> > 
> > I'm not sure if we can block during the m_layerTreeStateIsFrozen is true. If
> > we block to update display, can black screen be shown during the period ?
> 
> Well, I can't imagine that scenario.
> Are you considering with direct rendering feature?
> 
> In that scenario, previous screen will be used because we didn't remove it.

This problem occurs on current WebKit EFL MiniBrowser. Other previous commit looks like to made this problem.

[WebKit Commit Bot]

Comment on attachment 240919 [details]
Patch

Clearing flags on attachment: 240919

Committed r175669: <http://trac.webkit.org/changeset/175669>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.