Bug 152265 - Guard code that uses class LayerTreeHost with USE(COORDINATED_GRAPHICS) or USE(TEXTURE_MAPPER)
Summary: Guard code that uses class LayerTreeHost with USE(COORDINATED_GRAPHICS) or US...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-14 12:55 PST by Daniel Bates
Modified: 2015-12-15 12:02 PST (History)
10 users (show)

See Also:

Attachments
Patch (5.03 KB, patch)
2015-12-14 12:56 PST, Daniel Bates 		thorton: review+
Details | Formatted Diff | Diff
Patch (2.65 KB, patch)
2015-12-15 10:02 PST, Daniel Bates 		no flags Details | Formatted Diff | Diff
Patch (7.88 KB, patch)
2015-12-15 10:03 PST, Daniel Bates 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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