Bug 152265

Summary: Guard code that uses class LayerTreeHost with USE(COORDINATED_GRAPHICS) or USE(TEXTURE_MAPPER)
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebKit2Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, darin, gyuyoung.kim, mcatanzaro, mrobinson, ossy, simon.fraser, thorton, yoon, zan
Priority: P2    
Version: WebKit Local Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
thorton: review+
Patch
none
Patch none

Description Daniel Bates 2015-12-14 12:55:20 PST
We do not use class LayerTreeHost on Mac or iOS. We should not compile code that makes use of LayerTreeHost on these platforms.
Comment 1 Daniel Bates 2015-12-14 12:56:16 PST
Created attachment 267316 [details]
Patch
Comment 2 Csaba Osztrogonác 2015-12-15 02:13:17 PST
(In reply to comment #1)
> Created attachment 267316 [details]
> Patch

Adding include "LayerTreeContext.h" to WebPage.h fixes (at least) the EFL build.
( But I don't know if it is the right fix or not. )
Comment 3 Michael Catanzaro 2015-12-15 04:57:20 PST
Before committing, please check with Darin or Carlos Garcia -- I think they prefer to add include guards around the entire header file, rather than around the #include statement, right? (Or do I have this backwards?)
Comment 4 Darin Adler 2015-12-15 09:13:31 PST
Comment on attachment 267316 [details]
Patch

Typically we guard entire header files for clarity and simplicity. It’s also OK to guard include statements; that can speed up builds and sometimes makes the .cpp files slightly easier to understand. This should be covered in some project style guide document; if it’s not I’m surprised.
Comment 5 Gwang Yoon Hwang 2015-12-15 09:19:30 PST
(In reply to comment #2)
> (In reply to comment #1)
> > Created attachment 267316 [details]
> > Patch
> 
> Adding include "LayerTreeContext.h" to WebPage.h fixes (at least) the EFL
> build.
> ( But I don't know if it is the right fix or not. )

LayerTreeContext.h should be included in the WebPage.h regardless of this modification. It was included by LayerTreeHost indirectly.

I hope this patch adds LayerTreeContext.h to the WebPage.h to remove red buttons in ews.
Comment 6 Daniel Bates 2015-12-15 10:02:23 PST
Created attachment 267376 [details]
Patch

(Patch for EWS)

Updated patch based on the feedback from Csaba Osztrogonác, Michael Catanzaro, Darin Adler, and Gwang Yoon Hwang.
Comment 7 Daniel Bates 2015-12-15 10:03:33 PST
Created attachment 267377 [details]
Patch
Comment 8 Daniel Bates 2015-12-15 10:40:59 PST
Committed r194108: <http://trac.webkit.org/changeset/194108>
Comment 9 Michael Catanzaro 2015-12-15 12:02:16 PST
(In reply to comment #4) 
> Typically we guard entire header files for clarity and simplicity. It’s also
> OK to guard include statements; that can speed up builds and sometimes makes
> the .cpp files slightly easier to understand. This should be covered in some
> project style guide document; if it’s not I’m surprised.

I don't believe it's discussed on https://webkit.org/code-style-guidelines