Avoid exposing gl bindings through cc public API headers

Avoid exposing gl bindings through cc public API headers

The cc library uses chromium's gles2 bindings in third_party/khronos/... to bind
to GL for rendering. However, users of cc may not be able to pull these headers
in when including cc API - for instance, on mac any code that includes
content/common/view_messages.h pulls in the Apple CGL api through npapi.h. On
the 10.6 SDK these headers collide with chromium's gl headers.

This avoids exposing gl2.h includes via layer_tree_host.h, mostly by not including
prioritized_resource_manager.h directly in l_t_h.h.

BUG=156175
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175554

[jamesr, 2013-01-06 23:00:29]

Example failure from patch https://chromiumcodereview.appspot.com/11575049/,
which adds an include to cc/layer_tree_host.h from
content/renderer/render_widget.cc:

FAILED: clang++ -MMD -MF obj/content/renderer/content_renderer.render_widget.o.d
-DCONTENT_IMPLEMENTATION -DADDRESS_SANITIZER -DCHROMIUM_BUILD
-DUSE_LIBJPEG_TURBO=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_REMOTING=1
-DENABLE_WEBRTC=1 -DENABLE_PEPPER_THREADING -DENABLE_CONFIGURATION_POLICY
-DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_HIDPI=1 -DENABLE_GPU=1
-DENABLE_EGLIMAGE=1 -DUSE_SKIA=1 -DENABLE_TASK_MANAGER=1 -DENABLE_WEB_INTENTS=1
-DENABLE_EXTENSIONS=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PLUGINS=1
-DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_BACKGROUND=1
-DENABLE_AUTOMATION=1 -DENABLE_GOOGLE_NOW=1 -DENABLE_LANGUAGE_DETECTION=1
-DENABLE_PRINTING=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1
-DSK_BUILD_NO_IMAGE_ENCODE -DSK_DEFERRED_CANVAS_USES_GPIPE=1
'-DGR_GL_CUSTOM_SETUP_HEADER="GrGLConfig_chrome.h"'
-DGR_AGGRESSIVE_SHADER_OPTS=1 -DSK_USE_POSIX_THREADS -DHYPHEN_CHROME_CLIENT
-DU_USING_ICU_NAMESPACE=0 -DU_STATIC_IMPLEMENTATION -DFEATURE_ENABLE_SSL
-DFEATURE_ENABLE_VOICEMAIL -DEXPAT_RELATIVE_PATH -DGTEST_RELATIVE_PATH
-DJSONCPP_RELATIVE_PATH -DNO_MAIN_THREAD_WRAPPING -DNO_SOUND_SYSTEM -DOSX
-DPOSIX -DXML_STATIC -DNO_NSPR_10_SUPPORT -DNSPR_STATIC -DNSS_ENABLE_ECC
-DNSS_STATIC -DNSS_USE_STATIC_LIBS -DUSE_UTIL_DIRECTLY -D__STDC_CONSTANT_MACROS
-D__STDC_FORMAT_MACROS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0
-D_FORTIFY_SOURCE=2 -I../../third_party/icu/public/i18n
-I../../third_party/icu/public/common -I../..
-I../../third_party/WebKit/Source/Platform/chromium -I../../third_party/khronos
-I../../gpu -Igen/content -I../../skia/config -I../../third_party/skia/src/core
-I../../third_party/skia/include/config -I../../third_party/skia/include/core
-I../../third_party/skia/include/effects -I../../third_party/skia/include/pdf
-I../../third_party/skia/include/gpu -I../../third_party/skia/include/gpu/gl
-I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports
-I../../third_party/skia/include/utils -I../../skia/ext
-I../../third_party/skia/include/utils/mac -I../../third_party/hyphen
-I../../third_party/libjingle/overrides -I../../third_party/libjingle/source
-I../../testing/gtest/include -I../../third_party -I../../third_party/webrtc
-I../../third_party/expat/files/lib
-I../../third_party/jsoncpp/overrides/include
-I../../third_party/jsoncpp/source/include -I../../third_party/npapi
-I../../third_party/npapi/bindings
-I../../third_party/WebKit/Source/Platform/chromium -Igen/webcore_headers
-I../../v8/include -I../../third_party/webrtc/system_wrappers/interface
-I../../third_party/webrtc/modules/interface
-I../../third_party/webrtc/modules/audio_device/include
-I../../third_party/webrtc/video_engine/include
-I../../third_party/webrtc/voice_engine/include
-I../../third_party/nss/mozilla/nsprpub/pr/include
-I../../third_party/nss/mozilla/nsprpub/lib/ds
-I../../third_party/nss/mozilla/nsprpub/lib/libc/include
-I../../third_party/nss/mozilla/security/nss/lib/base
-I../../third_party/nss/mozilla/security/nss/lib/certdb
-I../../third_party/nss/mozilla/security/nss/lib/certhigh
-I../../third_party/nss/mozilla/security/nss/lib/cryptohi
-I../../third_party/nss/mozilla/security/nss/lib/dev
-I../../third_party/nss/mozilla/security/nss/lib/freebl
-I../../third_party/nss/mozilla/security/nss/lib/freebl/ecl
-I../../third_party/nss/mozilla/security/nss/lib/nss
-I../../third_party/nss/mozilla/security/nss/lib/pk11wrap
-I../../third_party/nss/mozilla/security/nss/lib/pkcs7
-I../../third_party/nss/mozilla/security/nss/lib/pki
-I../../third_party/nss/mozilla/security/nss/lib/smime
-I../../third_party/nss/mozilla/security/nss/lib/softoken
-I../../third_party/nss/mozilla/security/nss/lib/util -isysroot
/Developer/SDKs/MacOSX10.6.sdk -O3 -gdwarf-2 -fvisibility=hidden -Werror
-Wnewline-eof -mmacosx-version-min=10.6 -arch i386 -Wall -Wendif-labels -Wextra
-Wno-unused-parameter -Wno-missing-field-initializers -Wheader-hygiene
-Wno-c++11-narrowing -Wno-reserved-user-defined-literal -Wno-char-subscripts
-Wno-unused-function -Wno-covered-switch-default -Wexit-time-destructors
-fno-rtti -fno-exceptions -fvisibility-inlines-hidden -fno-threadsafe-statics
-fsanitize=address -w -Xclang -load -Xclang
/Volumes/data/b/build/slave/Mac_ASAN_Builder/build/src/tools/clang/scripts/../../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib
-Xclang -add-plugin -Xclang find-bad-constructs -Xclang
-plugin-arg-find-bad-constructs -Xclang skip-virtuals-in-implementations
-fcolor-diagnostics -fno-strict-aliasing -gline-tables-only -std=gnu++11  -c
../../content/renderer/render_widget.cc -o
obj/content/renderer/content_renderer.render_widget.o
In file included from ../../content/renderer/render_widget.cc:21:
In file included from ../../content/common/view_messages.h:14:
In file included from ../../content/common/content_param_traits.h:19:
In file included from ../../webkit/glue/npruntime_util.h:8:
In file included from ../../third_party/npapi/bindings/npruntime.h:37:
In file included from ../../third_party/npapi/bindings/npapi.h:105:
In file included from
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/OpenGL.framework/Headers/OpenGL.h:10:
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/OpenGL.framework/Headers/gl.h:2765:13:error:
conflicting types for 'GLES2TexImage2D'
extern void glTexImage2D (GLenum target, GLint level, GLenum internalformat,
GLsizei width, GLsizei height, GLint border, GLenum format, GLenum type, const
GLvoid *pixels);
            ^
