Bug 134679 - Fix the !ENABLE(FILTERS) && !ENABLE(CSS_FILTERS) build after r167497
Summary: Fix the !ENABLE(FILTERS) && !ENABLE(CSS_FILTERS) build after r167497
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 131797
  Show dependency treegraph
 
Reported: 2014-07-07 03:48 PDT by Tibor Mészáros
Modified: 2014-07-14 14:29 PDT (History)
10 users (show)

See Also:


Attachments
Patch (1.10 KB, patch)
2014-07-07 03:52 PDT, Tibor Mészáros
darin: review-
darin: commit-queue-
Details | Formatted Diff | Diff
Patch v2 (1.53 KB, patch)
2014-07-14 09:03 PDT, Tibor Mészáros
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tibor Mészáros 2014-07-07 03:48:47 PDT
Build fails with --no-filters --no-css-filters option.

Command line:
Tools/Scripts/build-webkit --efl --no-filters --no-css-filters

Error messages:

Linking CXX shared library ../../lib/libwebcore_efl.so
CMakeFiles/WebCore.dir/html/ImageData.cpp.o: In function `WebCore::ImageData::ImageData(WebCore::IntSize const&)':
ImageData.cpp:(.text._ZN7WebCore9ImageDataC2ERKNS_7IntSizeE+0x2b): undefined reference to `JSC::GenericTypedArrayView<JSC::Uint8ClampedAdaptor>::createUninitialized(unsigned int)'
CMakeFiles/WebCore.dir/platform/graphics/cairo/ImageBufferCairo.cpp.o: In function `WTF::PassRefPtr<JSC::GenericTypedArrayView<JSC::Uint8ClampedAdaptor> > WebCore::getImageData<(WebCore::Multiply)1>(WebCore::IntRect const&, WebCore::ImageBufferData const&, WebCore::IntSize const&)':
ImageBufferCairo.cpp:(.text._ZN7WebCore12getImageDataILNS_8MultiplyE1EEEN3WTF10PassRefPtrIN3JSC21GenericTypedArrayViewINS4_19Uint8ClampedAdaptorEEEEERKNS_7IntRectERKNS_15ImageBufferDataERKNS_7IntSizeE[_ZN7WebCore12getImageDataILNS_8MultiplyE1EEEN3WTF10PassRefPtrIN3JSC21GenericTypedArrayViewINS4_19Uint8ClampedAdaptorEEEEERKNS_7IntRectERKNS_15ImageBufferDataERKNS_7IntSizeE]+0x41): undefined reference to `JSC::GenericTypedArrayView<JSC::Uint8ClampedAdaptor>::createUninitialized(unsigned int)'
CMakeFiles/WebCore.dir/platform/graphics/cairo/ImageBufferCairo.cpp.o: In function `WTF::PassRefPtr<JSC::GenericTypedArrayView<JSC::Uint8ClampedAdaptor> > WebCore::getImageData<(WebCore::Multiply)0>(WebCore::IntRect const&, WebCore::ImageBufferData const&, WebCore::IntSize const&)':
ImageBufferCairo.cpp:(.text._ZN7WebCore12getImageDataILNS_8MultiplyE0EEEN3WTF10PassRefPtrIN3JSC21GenericTypedArrayViewINS4_19Uint8ClampedAdaptorEEEEERKNS_7IntRectERKNS_15ImageBufferDataERKNS_7IntSizeE[_ZN7WebCore12getImageDataILNS_8MultiplyE0EEEN3WTF10PassRefPtrIN3JSC21GenericTypedArrayViewINS4_19Uint8ClampedAdaptorEEEEERKNS_7IntRectERKNS_15ImageBufferDataERKNS_7IntSizeE]+0x3e): undefined reference to `JSC::GenericTypedArrayView<JSC::Uint8ClampedAdaptor>::createUninitialized(unsigned int)'
collect2: error: ld returned 1 exit status
make[2]: *** [lib/libwebcore_efl.so.0.1.0] Error 1
make[1]: *** [Source/WebCore/CMakeFiles/WebCore.dir/all] Error 2
make: *** [all] Error 2
Comment 1 Tibor Mészáros 2014-07-07 03:52:56 PDT
Created attachment 234484 [details]
Patch

