Bug 152265

Summary:

Guard code that uses class LayerTreeHost with USE(COORDINATED_GRAPHICS) or USE(TEXTURE_MAPPER)

Product:

WebKit

Reporter:

Daniel Bates <dbates>

Component:

WebKit2

Assignee:

Daniel Bates <dbates>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cgarcia, darin, gyuyoung.kim, mcatanzaro, mrobinson, ossy, simon.fraser, thorton, yoon, zan

Priority:

Version:

WebKit Local Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	thorton: review+
Patch	none
Patch	none

Daniel Bates

Reported 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.

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 attachment proposed patch, testcase, etc.

Daniel Bates

Comment 1 2015-12-14 12:56:16 PST

Created attachment 267316 [details] Patch

Csaba Osztrogonác

Comment 2 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. )

Michael Catanzaro

Comment 3 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?)

Darin Adler

Comment 4 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.

Gwang Yoon Hwang

Comment 5 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.

Daniel Bates

Comment 6 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.

Daniel Bates

Comment 7 2015-12-15 10:03:33 PST

Created attachment 267377 [details] Patch

Daniel Bates

Comment 8 2015-12-15 10:40:59 PST

Committed r194108: <http://trac.webkit.org/changeset/194108>

Michael Catanzaro

Comment 9 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

Note You need to log in before you can comment on or make changes to this bug.