../../gpu/GLES2/gl2chromium_autogen.h:121:22: note: expanded from macro
'glTexImage2D'
#define glTexImage2D GLES2_GET_FUN(TexImage2D)
                     ^
../../gpu/GLES2/gl2chromium.h:19:29: note: expanded from macro 'GLES2_GET_FUN'
#define GLES2_GET_FUN(name) GLES2 ## name
                            ^
<scratch space>:165:1: note: expanded from here
GLES2TexImage2D
^
../../third_party/khronos/GLES2/gl2.h:580:37: note: previous declaration is here
GL_APICALL void         GL_APIENTRY glTexImage2D (GLenum target, GLint level,
GLint internalformat, GLsizei width, GLsizei height, GLint border, GLenum
format, GLenum type, const GLvoid* pixels);

                                    ^
../../gpu/GLES2/gl2chromium_autogen.h:121:22: note: expanded from macro
'glTexImage2D'
#define glTexImage2D GLES2_GET_FUN(TexImage2D)
                     ^
../../gpu/GLES2/gl2chromium.h:19:29: note: expanded from macro 'GLES2_GET_FUN'
#define GLES2_GET_FUN(name) GLES2 ## name
                            ^
<scratch space>:138:1: note: expanded from here
GLES2TexImage2D
^
1 error generated.

