2011-02-16 John Knottenbelt <jknotten@chromium.org>
authorjknotten@chromium.org <jknotten@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Mar 2011 10:15:49 +0000 (10:15 +0000)
committerjknotten@chromium.org <jknotten@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Mar 2011 10:15:49 +0000 (10:15 +0000)
        Reviewed by Dmitry Titov.

        Detach Geolocation from Frame when Page destroyed.
        https://bugs.webkit.org/show_bug.cgi?id=52877

        Ensure that all geolocation permission requests are cancelled
        when the page is detached from its frame.

        * fast/dom/Geolocation/page-reload-cancel-permission-requests-expected.txt: Added.
        * fast/dom/Geolocation/page-reload-cancel-permission-requests.html: Added.
        * fast/dom/Geolocation/resources/page-reload-cancel-permission-requests-inner.html: Added.
        * fast/dom/Geolocation/script-tests/page-reload-cancel-permission-requests.js: Added.
        * platform/gtk/Skipped:
        * platform/mac/Skipped:
        * platform/qt-wk2/Skipped:
2011-01-21  John Knottenbelt  <jknotten@chromium.org>

        Reviewed by Dmitry Titov.

        Detach Geolocation from Frame when Page destroyed.
        https://bugs.webkit.org/show_bug.cgi?id=52877

        On Page destruction, any outstanding Geolocation permission
        requests should be cancelled, because the Geolocation can only
        access the client indirectly via m_frame->page().

        Page destruction is signalled by a call to the
        Frame::pageDestroyed() method. This explictly calls
        DOMWindow::resetGeolocation which ultimately calls Geolocation::reset.

        Geolocation::reset() detaches from the GeolocationController,
        cancels requests, watches and single shots, and sets the
        permission state back to Unknown.

        Frame::pageDestroyed() is also called by FrameLoader even though
        the page is not destroyed. We should still cancel permission
        requests, because the GeolocationClient will become inaccessible
        to the Geolocation object after this call.

        Frame::transferChildFrameToNewDocument also indirectly calls
        Geolocation::reset when the frame is reparented between
        pages. Ideally we would like the Geolocation's activities to
        continue after reparenting, see bug
        https://bugs.webkit.org/show_bug.cgi?id=55577

        Since GeolocationController is owned by Page, and all Geolocation
        objects will now unsubscribe from the GeolocationController on
        pageDetached(), we no longer need to call stopUpdating() from the
        GeolocationController's destructor. Instead we can simply assert
        that there should be no no observers. See related bug
        https://bugs.webkit.org/show_bug.cgi?id=52216 .

        Introduced new method 'numberOfPendingPermissionRequests' on
        GeolocationClientMock to count the number of outstanding pending
        permission requests. This provides a reusable implementation for
        client-based implementations of the LayoutTestController's
        numberOfPendingGeolocationPermissionRequests method.

        Test: fast/dom/Geolocation/page-reload-cancel-permission-requests.html

        * page/DOMWindow.cpp:
        (WebCore::DOMWindow::resetGeolocation):
        * page/DOMWindow.h:
        * page/Frame.cpp:
        (WebCore::Frame::pageDestroyed):
        (WebCore::Frame::transferChildFrameToNewDocument):
        * page/Geolocation.cpp:
        (WebCore::Geolocation::~Geolocation):
        (WebCore::Geolocation::page):
        (WebCore::Geolocation::reset):
        (WebCore::Geolocation::disconnectFrame):
        (WebCore::Geolocation::lastPosition):
        (WebCore::Geolocation::requestPermission):
        (WebCore::Geolocation::startUpdating):
        (WebCore::Geolocation::stopUpdating):
        * page/Geolocation.h:
        * page/GeolocationController.cpp:
        (WebCore::GeolocationController::~GeolocationController):
        * page/Navigator.cpp:
        (WebCore::Navigator::resetGeolocation):
        * page/Navigator.h:
        * platform/mock/GeolocationClientMock.cpp:
        (WebCore::GeolocationClientMock::numberOfPendingPermissionRequests):
        * platform/mock/GeolocationClientMock.h:
2011-01-26  John Knottenbelt  <jknotten@chromium.org>

        Reviewed by Dmitry Titov.

        Detach Geolocation from Frame when Page destroyed.
        https://bugs.webkit.org/show_bug.cgi?id=52877

        Add accessors to the WebGeolocationClientMock to allow the number of
        pending geolocation permission requests to be queried.

        * public/WebGeolocationClientMock.h:
        * src/WebGeolocationClientMock.cpp:
        (WebKit::WebGeolocationClientMock::numberOfPendingPermissionRequests):
