See https://bugs.webkit.org/show_bug.cgi?id=34962 https://bugs.webkit.org/attachment.cgi?id=48890&action=diff#WebKit/chromium/src/ChromeClientImpl.h_sec1 A new method cancelGeolocationPermissionRequestForFrame was added, which will need implementing fully instead of the placeholder in WebKit/chromium/src/ChromeClientImpl.h (i.e. call out to IPC to get the browser process to dismiss the infobar)
Created attachment 52147 [details] Patch
Comment on attachment 52147 [details] Patch > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 56830) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,14 @@ > +2010-03-30 Marcus Bulach <bulach@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Implements cancelGeolocationPermissionRequestForFrame. > + No extra new line > + https://bugs.webkit.org/show_bug.cgi?id=35031 Put more of a description here. Explain why. > + * page/Geolocation.cpp: > + (WebCore::Geolocation::disconnectFrame): > + > 2010-03-31 John Gregg <johnnyg@google.com> > > Reviewed by Darin Fisher. > Index: WebCore/page/Geolocation.cpp > =================================================================== > --- WebCore/page/Geolocation.cpp (revision 56830) > +++ WebCore/page/Geolocation.cpp (working copy) > @@ -225,6 +225,7 @@ void Geolocation::disconnectFrame() > if (m_frame) { > if (m_frame->document()) > m_frame->document()->setUsingGeolocation(false); > + // FIXME: pass "this" to cancelGeolocationPermissionRequestForFrame, similar to requestGeolocationPermissionForFrame. Why not do this now? You can change the consumers of this API at the WebKit layer.
Created attachment 52160 [details] Patch
(In reply to comment #2) > (From update of attachment 52147 [details]) > > Index: WebCore/ChangeLog > > =================================================================== > > --- WebCore/ChangeLog (revision 56830) > > +++ WebCore/ChangeLog (working copy) > > @@ -1,3 +1,14 @@ > > +2010-03-30 Marcus Bulach <bulach@chromium.org> > > + > > + Reviewed by NOBODY (OOPS!). > > + > > + Implements cancelGeolocationPermissionRequestForFrame. > > + > > No extra new line > > > + https://bugs.webkit.org/show_bug.cgi?id=35031 > > Put more of a description here. Explain why. Done, let me know if it's clear enough. > > > + * page/Geolocation.cpp: > > + (WebCore::Geolocation::disconnectFrame): > > + > > 2010-03-31 John Gregg <johnnyg@google.com> > > > > Reviewed by Darin Fisher. > > Index: WebCore/page/Geolocation.cpp > > =================================================================== > > --- WebCore/page/Geolocation.cpp (revision 56830) > > +++ WebCore/page/Geolocation.cpp (working copy) > > @@ -225,6 +225,7 @@ void Geolocation::disconnectFrame() > > if (m_frame) { > > if (m_frame->document()) > > m_frame->document()->setUsingGeolocation(false); > > + // FIXME: pass "this" to cancelGeolocationPermissionRequestForFrame, similar to requestGeolocationPermissionForFrame. > > Why not do this now? You can change the consumers of this API at the WebKit > layer. Thanks for pointing this out! Done on all APIs I could find.
Attachment 52160 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 52160 [details] Patch > Index: WebKit/chromium/src/ChromeClientImpl.cpp > =================================================================== > --- WebKit/chromium/src/ChromeClientImpl.cpp (revision 56830) > +++ WebKit/chromium/src/ChromeClientImpl.cpp (working copy) > @@ -707,6 +707,12 @@ void ChromeClientImpl::requestGeolocatio > m_webView->client()->geolocationService()->requestPermissionForFrame(geolocationService->geolocationServiceBridge()->getBridgeId(), frame->document()->url()); > } > > +void ChromeClientImpl::cancelGeolocationPermissionRequestForFrame(Frame* frame, Geolocation* geolocation) > +{ > + GeolocationServiceChromium* geolocationService = reinterpret_cast<GeolocationServiceChromium*>(geolocation->getGeolocationService()); That reinterpret_cast can be a static_cast, no? Use static_cast when there exists an "is a" relationship between the types you are casting. Not doing so can cause memory errors if you ever start using multiple inheritance.
Comment on attachment 52160 [details] Patch Besides Darin's comment, LGTM.
Created attachment 52182 [details] Patch
(In reply to comment #6) > (From update of attachment 52160 [details]) > > Index: WebKit/chromium/src/ChromeClientImpl.cpp > > =================================================================== > > --- WebKit/chromium/src/ChromeClientImpl.cpp (revision 56830) > > +++ WebKit/chromium/src/ChromeClientImpl.cpp (working copy) > > @@ -707,6 +707,12 @@ void ChromeClientImpl::requestGeolocatio > > m_webView->client()->geolocationService()->requestPermissionForFrame(geolocationService->geolocationServiceBridge()->getBridgeId(), frame->document()->url()); > > } > > > > +void ChromeClientImpl::cancelGeolocationPermissionRequestForFrame(Frame* frame, Geolocation* geolocation) > > +{ > > + GeolocationServiceChromium* geolocationService = reinterpret_cast<GeolocationServiceChromium*>(geolocation->getGeolocationService()); > > That reinterpret_cast can be a static_cast, no? Use static_cast when there > exists an "is a" relationship between the types you are casting. Not doing so > can cause memory errors if you ever start using multiple inheritance. Oh, very good catch, thanks! Done.
Comment on attachment 52182 [details] Patch r=me
Comment on attachment 52182 [details] Patch Clearing flags on attachment: 52182 Committed r56851: <http://trac.webkit.org/changeset/56851>
All reviewed patches have been landed. Closing bug.
*** Bug 36833 has been marked as a duplicate of this bug. ***