Bug 134294 - Clean up the WebKit build from unused parameter warning in Webkit2/UIProcess module
Summary: Clean up the WebKit build from unused parameter warning in Webkit2/UIProcess ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-25 01:55 PDT by Rohit
Modified: 2014-07-01 09:59 PDT (History)
9 users (show)

See Also:


Attachments
Patch (1.55 KB, patch)
2014-06-25 02:32 PDT, Rohit
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (522.58 KB, application/zip)
2014-06-25 03:35 PDT, Build Bot
no flags Details
Patch (1.52 KB, patch)
2014-06-25 23:21 PDT, Rohit
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rohit 2014-06-25 01:55:28 PDT
Clean up WebKit build from usused parameters in newly added functionality in Webkit2/UIProcess module.

Build logs:

Linking CXX executable ../../bin/TestWebKitAPI/WebCore/LayoutUnit
[ 82%] Built target web-inspector-resources
[ 82%] Built target WebCoreTestSupport
[ 82%] Built target LayoutUnit
[ 82%] Built target URL
Scanning dependencies of target WebKit2
[ 82%] [ 82%] Building CXX object Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/API/C/WKPage.cpp.o
Building CXX object Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/WebPageProxy.cpp.o
In file included from /home/kumar.rohit/WebKit/Source/WebKit2/UIProcess/API/C/WKPage.cpp:35:0:
/home/kumar.rohit/WebKit/Source/WebKit2/UIProcess/API/APIUIClient.h:118:18: warning: unused parameter ‘totalBytesNeeded’ [-Wunused-parameter]
     virtual void reachedApplicationCacheOriginQuota(WebKit::WebPageProxy*, const WebCore::SecurityOrigin&, uint64_t currentQuota, uint64_t totalBytesNeeded, std::function<void (unsigned long long)> completionHandler)
                  ^
In file included from /home/kumar.rohit/WebKit/Source/WebKit2/UIProcess/WebPageProxy.cpp:35:0:
/home/kumar.rohit/WebKit/Source/WebKit2/UIProcess/API/APIUIClient.h:118:18: warning: unused parameter ‘totalBytesNeeded’ [-Wunused-parameter]
     virtual void reachedApplicationCacheOriginQuota(WebKit::WebPageProxy*, const WebCore::SecurityOrigin&, uint64_t currentQuota, uint64_t totalBytesNeeded, std::function<void (unsigned long long)> completionHandler)
                  ^
Linking CXX shared library ../../lib/libewebkit2.so
Comment 1 Rohit 2014-06-25 02:32:27 PDT
Created attachment 233799 [details]
Patch
Comment 2 Build Bot 2014-06-25 03:35:37 PDT
Comment on attachment 233799 [details]
Patch

Attachment 233799 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4958240152485888

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html
Comment 3 Build Bot 2014-06-25 03:35:40 PDT
Created attachment 233800 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 4 Shivakumar J M 2014-06-25 21:59:27 PDT
As per the rule https://lists.webkit.org/pipermail/webkit-dev/2012-October/022369.html, we can omit parameter names for unused parameters entirely.
Comment 5 Rohit 2014-06-25 22:19:03 PDT
(In reply to comment #4)
> As per the rule https://lists.webkit.org/pipermail/webkit-dev/2012-October/022369.html, we can omit parameter names for unused parameters entirely.


Yes, I tried that. But if we remove parameter name in function defintion then it shows error in check-webkit-style script.
Comment 6 Shivakumar J M 2014-06-25 22:57:30 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > As per the rule https://lists.webkit.org/pipermail/webkit-dev/2012-October/022369.html, we can omit parameter names for unused parameters entirely.
> 
> 
> Yes, I tried that. But if we remove parameter name in function defintion then it shows error in check-webkit-style script.

I feel that error is reported, without these fix as well, if we just run check-webkit-style for the file APIUIClient.h, it lists similar errors in some other lines also, for these kind of parameter( std::function<void (unsigned long long)> completionHandler). so we can fix that space error as per coding idiom or check can it be ignored as per coding idiom.
Comment 7 Rohit 2014-06-25 23:21:19 PDT
Created attachment 233888 [details]
Patch
Comment 8 WebKit Commit Bot 2014-06-25 23:23:30 PDT
Attachment 233888 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/APIUIClient.h:118:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Rohit 2014-06-25 23:30:21 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > As per the rule https://lists.webkit.org/pipermail/webkit-dev/2012-October/022369.html, we can omit parameter names for unused parameters entirely.
> > 
> > 
> > Yes, I tried that. But if we remove parameter name in function defintion then it shows error in check-webkit-style script.
> 
> I feel that error is reported, without these fix as well, if we just run check-webkit-style for the file APIUIClient.h, it lists similar errors in some other lines also, for these kind of parameter( std::function<void (unsigned long long)> completionHandler). so we can fix that space error as per coding idiom or check can it be ignored as per coding idiom.

Although I am not facing this space error if I don't omit parameter name, but I have cleaned up the warning using parameter name omission. I guess it happens because check-webkit-style script checks for only that particular statement where changes are made and hence it shows error only in that line and no other line in that file. For current patch I have ignored the warning.
Comment 10 Gyuyoung Kim 2014-06-30 22:13:26 PDT
LGTM based on Darin's comment.
https://bugs.webkit.org/show_bug.cgi?id=131260#c8

However, I would like to let Darin review this patch.
Comment 11 WebKit Commit Bot 2014-07-01 09:59:23 PDT
Comment on attachment 233888 [details]
Patch

Clearing flags on attachment: 233888

Committed r170643: <http://trac.webkit.org/changeset/170643>
Comment 12 WebKit Commit Bot 2014-07-01 09:59:28 PDT
All reviewed patches have been landed.  Closing bug.