2011-01-26  John Knottenbelt  <jknotten@chromium.org>

        Reviewed by Dmitry Titov.

        Detach Geolocation from Frame when Page destroyed.
        https://bugs.webkit.org/show_bug.cgi?id=52877

        Extend the layout test controller to expose the number of pending
        geolocation requests, so that we can test that the requests have
        been cancelled on page close.

        * DumpRenderTree/LayoutTestController.cpp:
        (numberOfPendingGeolocationPermissionRequestsCallback):
        (LayoutTestController::staticFunctions):
        * DumpRenderTree/LayoutTestController.h:
        * DumpRenderTree/chromium/LayoutTestController.cpp:
        (LayoutTestController::LayoutTestController):
        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
        * DumpRenderTree/chromium/LayoutTestController.h:
        * DumpRenderTree/gtk/LayoutTestControllerGtk.cpp:
        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
        * DumpRenderTree/mac/LayoutTestControllerMac.mm:
        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
        * DumpRenderTree/mac/UIDelegate.h:
        * DumpRenderTree/mac/UIDelegate.mm:
        (-[UIDelegate numberOfPendingGeolocationPermissionRequests]):
        * DumpRenderTree/qt/LayoutTestControllerQt.cpp:
        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
        * DumpRenderTree/qt/LayoutTestControllerQt.h:
        * DumpRenderTree/win/LayoutTestControllerWin.cpp:
        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
        * DumpRenderTree/wx/LayoutTestControllerWx.cpp:
        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@80701 268f45cc-cd09-0410-ab3c-d52691b4dbfc

35 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/Geolocation/page-reload-cancel-permission-requests-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Geolocation/page-reload-cancel-permission-requests.html [new file with mode: 0644]
LayoutTests/fast/dom/Geolocation/resources/page-reload-cancel-permission-requests-inner.html [new file with mode: 0644]
LayoutTests/fast/dom/Geolocation/script-tests/page-reload-cancel-permission-requests.js [new file with mode: 0644]
LayoutTests/platform/gtk/Skipped
LayoutTests/platform/mac/Skipped
LayoutTests/platform/qt-wk2/Skipped
Source/WebCore/ChangeLog
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/DOMWindow.h
Source/WebCore/page/Frame.cpp
Source/WebCore/page/Geolocation.cpp
Source/WebCore/page/Geolocation.h
Source/WebCore/page/GeolocationController.cpp
Source/WebCore/page/Navigator.cpp
Source/WebCore/page/Navigator.h
Source/WebCore/platform/mock/GeolocationClientMock.cpp
Source/WebCore/platform/mock/GeolocationClientMock.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/public/WebGeolocationClientMock.h
Source/WebKit/chromium/src/WebGeolocationClientMock.cpp
Tools/ChangeLog
Tools/DumpRenderTree/LayoutTestController.cpp
Tools/DumpRenderTree/LayoutTestController.h
Tools/DumpRenderTree/chromium/LayoutTestController.cpp
Tools/DumpRenderTree/chromium/LayoutTestController.h
Tools/DumpRenderTree/gtk/LayoutTestControllerGtk.cpp
Tools/DumpRenderTree/mac/LayoutTestControllerMac.mm
Tools/DumpRenderTree/mac/UIDelegate.h
Tools/DumpRenderTree/mac/UIDelegate.mm
Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp
Tools/DumpRenderTree/qt/LayoutTestControllerQt.h
Tools/DumpRenderTree/win/LayoutTestControllerWin.cpp
Tools/DumpRenderTree/wx/LayoutTestControllerWx.cpp

index 4e36c77..4d406fc 100644 (file)
@@ -1,3 +1,21 @@
+2011-02-16  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Dmitry Titov.
+
+        Detach Geolocation from Frame when Page destroyed.
+        https://bugs.webkit.org/show_bug.cgi?id=52877
+
+        Ensure that all geolocation permission requests are cancelled
+        when the page is detached from its frame.
+
+        * fast/dom/Geolocation/page-reload-cancel-permission-requests-expected.txt: Added.
+        * fast/dom/Geolocation/page-reload-cancel-permission-requests.html: Added.
+        * fast/dom/Geolocation/resources/page-reload-cancel-permission-requests-inner.html: Added.
+        * fast/dom/Geolocation/script-tests/page-reload-cancel-permission-requests.js: Added.
+        * platform/gtk/Skipped:
+        * platform/mac/Skipped:
+        * platform/qt-wk2/Skipped:
+
 2011-03-10  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         Unreviewed.
