[webkit-reviews] review denied: [Bug 35031] [chromium] Implement cancelGeolocationPermissionRequestForFrame : [Attachment 52147] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 31 03:50:22 PDT 2010


Jeremy Orlow <jorlow at chromium.org> has denied Marcus Bulach
<bulach at chromium.org>'s request for review:
Bug 35031: [chromium] Implement cancelGeolocationPermissionRequestForFrame
https://bugs.webkit.org/show_bug.cgi?id=35031

Attachment 52147: Patch
https://bugs.webkit.org/attachment.cgi?id=52147&action=review

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 56830)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,14 @@
> +2010-03-30  Marcus Bulach  <bulach at 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 at 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.


More information about the webkit-reviews mailing list