Bug 158716 – False-positive over-release of WebView in destroyWebViewAndOffscreenWindow() in DumpRenderTree.mm

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

The clang static analyzer complains that 'webView' is over-released in destroyWebViewAndOffscreenWindow() in DumpRenderTree.mm.

The current code retains 'webView' in createWebViewAndOffscreenWindow(), returns it, then assumes that [mainFrame webView] returns the exact same WebView in destroyWebViewAndOffscreenWindow().

[David Kilzer (:ddkilzer)]

Created attachment 281220 [details]
Patch v1

[Andy Estes]

Comment on attachment 281220 [details]
Patch v1

Patch looks good, but you seem to be running into problems because DumpRenderTreeMac.h is included in a .cpp file. r=me if you straighten that out.

[David Kilzer (:ddkilzer)]

Created attachment 281222 [details]
Patch v2

[Andy Estes]

Comment on attachment 281222 [details]
Patch v2

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

> Tools/DumpRenderTree/mac/DumpRenderTreeMac.h:71
> +WebView* createWebViewAndOffscreenWindow() NS_RETURNS_RETAINED;

I missed this the first time around, but the space should go before the *, not after.

[David Kilzer (:ddkilzer)]

Committed r202020: <http://trac.webkit.org/changeset/202020>

[David Kilzer (:ddkilzer)]

Comment on attachment 281222 [details]
Patch v2

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

>> Tools/DumpRenderTree/mac/DumpRenderTreeMac.h:71
>> +WebView* createWebViewAndOffscreenWindow() NS_RETURNS_RETAINED;
> 
> I missed this the first time around, but the space should go before the *, not after.

Fixed!

[Andy Estes]

For reasons I don't understand, this broke the El Capitan CMake Debug build: https://build.webkit.org/builders/Apple%20El%20Capitan%20CMake%20Debug%20%28Build%29/builds/5853

Seems like CMake is somehow ignoring the #if __OBJC__?

[Andy Estes]

Alex says this is a bug with the CMake build system. All files in DumpRenderTree are being built with -ObjC++.

[Joseph Pecoraro]

(In reply to comment #8)
> Alex says this is a bug with the CMake build system. All files in
> DumpRenderTree are being built with -ObjC++.

Source/WebKit/PlatformMac.cmake has what I think would be an example of how to do this:

> foreach (_file ${WebKit_SOURCES})
>     list(FIND C99_FILES ${_file} _c99_index)
>     if (${_c99_index} EQUAL -1)
>         set_source_files_properties(${_file} PROPERTIES COMPILE_FLAGS "-ObjC++ -std=c++14")
>     else ()
>         set_source_files_properties(${_file} PROPERTIES COMPILE_FLAGS -std=c99)
>     endif ()
> endforeach ()

[David Kilzer (:ddkilzer)]

(In reply to comment #9)
> (In reply to comment #8)
> > Alex says this is a bug with the CMake build system. All files in
> > DumpRenderTree are being built with -ObjC++.
> 
> Source/WebKit/PlatformMac.cmake has what I think would be an example of how
> to do this:
> 
> > foreach (_file ${WebKit_SOURCES})
> >     list(FIND C99_FILES ${_file} _c99_index)
> >     if (${_c99_index} EQUAL -1)
> >         set_source_files_properties(${_file} PROPERTIES COMPILE_FLAGS "-ObjC++ -std=c++14")
> >     else ()
> >         set_source_files_properties(${_file} PROPERTIES COMPILE_FLAGS -std=c99)
> >     endif ()
> > endforeach ()

I filed Bug 158743: REGRESSION (r202020): El Capitan CMake Debug build broken