diff --git a/LayoutTests/fast/dom/Geolocation/page-reload-cancel-permission-requests-expected.txt b/LayoutTests/fast/dom/Geolocation/page-reload-cancel-permission-requests-expected.txt
new file mode 100644 (file)
index 0000000..c7f2d2b
--- /dev/null
@@ -0,0 +1,10 @@
+Tests that when a page is reloaded, the frame is properly detached from the Geolocation object to ensure that no permission requests are in progress.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS numPendingRequests is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Geolocation/page-reload-cancel-permission-requests.html b/LayoutTests/fast/dom/Geolocation/page-reload-cancel-permission-requests.html
new file mode 100644 (file)
index 0000000..5b6aa5c
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">\r
+<html>\r
+<head>\r
+<link rel="stylesheet" href="../../js/resources/js-test-style.css">\r
+<script src="../../js/resources/js-test-pre.js"></script>\r
+</head>\r
+<body>\r
+<p id="description"></p>\r
+<div id="console"></div>\r
+<script src="script-tests/page-reload-cancel-permission-requests.js"></script>\r
+<script src="../../js/resources/js-test-post.js"></script>\r
+</body>\r
+</html>\r
diff --git a/LayoutTests/fast/dom/Geolocation/resources/page-reload-cancel-permission-requests-inner.html b/LayoutTests/fast/dom/Geolocation/resources/page-reload-cancel-permission-requests-inner.html
new file mode 100644 (file)
index 0000000..4a4a715
--- /dev/null
@@ -0,0 +1,7 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+  <head>
+  </head>
+  <body onload="window.parent.onIframeReady()">
+  </body>
+</html>
diff --git a/LayoutTests/fast/dom/Geolocation/script-tests/page-reload-cancel-permission-requests.js b/LayoutTests/fast/dom/Geolocation/script-tests/page-reload-cancel-permission-requests.js
new file mode 100644 (file)
index 0000000..7ac291b
--- /dev/null
@@ -0,0 +1,44 @@
+description("Tests that when a page is reloaded, the frame is properly detached from the Geolocation object " +
+            "to ensure that no permission requests are in progress.");
+
+window.jsTestIsAsync = true;
+
+var numPendingRequests;
+var isReload = false;
+
+if ("#reload" == location.hash)
+    isReload = true;
+
+if (window.layoutTestController) {
+    numPendingRequests = layoutTestController.numberOfPendingGeolocationPermissionRequests();
+    shouldBe('numPendingRequests', '0');
+
+    if (isReload)
+        finishJSTest();
+}
+
+if (!isReload) {
+    // Kick off a position request and then reload the page, this should set up
+    // a permission request. Permission should be undecided at this point, so the
+    // permission request should still be outstanding by page reload.
+
+    function onIframeReady()
+    {
+        // Make request on remote frame's Geolocation object.
+        iframe.contentWindow.navigator.geolocation.getCurrentPosition(
+            function(p) {
+                testFailed('Permission should not be determined for this page: ' + p);
+                finishJSTest();
+            });
+
+        location.hash = '#reload';
+        location.reload();
+    }
+
+    debug("Create IFrame");
+    var iframe = document.createElement('iframe');
+    iframe.src = 'resources/page-reload-cancel-permission-requests-inner.html';
+    document.body.appendChild(iframe);
+}
+
+window.successfullyParsed = true;
index 211adbe..7bb6201 100644 (file)
@@ -765,6 +765,11 @@ fast/dom/Geolocation/timeout.html
 fast/dom/Geolocation/timestamp.html
 fast/dom/Geolocation/watch.html
 fast/dom/Geolocation/window-close-crash.html
+fast/dom/Geolocation/iframe-reparent.html
+
+# Needs LayoutTestController::numberOfPendingGeolocationPermissionRequests()
+# See https://bugs.webkit.org/show_bug.cgi?id=54197
+fast/dom/Geolocation/page-reload-cancel-permission-requests.html
 
 # We're not quite ready to run these tests, because the slaves need
 # Geoclue support. If we implement support for the mock geolocation service
index 5aef256..6b7435c 100644 (file)
@@ -305,3 +305,7 @@ compositing/webgl/webgl-nonpremultiplied-blend.html
 
 # DataTransferItems is not yet implemented.
 editing/pasteboard/data-transfer-items.html
+
+# Need to implement WebGeolocationRequest::cancelPermissionRequest on mac.
+# https://bugs.webkit.org/show_bug.cgi?id=55944
+fast/dom/Geolocation/page-reload-cancel-permission-requests.html
\ No newline at end of file
index 69a8f1c..30f726c 100644 (file)
@@ -1722,8 +1722,10 @@ fast/dom/Geolocation/disconnected-frame-already.html
 fast/dom/Geolocation/disconnected-frame-permission-denied.html
 fast/dom/Geolocation/disconnected-frame.html
 fast/dom/Geolocation/error.html
+fast/dom/Geolocation/iframe-reparent.html
 fast/dom/Geolocation/multiple-requests.html
 fast/dom/Geolocation/notimer-after-unload.html
