Fix crash in ~WebProcessPool when using Geolocation with useNetworkProcess=1
Created attachment 266026 [details] Patch
Comment on attachment 266026 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266026&action=review > Source/WebKit2/UIProcess/WebGeolocationManagerProxy.cpp:66 > + if (wasUpdating && !isUpdating()) > + m_provider.stopUpdating(this); > + else { Actually, you only need ASSERT(!isUpdating()); if (wasUpdating) m_provider.stopUpdating(this); If isUpdating() is true, something really fucked up is going on. It looks like the API contract does not require you to change the high accuracy settings if you stop the providers. Let's remove the else {} case. > Tools/ChangeLog:14 > + Properly load about:blank in all WebViews to clean up. IMHO, it would be worth explaining why. Can you add a paragraph explaining that we had a Geolocation provider stopping after its state tracker was destroyed with its stack frame? > Tools/TestWebKitAPI/Tests/WebKit2/Geolocation.cpp:235 > + EXPECT_EQ(GeolocationEvent::DisableHighAccuracy, events[3]); We know these do not work. Can you find a way to make them work? If not, maybe turn them into assertions?
(In reply to comment #2) > Comment on attachment 266026 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=266026&action=review > > > Source/WebKit2/UIProcess/WebGeolocationManagerProxy.cpp:66 > > + if (wasUpdating && !isUpdating()) > > + m_provider.stopUpdating(this); > > + else { > > Actually, you only need > ASSERT(!isUpdating()); > if (wasUpdating) > m_provider.stopUpdating(this); > > If isUpdating() is true, something really fucked up is going on. > > It looks like the API contract does not require you to change the high > accuracy settings if you stop the providers. Let's remove the else {} case. Done. > > > Tools/ChangeLog:14 > > + Properly load about:blank in all WebViews to clean up. > > IMHO, it would be worth explaining why. > Can you add a paragraph explaining that we had a Geolocation provider > stopping after its state tracker was destroyed with its stack frame? Done. > > > Tools/TestWebKitAPI/Tests/WebKit2/Geolocation.cpp:235 > > + EXPECT_EQ(GeolocationEvent::DisableHighAccuracy, events[3]); > > We know these do not work. Can you find a way to make them work? > > If not, maybe turn them into assertions? These do not work if the process crashes, but they do work if the process does not crash. I'm leaving them as they are.
http://trac.webkit.org/changeset/192747