Fix for the bug
Comment 2 Darin Adler 2014-07-07 11:03:27 PDT
Comment on attachment 234484 [details]
Patch

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

I’d like to hear what some JavaScript bindings expert has to say about this.

> Source/WebCore/platform/graphics/ImageBuffer.h:39
> +#include <runtime/JSCInlines.h>
> +#include <runtime/TypedArrayInlines.h>

This is really unfortunate; includes that are only needed for the JavaScript bindings really don’t belong in the DOM implementation itself. However, there may be no other solution to this problem.

> Source/WebCore/platform/graphics/ImageBuffer.h:40
>  #include <runtime/Uint8ClampedArray.h>

This already-existing include is equally bad.
Comment 3 Darin Adler 2014-07-07 16:06:54 PDT
Comment on attachment 234484 [details]
Patch

Can we put these includes into the two affected .cpp files rather than into the header?
Comment 4 Tibor Mészáros 2014-07-09 07:09:44 PDT
(In reply to comment #3)
> (From update of attachment 234484 [details])
> Can we put these includes into the two affected .cpp files rather than into the header?

I will try to do what you wished. If I got some result, i will upload a new patch.
Comment 5 Csaba Osztrogonác 2014-07-14 06:25:59 PDT
Comment on attachment 234484 [details]
Patch

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

This bug was already fixed by https://bugs.webkit.org/show_bug.cgi?id=130394,
but r167497 removed the fix. The goal of this bug is to revert this incorrect
removal which caused the build failure mentioned in the description.

>> Source/WebCore/platform/graphics/ImageBuffer.h:39
>> +#include <runtime/TypedArrayInlines.h>
> 
> This is really unfortunate; includes that are only needed for the JavaScript bindings really don’t belong in the DOM implementation itself. However, there may be no other solution to this problem.

There are many includes of this headers everywhere, I don't think if we can avoid them:

$ grep -R "runtime/TypedArrayInlines.h" .
./WebCore/Modules/webaudio/AudioBuffer.cpp:#include <runtime/TypedArrayInlines.h>
./WebCore/testing/MockCDM.cpp:#include <runtime/TypedArrayInlines.h>
./WebCore/platform/graphics/filters/FilterEffect.cpp:#include <runtime/TypedArrayInlines.h>
./WebCore/platform/graphics/filters/FEGaussianBlur.cpp:#include <runtime/TypedArrayInlines.h>
./WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:#import <runtime/TypedArrayInlines.h>
./WebCore/html/canvas/WebGLRenderingContext.cpp:#include <runtime/TypedArrayInlines.h>
./WebCore/bindings/js/SerializedScriptValue.cpp:#include <runtime/TypedArrayInlines.h>
./WebCore/bindings/js/JSDOMBinding.h:#include <runtime/TypedArrayInlines.h>
./WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp:#include <runtime/TypedArrayInlines.h>
./WebCore/bindings/js/JSCryptoAlgorithmBuilder.cpp:#include <runtime/TypedArrayInlines.h>

$ grep -R "runtime/JSCInlines.h" .
./WebCore/inspector/WebConsoleAgent.cpp:#include <runtime/JSCInlines.h>
./WebCore/inspector/InspectorDOMAgent.cpp:#include <runtime/JSCInlines.h>
./WebCore/xml/XMLHttpRequest.cpp:#include <runtime/JSCInlines.h>
./WebCore/bridge/jsc/BridgeJSC.h:#include <runtime/JSCInlines.h>
./WebCore/bridge/c/c_utility.h:#include <runtime/JSCInlines.h>
./WebCore/Modules/webaudio/AudioBuffer.cpp:#include <runtime/JSCInlines.h>
./WebCore/Modules/mediasource/SourceBuffer.cpp:#include <runtime/JSCInlines.h>
./WebCore/testing/MockCDM.cpp:#include <runtime/JSCInlines.h>
./WebCore/testing/Internals.cpp:#include <runtime/JSCInlines.h>
./WebCore/platform/SerializedPlatformRepresentation.h:#include <runtime/JSCInlines.h>
./WebCore/platform/graphics/filters/FilterEffect.cpp:#include <runtime/JSCInlines.h>
./WebCore/platform/graphics/filters/FEGaussianBlur.cpp:#include <runtime/JSCInlines.h>
./WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:#import <runtime/JSCInlines.h>
./WebCore/html/HTMLCanvasElement.cpp:#include <runtime/JSCInlines.h>
./WebCore/html/track/DataCue.h:#include <runtime/JSCInlines.h>
./WebCore/html/canvas/WebGLRenderingContext.cpp:#include <runtime/JSCInlines.h>
./WebCore/html/HTMLImageLoader.cpp:#include <runtime/JSCInlines.h>
./WebCore/bindings/js/JSNodeFilterCondition.h:#include <runtime/JSCInlines.h>
./WebCore/bindings/js/SerializedScriptValue.cpp:#include <runtime/JSCInlines.h>
./WebCore/bindings/js/JSCryptoKeySerializationJWK.cpp:#include <runtime/JSCInlines.h>
./WebCore/bindings/js/JSCustomXPathNSResolver.h:#include <runtime/JSCInlines.h>
./WebCore/bindings/js/JSDictionary.h:#include <runtime/JSCInlines.h>
./WebCore/bindings/js/WebCoreTypedArrayController.cpp:#include <runtime/JSCInlines.h>
./WebCore/bindings/js/JSMessagePortCustom.h:#include <runtime/JSCInlines.h>
./WebCore/bindings/js/JSCryptoAlgorithmBuilder.cpp:#include <runtime/JSCInlines.h>
./WebCore/dom/PopStateEvent.cpp:#include <runtime/JSCInlines.h>
./WebCore/dom/CustomEvent.cpp:#include <runtime/JSCInlines.h>
./WebCore/dom/MessageEvent.cpp:#include <runtime/JSCInlines.h>
./WebCore/dom/Node.cpp:#include <runtime/JSCInlines.h>
./WebKit2/Shared/linux/WebMemorySamplerLinux.cpp:#include <runtime/JSCInlines.h>
./WebKit2/WebProcess/WebPage/WebPage.cpp:#include <runtime/JSCInlines.h>
./WebKit2/UIProcess/WebContext.cpp:#include <runtime/JSCInlines.h>

>> Source/WebCore/platform/graphics/ImageBuffer.h:40
>>  #include <runtime/Uint8ClampedArray.h>
> 
> This already-existing include is equally bad.

PassRefPtr<Uint8ClampedArray> is used in line 105 and 106, we really need this include:
http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ImageBuffer.h#L105

It didn't cause build failures before, because it was built into filters objects. 
But I don't think if it is good if ImageBuffer depends on something built to a filter file.


$ grep -R "runtime/Uint8ClampedArray.h" .
./WebCore/platform/graphics/filters/FilterEffect.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEComponentTransfer.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEGaussianBlur.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEConvolveMatrix.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FilterEffect.h:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEBlend.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEMorphology.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEColorMatrix.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEComposite.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEDropShadow.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FELighting.h:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FEDisplacementMap.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/filters/FETurbulence.cpp:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/cg/ImageBufferDataCG.h:#include <runtime/Uint8ClampedArray.h>
./WebCore/platform/graphics/ImageBuffer.h:#include <runtime/Uint8ClampedArray.h>
./WebCore/html/ImageData.h:#include <runtime/Uint8ClampedArray.h>
Comment 6 Csaba Osztrogonác 2014-07-14 06:28:33 PDT
cc-ing Filip too, he landed a huge patch related to this inlines files - https://trac.webkit.org/changeset/163844
Comment 7 Tibor Mészáros 2014-07-14 09:03:34 PDT
Created attachment 234854 [details]
Patch v2

This patch will add the two include into the affected .cpp files.
Comment 8 WebKit Commit Bot 2014-07-14 14:29:30 PDT
Comment on attachment 234854 [details]
Patch v2

Clearing flags on attachment: 234854

Committed r171083: <http://trac.webkit.org/changeset/171083>
Comment 9 WebKit Commit Bot 2014-07-14 14:29:35 PDT
All reviewed patches have been landed.  Closing bug.