Bug 70591 – [WK2]Critical warning while building WebKit2 GTK+

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Vamshikrishna Yellenki]

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.

[Vamshikrishna Yellenki]

Created attachment 111927 [details]
patch for critical warnings while building webkit

patch for critical warnings while building webkit

[Martin Robinson]

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.

[Vamshikrishna Yellenki]

Created attachment 112151 [details]
Incorporated review comments and Updated patch 

Incorporated review comments and Updated patch

[WebKit Review Bot]

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.

[Darin Adler]

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

[Vamshikrishna Yellenki]

Created attachment 112153 [details]
Updated the new patch

Updated the new patch.

[Martin Robinson]

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.

[Vamshikrishna Yellenki]

I verified style on patch. I do not know that it won't work on patch.

[Martin Robinson]

(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."

[Vamshikrishna Yellenki]

It's not a spelling error. Error due to missing white space in ChangeLog file.

[Martin Robinson]

(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.

[Vamshikrishna Yellenki]

I already submitted new patch(comment #6).

[Martin Robinson]

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.

[Vamshikrishna Yellenki]

Created attachment 112301 [details]
Updated spelling mistake

Updated spelling mistake. Thanks Robinson.

[WebKit Review Bot]

Comment on attachment 112301 [details]
Updated spelling mistake

Clearing flags on attachment: 112301

Committed r98325: <http://trac.webkit.org/changeset/98325>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.