+fast/dom/Geolocation/page-reload-cancel-permission-requests.html
 fast/dom/Geolocation/permission-denied-already-clear-watch.html
 fast/dom/Geolocation/permission-denied-already-error.html
 fast/dom/Geolocation/permission-denied-already-success.html
index 67efee2..4426785 100644 (file)
@@ -1,3 +1,73 @@
+2011-01-21  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Dmitry Titov.
+
+        Detach Geolocation from Frame when Page destroyed.
+        https://bugs.webkit.org/show_bug.cgi?id=52877
+
+        On Page destruction, any outstanding Geolocation permission
+        requests should be cancelled, because the Geolocation can only
+        access the client indirectly via m_frame->page().
+
+        Page destruction is signalled by a call to the
+        Frame::pageDestroyed() method. This explictly calls
+        DOMWindow::resetGeolocation which ultimately calls Geolocation::reset.
+
+        Geolocation::reset() detaches from the GeolocationController,
+        cancels requests, watches and single shots, and sets the
+        permission state back to Unknown.
+
+        Frame::pageDestroyed() is also called by FrameLoader even though
+        the page is not destroyed. We should still cancel permission
+        requests, because the GeolocationClient will become inaccessible
+        to the Geolocation object after this call.
+
+        Frame::transferChildFrameToNewDocument also indirectly calls
+        Geolocation::reset when the frame is reparented between
+        pages. Ideally we would like the Geolocation's activities to
+        continue after reparenting, see bug
+        https://bugs.webkit.org/show_bug.cgi?id=55577
+
+        Since GeolocationController is owned by Page, and all Geolocation
+        objects will now unsubscribe from the GeolocationController on
+        pageDetached(), we no longer need to call stopUpdating() from the
+        GeolocationController's destructor. Instead we can simply assert
+        that there should be no no observers. See related bug
+        https://bugs.webkit.org/show_bug.cgi?id=52216 .
+
+        Introduced new method 'numberOfPendingPermissionRequests' on
+        GeolocationClientMock to count the number of outstanding pending
+        permission requests. This provides a reusable implementation for
+        client-based implementations of the LayoutTestController's
+        numberOfPendingGeolocationPermissionRequests method.
+
+        Test: fast/dom/Geolocation/page-reload-cancel-permission-requests.html
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::resetGeolocation):
+        * page/DOMWindow.h:
+        * page/Frame.cpp:
+        (WebCore::Frame::pageDestroyed):
+        (WebCore::Frame::transferChildFrameToNewDocument):
+        * page/Geolocation.cpp:
+        (WebCore::Geolocation::~Geolocation):
+        (WebCore::Geolocation::page):
+        (WebCore::Geolocation::reset):
+        (WebCore::Geolocation::disconnectFrame):
+        (WebCore::Geolocation::lastPosition):
+        (WebCore::Geolocation::requestPermission):
+        (WebCore::Geolocation::startUpdating):
+        (WebCore::Geolocation::stopUpdating):
+        * page/Geolocation.h:
+        * page/GeolocationController.cpp:
+        (WebCore::GeolocationController::~GeolocationController):
+        * page/Navigator.cpp:
+        (WebCore::Navigator::resetGeolocation):
+        * page/Navigator.h:
+        * platform/mock/GeolocationClientMock.cpp:
+        (WebCore::GeolocationClientMock::numberOfPendingPermissionRequests):
+        * platform/mock/GeolocationClientMock.h:
+
 2011-03-10  Ojan Vafai  <ojan@chromium.org>
 
         Reviewed by Darin Adler.
index 22ec0cc..5375541 100644 (file)
@@ -711,6 +711,13 @@ void DOMWindow::pageDestroyed()
 #endif
 }
 
