[webkit-reviews] review denied: [Bug 74499] GeolocationClient should have a setController() method. : [Attachment 119204] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied Alexander Færøy
<ahf at 0x90.dk>'s request for review:
Bug 74499: GeolocationClient should have a setController() method.
https://bugs.webkit.org/show_bug.cgi?id=74499

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

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119204&action=review


I'm not sure this change is a good one. It's common to keep the page as a
member of the clients. I also wonder if it's possible that you might end up
holding a different controller than what the page is associated with in some
cases. Also, it doesn't seem to gain much in terms of code simplification.
Would you mind arguing the case for going through with this change?

> Source/WebKit/gtk/WebCoreSupport/GeolocationClientGtk.h:46
> +    void setController(GeolocationController*);
> +

The GTK+ build fails because this class is inside the WebCore namespace and
there's no using namespace WebCore in this file, so you have to prefix it.