Bug 114842 – [WK2] The nonCompositedContentLayer does not show debug borders even if WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS is set to 1.

• Component: New Bugs
• Status: UNCONFIRMED

[Keonil Jeong]

The nonCompositedContentLayer does not show even if WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS is set to 1.

[Keonil Jeong]

Created attachment 198784 [details]
Patch

[WebKit Commit Bot]

Attachment 198784 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.cpp']" exit_code: 1
Source/WebKit2/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keonil Jeong]

Created attachment 198786 [details]
Patch

[Benjamin Poulain]

Comment on attachment 198786 [details]
Patch

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

> Source/WebKit2/ChangeLog:8
> +        WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS is set to 1.
> +        https://bugs.webkit.org/show_bug.cgi?id=114842
> +
> +        Reviewed by NOBODY (OOPS!).
> +

You changelog needs an explanation for the change:
-Why the previous code was incorrect.
-Why the new code is correct.

[Keonil Jeong]

Created attachment 198788 [details]
Patch

[Benjamin Poulain]

Comment on attachment 198788 [details]
Patch

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

> Source/WebKit2/ChangeLog:12
> +        The showDebugBorders() and showRepaintCounter() functions of settings
> +        return false even if WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS is set to 1
> +        when CoordinatedLayerTreeHost is created.
> +        This patch moves the position of functions call after settings is made.

This is a good start. But the first paragraph does not tell why showDebugBorders() and showRepaintCounter() return false at that point.

The second paragraph does not say why setNonCompositedContentsNeedDisplay() is the right place for the settings.
Of all places, setNonCompositedContentsNeedDisplay() is an odd call site for changing a layer configuration.

[Noam Rosenthal]

We need to get rid of the non composited contents layer altogether.
https://bugs.webkit.org/show_bug.cgi?id=110355

[Keonil Jeong]

Created attachment 198926 [details]
Patch

[Keonil Jeong]

(In reply to comment #7)
> We need to get rid of the non composited contents layer altogether.
> https://bugs.webkit.org/show_bug.cgi?id=110355

The non composited contents layer will be discarded? I just want to turn on the debug border properly for now.

[Keonil Jeong]

(In reply to comment #7)
> We need to get rid of the non composited contents layer altogether.
> https://bugs.webkit.org/show_bug.cgi?id=110355

I am still wondering about something. The non composited contents layer is still used.
So, the debug border does not show properly because the non composited contents layer is not removed for now.
Aren't these kind of patches patched separately before the bug Noam said is fixed ?

[Benjamin Poulain]

> I am still wondering about something. The non composited contents layer is still used.
> So, the debug border does not show properly because the non composited contents layer is not removed for now.
> Aren't these kind of patches patched separately before the bug Noam said is fixed ?

Yep, it can be fixed separately before https://bugs.webkit.org/show_bug.cgi?id=110355.

But personally, I am still not convinced the patch is correct. I let Noam decide what to do before reviewing.

[Noam Rosenthal]

Comment on attachment 198926 [details]
Patch

In Qt there is no problem like this, are you getting this problem in EFL?

[Keonil Jeong]

(In reply to comment #12)
> (From update of attachment 198926 [details])
> In Qt there is no problem like this, are you getting this problem in EFL?

I am using QtWebKit. The debug border and repaint counter are set during CoordinatedLayerTreeHost has been created. But, the settings is made in the updatePreferences() function of WebPage.cpp after the creation.
In my case, i can not see the debug border at the site like www.google.com. Please let me know if i am doing wrong approach.

[Darin Adler]

Comment on attachment 198926 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:-2457
> -        m_drawingArea->updatePreferences(store);

Why is it OK to remove this? Patch otherwise looks OK.

[Brent Fulgham]

Comment on attachment 198926 [details]
Patch

Sadly, this patch no longer applies cleanly. Can you please revise the patch for the current source tree?