+void DOMWindow::resetGeolocation()
+{
+    // Geolocation should cancel activities and permission requests when the page is detached.
+    if (m_navigator)
+        m_navigator->resetGeolocation();
+}
+
 #if ENABLE(INDEXED_DATABASE)
 IDBFactory* DOMWindow::webkitIndexedDB() const
 {
index af984c9..c146d11 100644 (file)
@@ -222,6 +222,7 @@ namespace WebCore {
         String crossDomainAccessErrorMessage(DOMWindow* activeWindow);
 
         void pageDestroyed();
+        void resetGeolocation();
 
         void postMessage(PassRefPtr<SerializedScriptValue> message, const MessagePortArray*, const String& targetOrigin, DOMWindow* source, ExceptionCode&);
         // FIXME: remove this when we update the ObjC bindings (bug #28774).
index 6458b93..3e4bb2e 100644 (file)
@@ -686,8 +686,10 @@ void Frame::pageDestroyed()
     if (Frame* parent = tree()->parent())
         parent->loader()->checkLoadComplete();
 
-    if (m_domWindow)
+    if (m_domWindow) {
+        m_domWindow->resetGeolocation();
         m_domWindow->pageDestroyed();
+    }
 
     // FIXME: It's unclear as to why this is called more than once, but it is,
     // so page() could be NULL.
@@ -732,6 +734,13 @@ void Frame::transferChildFrameToNewDocument()
              m_page->decrementFrameCount();
         }
 
+        // FIXME: We should ideally allow existing Geolocation activities to continue
+        // when the Geolocation's iframe is reparented.
+        // See https://bugs.webkit.org/show_bug.cgi?id=55577
+        // and https://bugs.webkit.org/show_bug.cgi?id=52877
+        if (m_domWindow)
+            m_domWindow->resetGeolocation();
+
         m_page = newPage;
 
         if (newPage)
index 40f53a1..2a4635f 100644 (file)
@@ -228,19 +228,35 @@ Geolocation::Geolocation(Frame* frame)
 
 Geolocation::~Geolocation()
 {
+    ASSERT(m_allowGeolocation != InProgress);
+    ASSERT(!m_frame);
 }
 
-void Geolocation::disconnectFrame()
+Page* Geolocation::page() const
+{
+    return m_frame ? m_frame->page() : 0;
+}
+
+void Geolocation::reset()
 {
-    if (m_frame && m_frame->page() && m_allowGeolocation == InProgress) {
+    Page* page = this->page();
+    if (page && m_allowGeolocation == InProgress) {
 #if ENABLE(CLIENT_BASED_GEOLOCATION)
-        m_frame->page()->geolocationController()->cancelPermissionRequest(this);
+        page->geolocationController()->cancelPermissionRequest(this);
 #else
-        m_frame->page()->chrome()->cancelGeolocationPermissionRequestForFrame(m_frame, this);
+        page->chrome()->cancelGeolocationPermissionRequestForFrame(m_frame, this);
 #endif
     }
+    // The frame may be moving to a new page and we want to get the permissions from the new page's client.
+    m_allowGeolocation = Unknown;
     cancelAllRequests();
     stopUpdating();
+}
+
+void Geolocation::disconnectFrame()
+{
+    // Once we are disconnected from the Frame, it is no longer possible to perform any operations.
+    reset();
     if (m_frame && m_frame->document())
         m_frame->document()->setUsingGeolocation(false);
     m_frame = 0;
@@ -249,10 +265,7 @@ void Geolocation::disconnectFrame()
 Geoposition* Geolocation::lastPosition()
 {
 #if ENABLE(CLIENT_BASED_GEOLOCATION)
-    if (!m_frame)
-        return 0;
-
-    Page* page = m_frame->page();
+    Page* page = this->page();
     if (!page)
         return 0;
 
@@ -591,10 +604,7 @@ void Geolocation::requestPermission()
     if (m_allowGeolocation > Unknown)
         return;
 
-    if (!m_frame)
-        return;
-
-    Page* page = m_frame->page();
+    Page* page = this->page();
     if (!page)
         return;
 
@@ -688,10 +698,7 @@ void Geolocation::geolocationServiceErrorOccurred(GeolocationService* service)
 bool Geolocation::startUpdating(GeoNotifier* notifier)
 {
 #if ENABLE(CLIENT_BASED_GEOLOCATION)
-    if (!m_frame)
-        return false;
-
-    Page* page = m_frame->page();
+    Page* page = this->page();
     if (!page)
         return false;
 
@@ -705,10 +712,7 @@ bool Geolocation::startUpdating(GeoNotifier* notifier)
 void Geolocation::stopUpdating()
 {
 #if ENABLE(CLIENT_BASED_GEOLOCATION)
-    if (!m_frame)
-        return;
-
-    Page* page = m_frame->page();
+    Page* page = this->page();
     if (!page)
         return;
 
@@ -749,6 +753,8 @@ namespace WebCore {
 
 void Geolocation::clearWatch(int) {}
 
+void Geolocation::reset() {}
+
 void Geolocation::disconnectFrame() {}
 
 Geolocation::Geolocation(Frame*) {}
index 7c06ae4..b52ad45 100644 (file)
@@ -58,6 +58,7 @@ public:
 
     ~Geolocation();
 
+    void reset();
     void disconnectFrame();
     
     void getCurrentPosition(PassRefPtr<PositionCallback>, PassRefPtr<PositionErrorCallback>, PassRefPtr<PositionOptions>);
@@ -86,6 +87,8 @@ private:
     
     Geolocation(Frame*);
 
+    Page* page() const;
+
     class GeoNotifier : public RefCounted<GeoNotifier> {
     public:
         static PassRefPtr<GeoNotifier> create(Geolocation* geolocation, PassRefPtr<PositionCallback> positionCallback, PassRefPtr<PositionErrorCallback> positionErrorCallback, PassRefPtr<PositionOptions> options) { return adoptRef(new GeoNotifier(geolocation, positionCallback, positionErrorCallback, options)); }
index 764b913..b9533ca 100644 (file)
@@ -41,12 +41,10 @@ GeolocationController::GeolocationController(Page* page, GeolocationClient* clie
 
 GeolocationController::~GeolocationController()
 {
-    if (m_client) {
-        if (!m_observers.isEmpty())
-            m_client->stopUpdating();
+    ASSERT(m_observers.isEmpty());
 
+    if (m_client)
         m_client->geolocationDestroyed();
-    }
 }
 
 void GeolocationController::addObserver(Geolocation* observer, bool enableHighAccuracy)
index c8237ad..6a267ad 100644 (file)
@@ -55,6 +55,12 @@ Navigator::~Navigator()
     disconnectFrame();
 }
 
+void Navigator::resetGeolocation()
+{
+    if (m_geolocation)
+        m_geolocation->reset();
+}
+
 void Navigator::disconnectFrame()
 {
     if (m_plugins) {
index 79573b3..693bde1 100644 (file)
@@ -41,6 +41,7 @@ public:
     static PassRefPtr<Navigator> create(Frame* frame) { return adoptRef(new Navigator(frame)); }
     virtual ~Navigator();
 
+    void resetGeolocation();
     void disconnectFrame();
     Frame* frame() const { return m_frame; }
 
index 5255b34..d067c8a 100644 (file)
@@ -79,6 +79,11 @@ void GeolocationClientMock::setPermission(bool allowed)
     asyncUpdatePermission();
 }
 
+int GeolocationClientMock::numberOfPendingPermissionRequests() const
+{
+    return m_pendingPermission.size();
+}
+
 void GeolocationClientMock::requestPermission(Geolocation* geolocation)
 {
     m_pendingPermission.add(geolocation);
index df35316..b400166 100644 (file)
@@ -59,6 +59,7 @@ public:
     void setError(PassRefPtr<GeolocationError>);
     void setPosition(PassRefPtr<GeolocationPosition>);
     void setPermission(bool allowed);
+    int numberOfPendingPermissionRequests() const;
 
     // GeolocationClient
     virtual void geolocationDestroyed();
index 0cba419..ad6d361 100644 (file)
@@ -1,3 +1,17 @@
+2011-01-26  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Dmitry Titov.
+
+        Detach Geolocation from Frame when Page destroyed.
+        https://bugs.webkit.org/show_bug.cgi?id=52877
+
+        Add accessors to the WebGeolocationClientMock to allow the number of
+        pending geolocation permission requests to be queried.
+
+        * public/WebGeolocationClientMock.h:
+        * src/WebGeolocationClientMock.cpp:
+        (WebKit::WebGeolocationClientMock::numberOfPendingPermissionRequests):
+
 2011-03-08  Hans Wennborg  <hans@chromium.org>
 
         Reviewed by Jeremy Orlow.
index 08a85e2..9a39e2e 100644 (file)
@@ -51,6 +51,7 @@ public:
     WEBKIT_API void setPosition(double latitude, double longitude, double accuracy);
     WEBKIT_API void setError(int errorCode, const WebString& message);
     WEBKIT_API void setPermission(bool);
+    WEBKIT_API int numberOfPendingPermissionRequests() const;
     WEBKIT_API void resetMock();
 
     virtual void startUpdating();
index 1ec3dd1..0ad47da 100644 (file)
@@ -82,6 +82,11 @@ void WebGeolocationClientMock::setPermission(bool allowed)
     m_clientMock->setPermission(allowed);
 }
 
+int WebGeolocationClientMock::numberOfPendingPermissionRequests() const
+{
+    return m_clientMock->numberOfPendingPermissionRequests();
+}
+
 void WebGeolocationClientMock::resetMock()
 {
     m_clientMock->reset();
index 012c066..885dde7 100644 (file)
@@ -1,3 +1,38 @@
+2011-01-26  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Dmitry Titov.
+
+        Detach Geolocation from Frame when Page destroyed.
+        https://bugs.webkit.org/show_bug.cgi?id=52877
+
+        Extend the layout test controller to expose the number of pending
+        geolocation requests, so that we can test that the requests have
+        been cancelled on page close.
+
+        * DumpRenderTree/LayoutTestController.cpp:
+        (numberOfPendingGeolocationPermissionRequestsCallback):
+        (LayoutTestController::staticFunctions):
+        * DumpRenderTree/LayoutTestController.h:
+        * DumpRenderTree/chromium/LayoutTestController.cpp:
+        (LayoutTestController::LayoutTestController):
+        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
+        * DumpRenderTree/chromium/LayoutTestController.h:
+        * DumpRenderTree/gtk/LayoutTestControllerGtk.cpp:
+        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
+        * DumpRenderTree/mac/LayoutTestControllerMac.mm:
+        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
+        * DumpRenderTree/mac/UIDelegate.h:
+        * DumpRenderTree/mac/UIDelegate.mm:
+        (-[UIDelegate numberOfPendingGeolocationPermissionRequests]):
+        * DumpRenderTree/qt/LayoutTestControllerQt.cpp:
+        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
+        * DumpRenderTree/qt/LayoutTestControllerQt.h:
+        * DumpRenderTree/win/LayoutTestControllerWin.cpp:
+        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
+        * DumpRenderTree/wx/LayoutTestControllerWx.cpp:
+        (LayoutTestController::numberOfPendingGeolocationPermissionRequests):
+
+
 2011-03-09  Adam Roben  <aroben@apple.com>
 
         Hide Leaks Viewer's URL prompt by default
index 9f1877c..6509db7 100644 (file)
@@ -769,6 +769,12 @@ static JSValueRef numberOfPagesCallback(JSContextRef context, JSObjectRef functi
     return JSValueMakeNumber(context, controller->numberOfPages(pageWidthInPixels, pageHeightInPixels));
 }
 
+static JSValueRef numberOfPendingGeolocationPermissionRequestsCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
+{
+    LayoutTestController* controller = static_cast<LayoutTestController*>(JSObjectGetPrivate(thisObject));
+    return JSValueMakeNumber(context, controller->numberOfPendingGeolocationPermissionRequests());
+}
+
 static JSValueRef pagePropertyCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     char* propertyName = 0;
@@ -2074,6 +2080,7 @@ JSStaticFunction* LayoutTestController::staticFunctions()
         { "keepWebHistory", keepWebHistoryCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "layerTreeAsText", layerTreeAsTextCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "numberOfPages", numberOfPagesCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
+        { "numberOfPendingGeolocationPermissionRequests", numberOfPendingGeolocationPermissionRequestsCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "markerTextForListItem", markerTextForListItemCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "notifyDone", notifyDoneCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "numberOfActiveAnimations", numberOfActiveAnimationsCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
index 4b1c04e..1d72e8a 100644 (file)
@@ -68,6 +68,7 @@ public:
     JSValueRef nodesFromRect(JSContextRef, JSValueRef, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping);
     void notifyDone();
     int numberOfPages(float pageWidthInPixels, float pageHeightInPixels);
+    int numberOfPendingGeolocationPermissionRequests();
     void overridePreference(JSStringRef key, JSStringRef value);
     int pageNumberForElementById(JSStringRef id, float pageWidthInPixels, float pageHeightInPixels);
     JSRetainPtr<JSStringRef> pageProperty(const char* propertyName, int pageNumber) const;
index b891d1e..c2b1b12 100644 (file)
@@ -115,6 +115,7 @@ LayoutTestController::LayoutTestController(TestShell* shell)
     bindMethod("notifyDone", &LayoutTestController::notifyDone);
     bindMethod("numberOfActiveAnimations", &LayoutTestController::numberOfActiveAnimations);
     bindMethod("numberOfPages", &LayoutTestController::numberOfPages);
+    bindMethod("numberOfPendingGeolocationPermissionRequests", &LayoutTestController:: numberOfPendingGeolocationPermissionRequests);
     bindMethod("objCIdentityIsEqual", &LayoutTestController::objCIdentityIsEqual);
     bindMethod("overridePreference", &LayoutTestController::overridePreference);
     bindMethod("pageNumberForElementById", &LayoutTestController::pageNumberForElementById);
@@ -1452,6 +1453,16 @@ void LayoutTestController::numberOfPages(const CppArgumentList& arguments, CppVa
     result->set(numberOfPages);
 }
 
+void LayoutTestController::numberOfPendingGeolocationPermissionRequests(const CppArgumentList& arguments, CppVariant* result)
+{
+    result->setNull();
+    Vector<WebViewHost*> windowList = m_shell->windowList();
+    int numberOfRequests = 0;
+    for (size_t i = 0; i < windowList.size(); i++)
+        numberOfRequests += windowList[i]->geolocationClientMock()->numberOfPendingPermissionRequests();
+    result->set(numberOfRequests);
+}
+
 void LayoutTestController::logErrorToConsole(const std::string& text)
 {
     m_shell->webViewHost()->didAddMessageToConsole(
index 6150133..a8fcd99 100644 (file)
@@ -305,6 +305,8 @@ public:
     // Gets the number of pages to be printed.
     void numberOfPages(const CppArgumentList&, CppVariant*);
 
+    // Gets the number of geolocation permissions requests pending.
+    void numberOfPendingGeolocationPermissionRequests(const CppArgumentList&, CppVariant*);
 
     // Allows layout tests to start Timeline profiling.
     void setTimelineProfilingEnabled(const CppArgumentList&, CppVariant*);
index e73eb46..7697cab 100644 (file)
@@ -488,6 +488,12 @@ void LayoutTestController::setGeolocationPermission(bool allow)
     setGeolocationPermissionCommon(allow);
 }
 
+int LayoutTestController::numberOfPendingGeolocationPermissionRequests()
+{
+    // FIXME: Implement for Geolocation layout tests.
+    return -1;
+}
+
 void LayoutTestController::addMockSpeechInputResult(JSStringRef result, double confidence, JSStringRef language)
 {
     // FIXME: Implement for speech input layout tests.
index 72ec759..3dfedd0 100644 (file)
@@ -254,6 +254,11 @@ int LayoutTestController::numberOfPages(float pageWidthInPixels, float pageHeigh
     return [mainFrame numberOfPages:pageWidthInPixels:pageHeightInPixels];
 }
 
+int LayoutTestController::numberOfPendingGeolocationPermissionRequests()
+{
+    return [[[mainFrame webView] UIDelegate] numberOfPendingGeolocationPermissionRequests];
+}
+
 size_t LayoutTestController::webHistoryItemCount()
 {
     return [[[WebHistory optionalSharedHistory] allItems] count];
index a8017ad..982b480 100644 (file)
@@ -37,5 +37,6 @@
 }
 
 - (void)didSetMockGeolocationPermission;
+- (int)numberOfPendingGeolocationPermissionRequests;
 
 @end
index 06a71f8..efcb47a 100644 (file)
@@ -205,6 +205,14 @@ DumpRenderTreeDraggingInfo *draggingInfo = nil;
         m_timer = [NSTimer scheduledTimerWithTimeInterval:0 target:self selector:@selector(timerFired) userInfo:0 repeats:NO];
 }
 
+- (int)numberOfPendingGeolocationPermissionRequests
+{
+    if (!m_pendingGeolocationPermissionListeners)
+        return 0;
+    return [m_pendingGeolocationPermissionListeners count];
+}
+
+
 - (void)timerFired
 {
     ASSERT(gLayoutTestController->isGeolocationPermissionSet());
index 74055e2..ddaf73e 100644 (file)
@@ -766,6 +766,12 @@ void LayoutTestController::setGeolocationPermission(bool allow)
     DumpRenderTreeSupportQt::setMockGeolocationPermission(m_drt->webPage(), allow);
 }
 
+int LayoutTestController::numberOfPendingGeolocationPermissionRequests()
+{
+    // FIXME: Implement for Geolocation layout tests.
+    return -1;
+}
+
 void LayoutTestController::setGeolocationPermissionCommon(bool allow)
 {
      m_isGeolocationPermissionSet = true;
index 0b5bbba..31a1faf 100644 (file)
@@ -219,6 +219,7 @@ public slots:
     void setMockGeolocationError(int code, const QString& message);
     void setMockGeolocationPosition(double latitude, double longitude, double accuracy);
     void setGeolocationPermission(bool allow);
+    int numberOfPendingGeolocationPermissionRequests();
     bool isGeolocationPermissionSet() const { return m_isGeolocationPermissionSet; }
     bool geolocationPermission() const { return m_geolocationPermission; }
 
index 12a3a55..ae777ac 100644 (file)
@@ -411,6 +411,12 @@ void LayoutTestController::setGeolocationPermission(bool allow)
     setGeolocationPermissionCommon(allow);
 }
 
+int LayoutTestController::numberOfPendingGeolocationPermissionRequests()
+{
+    // FIXME: Implement for Geolocation layout tests.
+    return -1;
+}
+
 void LayoutTestController::addMockSpeechInputResult(JSStringRef result, double confidence, JSStringRef language)
 {
     // FIXME: Implement for speech input layout tests.
index a95aa50..e5b7541 100644 (file)
@@ -325,6 +325,12 @@ void LayoutTestController::setGeolocationPermission(bool allow)
     setGeolocationPermissionCommon(allow);
 }
 
+int LayoutTestController::numberOfPendingGeolocationPermissionRequests()
+{
+    // FIXME: Implement for Geolocation layout tests.
+    return -1;
+}
+
 void LayoutTestController::addMockSpeechInputResult(JSStringRef result, double confidence, JSStringRef language)
 {
     // FIXME: Implement for speech input layout tests.