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

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