Bug 53390 – Files missing ENABLE_VIDEO #ifdef

• Component: WebGL
• Status: RESOLVED FIXED

[Andrew Wason]

Created attachment 80584 [details]
wrap HTMLVideoElement references with ENABLE_VIDEO ifdef in WebGLRenderingContext.idl

Some files in Source/WebCore/html/canvas reference HTMLVideoElement without wrapping in ENABLE_VIDEO ifdef.

This is in svn r77075
(You will also need the patch from bug 53041 comment #15 applied so that --3d-canvas takes effect)


QTDIR=/usr/share/qt4 ./build-webkit --qt --makeargs="-j5" --debug --3d-canvas --no-video

Build fails with:

g++ -c -pipe -Wall -Wextra -Wreturn-type -fno-strict-aliasing -Wcast-align -Wchar-subscripts -Wformat-security -Wreturn-type -Wno-unused-parameter -Wno-sign-compare -Wno-switch -Wno-switch-enum -Wundef -Wmissing-noreturn -Winit-self -Werror -ffunction-sections -fdata-sections -g -D_REENTRANT -fPIC -DENABLE_WEBGL=1 -DBUILDING_QT__=1 -DQT_MAKEDLL -DBUILDING_WEBKIT -DBUILDING_JavaScriptCore -DBUILDING_WTF -DENABLE_JAVASCRIPT_DEBUGGER=1 -DENABLE_DATABASE=1 -DENABLE_EVENTSOURCE=1 -DENABLE_OFFLINE_WEB_APPLICATIONS=1 -DENABLE_DOM_STORAGE=1 -DENABLE_ICONDATABASE=1 -DENABLE_CHANNEL_MESSAGING=1 -DENABLE_DIRECTORY_UPLOAD=0 -DENABLE_FILE_SYSTEM=0 -DENABLE_SQLITE=1 -DENABLE_DASHBOARD_SUPPORT=0 -DENABLE_FILTERS=1 -DENABLE_XPATH=1 -DENABLE_WCSS=0 -DENABLE_WML=0 -DENABLE_SHARED_WORKERS=1 -DENABLE_WORKERS=1 -DENABLE_XHTMLMP=0 -DENABLE_DATAGRID=0 -DENABLE_METER_TAG=1 -DENABLE_PROGRESS_TAG=1 -DENABLE_BLOB=1 -DENABLE_NOTIFICATIONS=1 -DENABLE_IMAGE_RESIZER=0 -DENABLE_INPUT_SPEECH=0 -DENABLE_INSPECTOR=1 -DENABLE_3D_RENDERING=1 -DENABLE_WEB_AUDIO=0 -DENABLE_SVG=1 -DENABLE_SVG_FONTS=1 -DENABLE_SVG_FOREIGN_OBJECT=1 -DENABLE_SVG_ANIMATION=1 -DENABLE_SVG_AS_IMAGE=1 -DENABLE_SVG_USE=1 -DENABLE_DATALIST=1 -DENABLE_TILED_BACKING_STORE=1 -DENABLE_NETSCAPE_PLUGIN_API=1 -DENABLE_WEB_SOCKETS=1 -DENABLE_WEB_TIMING=1 -DENABLE_XSLT=1 -DENABLE_QT_BEARER=1 -DENABLE_TOUCH_EVENTS=1 -DENABLE_VIDEO=0 -DXP_UNIX -DENABLE_NETSCAPE_PLUGIN_METADATA_CACHE=1 -DQT_XMLPATTERNS_LIB -DQT_OPENGL_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_CORE_LIB -DQT_SHARED -I/usr/share/qt4/mkspecs/linux-g++ -I../../../Source/WebCore -I/usr/include/qt4/QtCore -I/usr/include/qt4/QtNetwork -I/usr/include/qt4/QtGui -I/usr/include/qt4/QtOpenGL -I/usr/include/qt4/QtXmlPatterns -I/usr/include/qt4 -I../../../Source/WebCore/bridge/qt -I../../../Source/WebCore/page/qt -I../../../Source/WebCore/platform/graphics/qt -I../../../Source/WebCore/platform/network/qt -I../../../Source/WebCore/platform/qt -I../../../Source/WebKit/qt/Api -I../../../Source/WebKit/qt/WebCoreSupport -I../../../Source/WebCore -I../../../Source/WebCore/accessibility -I../../../Source/WebCore/bindings -I../../../Source/WebCore/bindings/generic -I../../../Source/WebCore/bridge -I../../../Source/WebCore/css -I../../../Source/WebCore/dom -I../../../Source/WebCore/dom/default -I../../../Source/WebCore/editing -I../../../Source/WebCore/fileapi -I../../../Source/WebCore/history -I../../../Source/WebCore/html -I../../../Source/WebCore/html/canvas -I../../../Source/WebCore/html/parser -I../../../Source/WebCore/html/shadow -I../../../Source/WebCore/inspector -I../../../Source/WebCore/loader -I../../../Source/WebCore/loader/appcache -I../../../Source/WebCore/loader/archive -I../../../Source/WebCore/loader/cache -I../../../Source/WebCore/loader/icon -I../../../Source/WebCore/mathml -I../../../Source/WebCore/notifications -I../../../Source/WebCore/page -I../../../Source/WebCore/page/animation -I../../../Source/WebCore/platform -I../../../Source/WebCore/platform/animation -I../../../Source/WebCore/platform/audio -I../../../Source/WebCore/platform/graphics -I../../../Source/WebCore/platform/graphics/filters -I../../../Source/WebCore/platform/graphics/transforms -I../../../Source/WebCore/platform/image-decoders -I../../../Source/WebCore/platform/mock -I../../../Source/WebCore/platform/network -I../../../Source/WebCore/platform/sql -I../../../Source/WebCore/platform/text -I../../../Source/WebCore/platform/text/transcoder -I../../../Source/WebCore/plugins -I../../../Source/WebCore/rendering -I../../../Source/WebCore/rendering/mathml -I../../../Source/WebCore/rendering/style -I../../../Source/WebCore/rendering/svg -I../../../Source/WebCore/storage -I../../../Source/WebCore/svg -I../../../Source/WebCore/svg/animation -I../../../Source/WebCore/svg/graphics -I../../../Source/WebCore/svg/graphics/filters -I../../../Source/WebCore/svg/properties -I../../../Source/WebCore/webaudio -I../../../Source/WebCore/websockets -I../../../Source/WebCore/wml -I../../../Source/WebCore/workers -I../../../Source/WebCore/xml -Igenerated -I../../../Source/WebCore/bridge/jsc -I../../../Source/WebCore/bindings/js -I/home/aw/Projects/foundation/WebKit/Source/WebCore/bindings/js/specialization -I../../../Source/WebCore/bridge/c -I../../../Source/JavaScriptCore -I../../../Source -I../../../Source/ThirdParty -I../../../Source/JavaScriptCore/assembler -I../../../Source/JavaScriptCore/bytecode -I../../../Source/JavaScriptCore/bytecompiler -I../../../Source/JavaScriptCore/debugger -I../../../Source/JavaScriptCore/interpreter -I../../../Source/JavaScriptCore/jit -I../../../Source/JavaScriptCore/parser -I../../../Source/JavaScriptCore/pcre -I../../../Source/JavaScriptCore/profiler -I../../../Source/JavaScriptCore/runtime -I../../../Source/JavaScriptCore/wtf -I../../../Source/JavaScriptCore/wtf/gobject -I/home/aw/Projects/foundation/WebKit/Source/JavaScriptCore/wtf/symbian -I../../../Source/JavaScriptCore/wtf/unicode -I../../../Source/JavaScriptCore/yarr -I../../../Source/JavaScriptCore/API -I../../../Source/JavaScriptCore/ForwardingHeaders -I../JavaScriptCore/generated -I../include/QtWebKit -I/usr/src/3rdparty/sqlite/ -I/usr/X11R6/include -I/usr/X11R6/include -I. -I../../../Source/WebCore -I. -o obj/debug/JSWebGLRenderingContext.o generated/JSWebGLRenderingContext.cpp


generated/JSWebGLRenderingContext.cpp: In function ‘void* WebCore::jsWebGLRenderingContextPrototypeFunctionTexImage2D5(JSC::ExecState*)’:
generated/JSWebGLRenderingContext.cpp:3012: error: ‘JSHTMLVideoElement’ has not been declared
generated/JSWebGLRenderingContext.cpp:3014: error: ‘toHTMLVideoElement’ was not declared in this scope
generated/JSWebGLRenderingContext.cpp: In function ‘void* WebCore::jsWebGLRenderingContextPrototypeFunctionTexImage2D(JSC::ExecState*)’:
generated/JSWebGLRenderingContext.cpp:3033: error: ‘JSHTMLVideoElement’ has not been declared
generated/JSWebGLRenderingContext.cpp: In function ‘void* WebCore::jsWebGLRenderingContextPrototypeFunctionTexSubImage2D5(JSC::ExecState*)’:
generated/JSWebGLRenderingContext.cpp:3218: error: ‘JSHTMLVideoElement’ has not been declared
generated/JSWebGLRenderingContext.cpp:3220: error: ‘toHTMLVideoElement’ was not declared in this scope
generated/JSWebGLRenderingContext.cpp: In function ‘void* WebCore::jsWebGLRenderingContextPrototypeFunctionTexSubImage2D(JSC::ExecState*)’:
generated/JSWebGLRenderingContext.cpp:3239: error: ‘JSHTMLVideoElement’ has not been declared
make[1]: *** [obj/debug/JSWebGLRenderingContext.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory `/home/aw/Projects/foundation/WebKit/WebKitBuild/Debug/WebCore'
make: *** [sub-WebCore-make_default-ordered] Error 2

[Andrew Wason]

Created attachment 80642 [details]
wrap HTMLVideoElement references with ENABLE_VIDEO ifdef in WebGLRenderingContext

regenerate patch using prepare-ChangeLog and svn-create-patch

[Patrick R. Gansterer]

Comment on attachment 80642 [details]
wrap HTMLVideoElement references with ENABLE_VIDEO ifdef in WebGLRenderingContext

View in context: https://bugs.webkit.org/attachment.cgi?id=80642&action=review

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2990
> +#if defined(ENABLE_VIDEO) && ENABLE_VIDEO

Please use the ENABLE macro in non-idl files. This should be written as ENABLE(VIDEO).

[Andrew Wason]

Created attachment 81738 [details]
wrap HTMLVideoElement references with ENABLE(VIDEO) ifdef in WebGLRenderingContext

Use ENABLE(VIDEO) in non-idl file.

[David Levin]

Comment on attachment 81738 [details]
wrap HTMLVideoElement references with ENABLE(VIDEO) ifdef in WebGLRenderingContext

View in context: https://bugs.webkit.org/attachment.cgi?id=81738&action=review

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2988
>  PassRefPtr<Image> WebGLRenderingContext::videoFrameToImage(HTMLVideoElement* video)

Can't you put the #if ENABLE(VIDEO) on the outside of this method? (In the header too.)

Ditto for the other instances in this file.

[Andrew Wason]

Created attachment 82136 [details]
wrap HTMLVideoElement references with ENABLE(VIDEO) ifdef in WebGLRenderingContext

Put #ifdefs outside methods, and in header.

[David Levin]

Comment on attachment 82136 [details]
wrap HTMLVideoElement references with ENABLE(VIDEO) ifdef in WebGLRenderingContext

View in context: https://bugs.webkit.org/attachment.cgi?id=82136&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests.

ok, note that typically this should say explain why there are no new test (or have the tests).

For example, "No new functionality exposed so no new tests."

[WebKit Commit Bot]

Comment on attachment 82136 [details]
wrap HTMLVideoElement references with ENABLE(VIDEO) ifdef in WebGLRenderingContext

Clearing flags on attachment: 82136

Committed r78355: <http://trac.webkit.org/changeset/78355>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.