Bug 118240 – [Qt] Crash on close if build with geolocation

• Component: WebKit Qt
• Status: RESOLVED INVALID

[Allan Sandfeld Jensen]

If WebKit has geolocation, but we have no geolocation supplement, webkit will crash on exit.

[Allan Sandfeld Jensen]

Created attachment 205806 [details]
Patch

[Benjamin Poulain]

Comment on attachment 205806 [details]
Patch

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

I am confused by this. I don't see when a GeolocationObject would outlive it's page.

Can you add a test + an explanation of the case in the ChangeLog?

> Source/WebCore/ChangeLog:9
> +        This also matches similar checkes elsewhere in Geolocation.

What?

[Allan Sandfeld Jensen]

(In reply to comment #2)
> (From update of attachment 205806 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205806&action=review
> 
> I am confused by this. I don't see when a GeolocationObject would outlive it's page.
> 
It is not that it outlives the page, it is that the page has no GeolocationController.

[Jocelyn Turcotte]

Comment on attachment 205806 [details]
Patch

Normally ENABLE_GEOLOCATION isn't set if QtLocation isn't available.
We have an exception for !production_build to allow layout tests to provide a mock controller in Tools/qmake/mkspecs/features/features.prf

If we want to avoid the crash with !production_build, a different fix that would avoid adding null-checks to Geolocation.cpp could be to use GeolocationClientMock also if !HAVE(QTLOCATION) in QWebPageAdapter::initializeWebCorePage.
Maybe something else wouldn't work in this case, I didn't investigate far.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.