[jamesr, 2013-01-06 23:42:15]

This also makes cc/*layer.h safe to include without picking up
third_party/khronos/GLES2/gl2.h.  It's mostly out-of-lining, replacing includes
in headers with forward declarations in headers and includes in .cc and removing
unnecessary includes.  All of these are good things to do anyway.  I also had to
remove a couple GLenum instances with unsigned.  Since using GL or not is an
implementation detail of cc, having GLenum exposed in public headers seems like
a slightly bad thing anyway.

Another approach that I imagine we'll want to do some day is to separate API
headers (probably just vtables) from implementations, but we can get there later
IMO.

[danakj, 2013-01-07 14:48:03]

I don't mind the idea of replacing all GLenum with unsigned (or maybe our own
typedef?)

How can we prevent this from happening again? How about a unittest file that
includes a GL header that would conflict (or defines some stuff itself to
conflict) and then #includes our public API headers?

https://codereview.chromium.org/11794019/diff/7001/cc/layer_updater.cc
File cc/layer_updater.cc (right):

https://codereview.chromium.org/11794019/diff/7001/cc/layer_updater.cc#newcode6
cc/layer_updater.cc:6: #include "cc/prioritized_resource.h"
stylebot: 1 blank line after layer_updater.h

https://codereview.chromium.org/11794019/diff/7001/cc/layer_updater.h
File cc/layer_updater.h (right):

https://codereview.chromium.org/11794019/diff/7001/cc/layer_updater.h#newcode11
cc/layer_updater.h:11: #include "ui/gfx/rect.h"
not sure why this can't be forward declared here still?

[jamesr, 2013-01-07 17:43:45]

On 2013/01/07 14:48:03, danakj wrote:
> I don't mind the idea of replacing all GLenum with unsigned (or maybe our own
> typedef?)
> 
> How can we prevent this from happening again? How about a unittest file that
> includes a GL header that would conflict (or defines some stuff itself to
> conflict) and then #includes our public API headers?

Pretty soon (specifically once https://chromiumcodereview.appspot.com/11575049/
lands), content won't compile on mac if someone breaks this.

> 
> https://codereview.chromium.org/11794019/diff/7001/cc/layer_updater.cc
> File cc/layer_updater.cc (right):
> 
>
https://codereview.chromium.org/11794019/diff/7001/cc/layer_updater.cc#newcode6
> cc/layer_updater.cc:6: #include "cc/prioritized_resource.h"
> stylebot: 1 blank line after layer_updater.h
> 
> https://codereview.chromium.org/11794019/diff/7001/cc/layer_updater.h
> File cc/layer_updater.h (right):
> 
>
https://codereview.chromium.org/11794019/diff/7001/cc/layer_updater.h#newcode11
> cc/layer_updater.h:11: #include "ui/gfx/rect.h"
> not sure why this can't be forward declared here still?

Done and done

[danakj, 2013-01-07 17:46:06]

Ok, cool. LGTM

[commit-bot: I haz the power, 2013-01-07 18:51:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11794019/11004

[jamesr, 2013-01-07 21:10:40]

On 2013/01/07 14:48:03, danakj wrote:
> How can we prevent this from happening again? How about a unittest file that
> includes a GL header that would conflict (or defines some stuff itself to
> conflict) and then #includes our public API headers?

Another option would be to move public API headers to a separate directory, make
them completely independent of implementation headers, and use the DEPS to
enforce that they don't include anything from third_party/khronos/ or from cc/. 
Seems like a bit too much boilerplate for now, but maybe not.

[commit-bot: I haz the power, 2013-01-07 21:16:35]

Retried try job too often on win_aura for step(s) content_browsertests

[jamesr, 2013-01-07 21:17:30]

On 2013/01/07 21:16:35, I haz the power (commit-bot) wrote:
> Retried try job too often on win_aura for step(s) content_browsertests

Error from that bot:

Traceback (most recent call last):
File
"E:\b\build\slave\win_aura\build\src\third_party\WebKit\Tools\Scripts\new-run-webkit-httpd",
line 99, in <module>
main()
File
"E:\b\build\slave\win_aura\build\src\third_party\WebKit\Tools\Scripts\new-run-webkit-httpd",
line 95, in main
run(options)
File
"E:\b\build\slave\win_aura\build\src\third_party\WebKit\Tools\Scripts\new-run-webkit-httpd",
line 72, in run
httpd.start()
File
"E:\b\build\slave\win_aura\build\src\third_party\WebKit\Tools\Scripts\webkitpy\layout_tests\servers\http_server_base.py",
line 78, in start
self._pid = int(self._filesystem.read_text_file(self._pid_file))
UnicodeEncodeError: 'decimal' codec can't encode character u'\x00' in position
0: invalid decimal Unicode string
[6064:2764:0107/131421:1825040:INFO:layout_browsertest.cc(153)] Navigating to
URL http://127.0.0.1:53918/xmlhttprequest/workers/close.html and blocking.
[6064:2764:0107/131421:1825055:ERROR:desktop_root_window_host_win.cc(590)] NOT
IMPLEMENTED
[6064:2764:0107/131425:1828362:INFO:layout_browsertest.cc(158)] Navigation
completed.
e:\b\build\slave\win_aura\build\src\content\test\layout_browsertest.cc(187):
error: Value of: actual_text
Actual: "layer at (0,0) size 780x524\n  RenderView at (0,0) size 780x524\nlayer
at (0,0) size 780x524\n  RenderBlock {HTML} at (0,0) size 780x524\n   
RenderBody {BODY} at (8,8) size 764x508\n"
Expected: expected_text
Which is: "Tests invoking close() in the middle of an XMLHttpRequest.\n\nPASS:
Async test\nPASS: sync test\nDONE\n\n"
[6064:2764:0107/131425:1828394:INFO:layout_browsertest.cc(153)] Navigating to
URL http://127.0.0.1:53918/xmlhttprequest/workers/shared-worker-close.html and
blocking.
[6064:2764:0107/131425:1828394:ERROR:desktop_root_window_host_win.cc(590)] NOT
IMPLEMENTED
[6064:2764:0107/131428:1831389:INFO:layout_browsertest.cc(158)] Navigation
completed.
e:\b\build\slave\win_aura\build\src\content\test\layout_browsertest.cc(187):
error: Value of: actual_text
Actual: "layer at (0,0) size 780x524\n  RenderView at (0,0) size 780x524\nlayer
at (0,0) size 780x524\n  RenderBlock {HTML} at (0,0) size 780x524\n   
RenderBody {BODY} at (8,8) size 764x508\n"
Expected: expected_text
Which is: "Tests invoking close() in the middle of an XMLHttpRequest in a shared
worker.\n\nPASS: Async test\nPASS: sync test\nDONE\n\n"
[6064:2764:0107/131428:1831420:INFO:layout_browsertest.cc(153)] Navigating to
URL
http://127.0.0.1:53918/xmlhttprequest/workers/shared-worker-xhr-file-not-foun...
and blocking.
[6064:2764:0107/131428:1831436:ERROR:desktop_root_window_host_win.cc(590)] NOT
IMPLEMENTED
[6064:2764:0107/131431:1834431:INFO:layout_browsertest.cc(158)] Navigation
completed.
e:\b\build\slave\win_aura\build\src\content\test\layout_browsertest.cc(187):
error: Value of: actual_text
Actual: "layer at (0,0) size 780x524\n  RenderView at (0,0) size 780x524\nlayer
at (0,0) size 780x524\n  RenderBlock {HTML} at (0,0) size 780x524\n   
RenderBody {BODY} at (8,8) size 764x508\n"
Expected: expected_text
Which is: "Request a file that isn't present on the server in XMLHttpRequest in
a shared worker. If this test passes, there should be a single PASS
below.\n\nPASS\n"
[6064:2764:0107/131431:1834462:INFO:layout_browsertest.cc(153)] Navigating to
URL
http://127.0.0.1:53918/xmlhttprequest/workers/xmlhttprequest-file-not-found.html
and blocking.
[6064:2764:0107/131431:1834462:ERROR:desktop_root_window_host_win.cc(590)] NOT
IMPLEMENTED
[6064:2764:0107/131434:1837504:INFO:layout_browsertest.cc(158)] Navigation
completed.
e:\b\build\slave\win_aura\build\src\content\test\layout_browsertest.cc(187):
error: Value of: actual_text
Actual: "layer at (0,0) size 780x524\n  RenderView at (0,0) size 780x524\nlayer
at (0,0) size 780x524\n  RenderBlock {HTML} at (0,0) size 780x524\n   
RenderBody {BODY} at (8,8) size 764x508\n"
Expected: expected_text
Which is: "Request a file that isn't present on the server in XMLHttpRequest in
a worker. If this test passes, there should be a single PASS below.\n\nPASS\n"
Traceback (most recent call last):
File
"E:\b\build\slave\win_aura\build\src\third_party\WebKit\Tools\Scripts\new-run-webkit-httpd",
line 99, in <module>
main()
File
"E:\b\build\slave\win_aura\build\src\third_party\WebKit\Tools\Scripts\new-run-webkit-httpd",
line 95, in main
run(options)
File
"E:\b\build\slave\win_aura\build\src\third_party\WebKit\Tools\Scripts\new-run-webkit-httpd",
line 74, in run
httpd.stop()
File
"E:\b\build\slave\win_aura\build\src\third_party\WebKit\Tools\Scripts\webkitpy\layout_tests\servers\http_server_base.py",
line 98, in stop
actual_pid = int(self._filesystem.read_text_file(self._pid_file))
UnicodeEncodeError: 'decimal' codec can't encode character u'\x00' in position
0: invalid decimal Unicode string


wheeee

[commit-bot: I haz the power, 2013-01-07 21:19:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11794019/11004

[commit-bot: I haz the power, 2013-01-08 00:00:03]

Retried try job too often on win_aura for step(s) interactive_ui_tests

[commit-bot: I haz the power, 2013-01-08 01:14:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11794019/11004

[commit-bot: I haz the power, 2013-01-08 01:14:43]

Failed to apply patch for cc/layer_tree_host_unittest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/layer_tree_host_unittest.cc
  Hunk #1 FAILED at 12.
  1 out of 1 hunk FAILED -- saving rejects to file
cc/layer_tree_host_unittest.cc.rej

Patch:       cc/layer_tree_host_unittest.cc
Index: cc/layer_tree_host_unittest.cc
diff --git a/cc/layer_tree_host_unittest.cc b/cc/layer_tree_host_unittest.cc
index
e8d86ccb35d3274c97d3b8ef15313fd16c53282f..c152ba364e6b14854a834901cc10cc4084d720d7
100644
--- a/cc/layer_tree_host_unittest.cc
+++ b/cc/layer_tree_host_unittest.cc
@@ -12,6 +12,7 @@
 #include "cc/layer_tree_host_impl.h"
 #include "cc/layer_tree_impl.h"
 #include "cc/output_surface.h"
+#include "cc/prioritized_resource.h"
 #include "cc/resource_update_queue.h"
 #include "cc/single_thread_proxy.h"
 #include "cc/test/fake_content_layer.h"

[commit-bot: I haz the power, 2013-01-08 01:20:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11794019/7007

[commit-bot: I haz the power, 2013-01-08 17:13:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11794019/7007

[commit-bot: I haz the power, 2013-01-08 18:16:18]

Change committed as 175554