Bug 70591 - [WK2]Critical warning while building WebKit2 GTK+
Summary: [WK2]Critical warning while building WebKit2 GTK+
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-21 02:07 PDT by Vamshikrishna Yellenki
Modified: 2011-10-25 01:19 PDT (History)
7 users (show)

See Also:


Attachments
patch for critical warnings while building webkit (1.08 KB, patch)
2011-10-21 02:26 PDT, Vamshikrishna Yellenki
mrobinson: review-
Details | Formatted Diff | Diff
Incorporated review comments and Updated patch (1.02 KB, patch)
2011-10-23 22:42 PDT, Vamshikrishna Yellenki
no flags Details | Formatted Diff | Diff
Updated the new patch (1.02 KB, patch)
2011-10-23 23:00 PDT, Vamshikrishna Yellenki
mrobinson: review-
Details | Formatted Diff | Diff
Updated spelling mistake (1.02 KB, patch)
2011-10-25 00:12 PDT, Vamshikrishna Yellenki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vamshikrishna Yellenki 2011-10-21 02:07:29 PDT
Below are the critical warnings displayed while building webkit. 

../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp: In member function ‘void WebKit::WebPage::performDragControllerAction(uint64_t, WebCore::DragData)’:
../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1818:16: warning: possible problem detected in invocation of delete operator:
../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1816:24: warning: ‘data’ has incomplete type
../../Source/WebCore/platform/DragData.h:60:7: warning: forward declaration of ‘struct WebCore::DataObjectGtk’
../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1818:16: note: neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined.
../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1849:12: warning: possible problem detected in invocation of delete operator:
../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1847:20: warning: ‘data’ has incomplete type
../../Source/WebCore/platform/DragData.h:60:7: warning: forward declaration of ‘struct WebCore::DataObjectGtk’
../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1849:12: note: neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined.
Comment 1 Vamshikrishna Yellenki 2011-10-21 02:26:11 PDT
Created attachment 111927 [details]
patch for critical warnings while building webkit

patch for critical warnings while building webkit
Comment 2 Martin Robinson 2011-10-21 09:30:25 PDT
Comment on attachment 111927 [details]
patch for critical warnings while building webkit

I think it would be better to include DataObjectGtk.h in the actual C++ file instead of removing this forward-declaration.
Comment 3 Vamshikrishna Yellenki 2011-10-23 22:42:33 PDT
Created attachment 112151 [details]
Incorporated review comments and Updated patch 

Incorporated review comments and Updated patch
Comment 4 WebKit Review Bot 2011-10-23 22:45:00 PDT
Attachment 112151 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/ChangeLog:8:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Darin Adler 2011-10-23 22:47:57 PDT
Comment on attachment 112151 [details]
Incorporated review comments and Updated patch 

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:132
> +#include"DataObjectGtk.h"

missing space here after include

>> Source/WebKit2/ChangeLog:8
>> +        * WebProcess/WebPage/WebPage.cpp:Included DataObjectGtk.h to avoid warrings for GTK
> 
> Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]

warnings rather than warrings
Comment 6 Vamshikrishna Yellenki 2011-10-23 23:00:20 PDT
Created attachment 112153 [details]
Updated the new patch

Updated the new patch.
Comment 7 Martin Robinson 2011-10-23 23:05:38 PDT
Comment on attachment 112153 [details]
Updated the new patch

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

> Source/WebKit2/ChangeLog:8
> +        * WebProcess/WebPage/WebPage.cpp: Included DataObjectGtk.h to avoid warrings for GTK

The code change is fine, but I'm curious why you did not fix the spelling of warning in the ChangeLog.
Comment 8 Vamshikrishna Yellenki 2011-10-24 22:23:58 PDT
I verified style on patch. I do not know that it won't work on patch.
Comment 9 Martin Robinson 2011-10-24 22:33:53 PDT
(In reply to comment #8)
> I verified style on patch. I do not know that it won't work on patch.

I'm confused now. The style bot cannot find spelling errors as far as I know. :) Maybe you just missed in Darin's review he mentioned that you misspelled "warnings" as "warrings."
Comment 10 Vamshikrishna Yellenki 2011-10-24 23:15:37 PDT
It's not a spelling error. Error due to missing white space in ChangeLog file.
Comment 11 Martin Robinson 2011-10-24 23:49:22 PDT
(In reply to comment #10)
> It's not a spelling error. Error due to missing white space in ChangeLog file.

In the last line of Darin's review he said: "warnings rather than warrings" which referred to a misspelling in the ChangeLog. I'm sorry to make such a big deal out of such an insignificant error, but I guess you didn't see that in the review. I'll gladly r+ this patch ASAP if you fix that.
Comment 12 Vamshikrishna Yellenki 2011-10-24 23:53:23 PDT
I already submitted new patch(comment #6).
Comment 13 Martin Robinson 2011-10-24 23:56:08 PDT
Comment on attachment 112153 [details]
Updated the new patch

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

>> Source/WebKit2/ChangeLog:8
>> +        * WebProcess/WebPage/WebPage.cpp: Included DataObjectGtk.h to avoid warrings for GTK
> 
> The code change is fine, but I'm curious why you did not fix the spelling of warning in the ChangeLog.

The word is still misspelled.
Comment 14 Vamshikrishna Yellenki 2011-10-25 00:12:08 PDT
Created attachment 112301 [details]
Updated spelling mistake

Updated spelling mistake. Thanks Robinson.
Comment 15 WebKit Review Bot 2011-10-25 01:19:02 PDT
Comment on attachment 112301 [details]
Updated spelling mistake

Clearing flags on attachment: 112301

Committed r98325: <http://trac.webkit.org/changeset/98325>
Comment 16 WebKit Review Bot 2011-10-25 01:19:07 PDT
All reviewed patches have been landed.  Closing bug.