Bug 124199

Summary: Fixed unused parameter warnings in WebCore
Product: WebKit Reporter: Tibor Mészáros <mtiborinf>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, dbates, dino, dstockwell, esprehn+autocc, galpeter, glenn, gyuyoung.kim, gyuyoung.kim, japhet, kangil.han, kondapallykalyan, macpherson, menard, mkwst, ossy, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch v2
none
Patch v3
none
Patch v4
none
Patch v5
none
Patch v6
ossy: review-, ossy: commit-queue-
Patch v7 none

Description Tibor Mészáros 2013-11-12 06:00:26 PST
There should be some unused parametes, that cause build warning during EFL / EFL --minimal build in WebCore.
Comment 1 Tibor Mészáros 2013-11-12 06:24:34 PST
Created attachment 216662 [details]
Patch

There were some unused parameters, that cause build warning during EFL / EFL --minimal build in WebCore, this patch will fix most of them.
Comment 2 Tibor Mészáros 2013-11-12 06:31:45 PST
Created attachment 216665 [details]
Patch v2

Fixed the title in the patch.
Comment 3 WebKit Commit Bot 2013-11-12 06:33:15 PST
Attachment 216665 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/css/CSSCursorImageValue.cpp', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/fileapi/ThreadableBlobRegistry.cpp', u'Source/WebCore/html/HTMLAnchorElement.cpp', u'Source/WebCore/html/parser/XSSAuditor.cpp', u'Source/WebCore/inspector/InspectorConsoleInstrumentation.h', u'Source/WebCore/loader/DocumentThreadableLoader.cpp', u'Source/WebCore/loader/cache/CachedImage.cpp', u'Source/WebCore/page/animation/ImplicitAnimation.cpp', u'Source/WebCore/page/animation/KeyframeAnimation.cpp', u'Source/WebCore/platform/graphics/WidthIterator.cpp', u'Source/WebCore/rendering/RenderView.cpp', u'Source/WebCore/rendering/style/RenderStyle.cpp', u'Source/WebCore/testing/Internals.cpp']" exit_code: 1
Source/WebCore/page/animation/ImplicitAnimation.cpp:81:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/page/animation/ImplicitAnimation.cpp:82:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/page/animation/ImplicitAnimation.cpp:83:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/page/animation/ImplicitAnimation.cpp:84:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/page/animation/KeyframeAnimation.cpp:187:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/page/animation/KeyframeAnimation.cpp:188:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 6 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Tibor Mészáros 2013-11-12 06:42:27 PST
Created attachment 216667 [details]
Patch v3

Fixed the [whitespace/tab] errors.
Comment 5 Gyuyoung Kim 2013-11-12 06:45:43 PST
Comment on attachment 216667 [details]
Patch v3

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

> Source/WebCore/ChangeLog:3
> +        [EFL] Cleanup the build from unused parameters in WebCore

[EFL] ? This patch is for WebCore, not EFL port.
Comment 6 Tibor Mészáros 2013-11-12 06:54:58 PST
Created attachment 216669 [details]
Patch v4

Fixed the title again.
Comment 7 Tibor Mészáros 2013-11-12 09:02:55 PST
Created attachment 216679 [details]
Patch v5

Updated the patch to be up to date.
Comment 8 Tibor Mészáros 2013-11-12 09:56:47 PST
Created attachment 216691 [details]
Patch v6

reupdated the patch, to be up to date.
Comment 9 Gyuyoung Kim 2013-11-12 17:27:06 PST
Comment on attachment 216691 [details]
Patch v6

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

> Source/WebCore/loader/cache/CachedImage.cpp:278
> +        UNUSED_PARAM(sizeType);

Wrong indentation.

> Source/WebCore/page/animation/ImplicitAnimation.cpp:80
> +    

Unneeded line.
Comment 10 Csaba Osztrogonác 2013-11-13 02:18:23 PST
Comment on attachment 216691 [details]
Patch v6

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

LGTM in general, but r- now due to style issues, please fix them.

> Source/WebCore/css/CSSCursorImageValue.cpp:178
> +#else
> +        UNUSED_PARAM(document);

wrong indentation here too

> Source/WebCore/dom/Document.cpp:4874
> +#else
> +        UNUSED_PARAM(isThrottled);

ditto

> Source/WebCore/loader/DocumentThreadableLoader.cpp:335
> +        UNUSED_PARAM(identifier);

ditto

> Source/WebCore/loader/DocumentThreadableLoader.cpp:361
> +        UNUSED_PARAM(identifier);

ditto

> Source/WebCore/rendering/style/RenderStyle.cpp:793
> +        UNUSED_PARAM(other);

ditto
Comment 11 Tibor Mészáros 2013-11-13 03:16:00 PST
Created attachment 216788 [details]
Patch v7

Fixed indentation problems.
Comment 12 Csaba Osztrogonác 2013-11-13 03:49:52 PST
Comment on attachment 216788 [details]
Patch v7

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

LGTM, r=me. I'll fix the style issue mentioned before landing.

> Source/WebCore/platform/graphics/WidthIterator.cpp:134
> +#else
> +        UNUSED_PARAM(iterator);
>  #endif
>          fontData->applyTransforms(glyphBuffer->glyphs(lastGlyphCount), advances + lastGlyphCount, glyphBufferSize - lastGlyphCount, typesettingFeatures);

This code is so ugly ... :-/

Maybe we should reorder it a little bit:
#if !ENABLE(SVG_FONTS)
     UNUSED_PARAM(iterator);
#else
    // We need to handle transforms on SVG fonts internally, since they are rendered internally.
    if 
    ....
    } else
#endif
        fontdata->applyTransforms(...);
Comment 13 Csaba Osztrogonác 2013-11-13 03:57:54 PST
Landed in https://trac.webkit.org/changeset/159188