2011-01-17 John Knottenbelt <jknotten@chromium.org>
authorjknotten@chromium.org <jknotten@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Jan 2011 12:21:55 +0000 (12:21 +0000)
committerjknotten@chromium.org <jknotten@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Jan 2011 12:21:55 +0000 (12:21 +0000)
        Reviewed by Jeremy Orlow.

        GeolocationController should call stopUpdating on destruction
        https://bugs.webkit.org/show_bug.cgi?id=52216

        fast/dom/window-close-crash.html tests that we do not fail the
        assertion in the mock GeolocationClient that the GeolocationClient
        is not updating when the GeolocationController is destroyed.

        * fast/dom/Geolocation/resources/window-close-popup.html: Added.
        * fast/dom/Geolocation/script-tests/window-close-crash.js: Added.
        (gotPosition):
        (waitForWindowToClose):
        (failedToCreateWatch):
        * fast/dom/Geolocation/window-close-crash-expected.txt: Added.
        * fast/dom/Geolocation/window-close-crash.html: Added.
        * platform/gtk/Skipped:
        * platform/mac-wk2/Skipped:
        * platform/qt-wk2/Skipped:
2011-01-17  John Knottenbelt  <jknotten@chromium.org>

        Reviewed by Jeremy Orlow.

        GeolocationController should call stopUpdating on destruction
        https://bugs.webkit.org/show_bug.cgi?id=52216

        Test: fast/dom/Geolocation/window-close-crash.html

        * page/GeolocationController.cpp:
        (WebCore::GeolocationController::~GeolocationController):
2011-01-17  John Knottenbelt  <jknotten@chromium.org>

        Reviewed by Jeremy Orlow.

        GeolocationController should call stopUpdating on destruction
        https://bugs.webkit.org/show_bug.cgi?id=52216

        fast/dom/Geolocation/window-close-crash.html requires that a
        Geolocation watch be started in a secondary window. Consequently,
        we need to allow geolocation permission and provide a mock
        geolocation position for the secondary window's
        GeolocationClientMock.

        * DumpRenderTree/chromium/LayoutTestController.cpp:
        (LayoutTestController::setGeolocationPermission):
        (LayoutTestController::setMockGeolocationPosition):
        (LayoutTestController::setMockGeolocationError):
        * DumpRenderTree/chromium/TestShell.h:
        (TestShell::windowList):

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/Geolocation/resources/window-close-popup.html [new file with mode: 0644]
LayoutTests/fast/dom/Geolocation/script-tests/window-close-crash.js [new file with mode: 0644]
LayoutTests/fast/dom/Geolocation/window-close-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Geolocation/window-close-crash.html [new file with mode: 0644]
LayoutTests/platform/gtk/Skipped
LayoutTests/platform/mac-wk2/Skipped
LayoutTests/platform/qt-wk2/Skipped
Source/WebCore/ChangeLog
Source/WebCore/page/GeolocationController.cpp
Tools/ChangeLog
Tools/DumpRenderTree/chromium/LayoutTestController.cpp
Tools/DumpRenderTree/chromium/TestShell.h

index 58541a5..2e422aa 100644 (file)
@@ -1,3 +1,25 @@
+2011-01-17  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Jeremy Orlow.
+
+        GeolocationController should call stopUpdating on destruction
+        https://bugs.webkit.org/show_bug.cgi?id=52216
+
+        fast/dom/window-close-crash.html tests that we do not fail the
+        assertion in the mock GeolocationClient that the GeolocationClient
+        is not updating when the GeolocationController is destroyed.
+
+        * fast/dom/Geolocation/resources/window-close-popup.html: Added.
+        * fast/dom/Geolocation/script-tests/window-close-crash.js: Added.
+        (gotPosition):
+        (waitForWindowToClose):
+        (failedToCreateWatch):
+        * fast/dom/Geolocation/window-close-crash-expected.txt: Added.
+        * fast/dom/Geolocation/window-close-crash.html: Added.
+        * platform/gtk/Skipped:
+        * platform/mac-wk2/Skipped:
+        * platform/qt-wk2/Skipped:
+
 2011-01-14  Ilya Tikhonovsky  <loislo@chromium.org>
 
         Reviewed by Pavel Feldman.
diff --git a/LayoutTests/fast/dom/Geolocation/resources/window-close-popup.html b/LayoutTests/fast/dom/Geolocation/resources/window-close-popup.html
new file mode 100644 (file)
index 0000000..cfeabae
--- /dev/null
@@ -0,0 +1,19 @@
+<script>
+var mockLatitude = 51.478;
+var mockLongitude = -0.166;
+var mockAccuracy = 100.0;
+
+function loadNext() {
+    var geolocation = navigator.geolocation;
+
+    if (window.layoutTestController) {
+       layoutTestController.setGeolocationPermission(true);
+       layoutTestController.setMockGeolocationPosition(mockLatitude, mockLongitude, mockAccuracy);
+    }
+
+    navigator.geolocation.watchPosition(
+       function(p) { window.opener.gotPosition(); },
+       function(e) { window.opener.failedToCreateWatch(e); });
+}
+</script>
+<body onload="loadNext()"></body>
diff --git a/LayoutTests/fast/dom/Geolocation/script-tests/window-close-crash.js b/LayoutTests/fast/dom/Geolocation/script-tests/window-close-crash.js
new file mode 100644 (file)
index 0000000..b27df61
--- /dev/null
@@ -0,0 +1,41 @@
+description("Tests the assertion that the GeolocationClient should not be updating<br>" +
+            "when the GeolocationController is destroyed.<br>" +
+            "See https://bugs.webkit.org/show_bug.cgi?id=52216");
+
+var otherWindow;
+
+if (window.layoutTestController) {
+    layoutTestController.waitUntilDone();
+    layoutTestController.setCanOpenWindows();
+    layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+} else
+    testFailed('This test can not be run without the LayoutTestController');
+
+function gotPosition(p)
+{
+    testPassed("Received Geoposition.");
+    otherWindow.close();
+    window.setTimeout(waitForWindowToClose, 0);
+}
+
+function waitForWindowToClose()
+{
+    if (!otherWindow.closed) {
+        window.setTimeout(waitForWindowToClose, 0);
+        return;
+    }
+    testPassed("Success - no crash!");
+    finishJSTest();
+}
+
+function failedToCreateWatch(e)
+{
+    testFailed("Failed to create watch: " + e);
+    finishJSTest();
+}
+
+debug("Main page opening resources/window-close-popup.html");
+otherWindow = window.open("resources/window-close-popup.html");
+
+window.jsTestIsAsync = true;
+window.successfullyParsed = true;
diff --git a/LayoutTests/fast/dom/Geolocation/window-close-crash-expected.txt b/LayoutTests/fast/dom/Geolocation/window-close-crash-expected.txt
new file mode 100644 (file)
index 0000000..bc136e6
--- /dev/null
@@ -0,0 +1,14 @@
+Tests the assertion that the GeolocationClient should not be updating
+when the GeolocationController is destroyed.
+See https://bugs.webkit.org/show_bug.cgi?id=52216
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Main page opening resources/window-close-popup.html
+PASS Received Geoposition.
+PASS Success - no crash!
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Geolocation/window-close-crash.html b/LayoutTests/fast/dom/Geolocation/window-close-crash.html
new file mode 100644 (file)
index 0000000..fc23692
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../js/resources/js-test-style.css">
+<script src="../../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/window-close-crash.js"></script>
+<script src="../../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 1cf8462..7b53e75 100644 (file)
@@ -4909,6 +4909,7 @@ fast/dom/Geolocation/timeout-clear-watch.html
 fast/dom/Geolocation/timeout.html
 fast/dom/Geolocation/timestamp.html
 fast/dom/Geolocation/watch.html
+fast/dom/Geolocation/window-close-crash.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 e55c9b2..087bd26 100644 (file)
@@ -1584,6 +1584,7 @@ fast/dom/Geolocation/success.html
 fast/dom/Geolocation/timeout.html
 fast/dom/Geolocation/timestamp.html
 fast/dom/Geolocation/watch.html
+fast/dom/Geolocation/window-close-crash.html
 
 # WebKitTestRunner needs layoutTestController.setMockGeolocationPosition
 fast/dom/Geolocation/timeout-clear-watch.html
index ce96a25..fd22554 100644 (file)
@@ -1737,6 +1737,7 @@ fast/dom/Geolocation/reentrant-success.html
 fast/dom/Geolocation/success.html
 fast/dom/Geolocation/timeout.html
 fast/dom/Geolocation/watch.html
+fast/dom/Geolocation/window-close-crash.html
 
 # WebKitTestRunner needs layoutTestController.setMockGeolocationPosition
 fast/dom/Geolocation/timeout-clear-watch.html
index 0c40fda..2e509cf 100644 (file)
@@ -1,3 +1,15 @@
+2011-01-17  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Jeremy Orlow.
+
+        GeolocationController should call stopUpdating on destruction
+        https://bugs.webkit.org/show_bug.cgi?id=52216
+
+        Test: fast/dom/Geolocation/window-close-crash.html
+
+        * page/GeolocationController.cpp:
+        (WebCore::GeolocationController::~GeolocationController):
+
 2011-01-17  Pavel Feldman  <pfeldman@chromium.org>
 
         Not reviewed: Qt build fix.
index 28d522d..764b913 100644 (file)
@@ -41,8 +41,12 @@ GeolocationController::GeolocationController(Page* page, GeolocationClient* clie
 
 GeolocationController::~GeolocationController()
 {
-    if (m_client)
+    if (m_client) {
+        if (!m_observers.isEmpty())
+            m_client->stopUpdating();
+
         m_client->geolocationDestroyed();
+    }
 }
 
 void GeolocationController::addObserver(Geolocation* observer, bool enableHighAccuracy)
index 2a01e81..c9266c8 100644 (file)
@@ -1,3 +1,23 @@
+2011-01-17  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Jeremy Orlow.
+
+        GeolocationController should call stopUpdating on destruction
+        https://bugs.webkit.org/show_bug.cgi?id=52216
+
+        fast/dom/Geolocation/window-close-crash.html requires that a
+        Geolocation watch be started in a secondary window. Consequently,
+        we need to allow geolocation permission and provide a mock
+        geolocation position for the secondary window's
+        GeolocationClientMock.
+
+        * DumpRenderTree/chromium/LayoutTestController.cpp:
+        (LayoutTestController::setGeolocationPermission):
+        (LayoutTestController::setMockGeolocationPosition):
+        (LayoutTestController::setMockGeolocationError):
+        * DumpRenderTree/chromium/TestShell.h:
+        (TestShell::windowList):
+
 2011-01-16  Adam Barth  <abarth@webkit.org>
 
         Update more include paths to reflect WebKit move.
index 0b2cfc9..4671438 100644 (file)
@@ -1515,12 +1515,16 @@ void LayoutTestController::setMockDeviceOrientation(const CppArgumentList& argum
     m_shell->webViewHost()->deviceOrientationClientMock()->setOrientation(orientation);
 }
 
+// FIXME: For greater test flexibility, we should be able to set each page's geolocation mock individually.
+// https://bugs.webkit.org/show_bug.cgi?id=52368
 void LayoutTestController::setGeolocationPermission(const CppArgumentList& arguments, CppVariant* result)
 {
     result->setNull();
     if (arguments.size() < 1 || !arguments[0].isBool())
         return;
-    m_shell->webViewHost()->geolocationClientMock()->setPermission(arguments[0].toBoolean());
+    Vector<WebViewHost*> windowList = m_shell->windowList();
+    for (size_t i = 0; i < windowList.size(); i++)
+        windowList[i]->geolocationClientMock()->setPermission(arguments[0].toBoolean());
 }
 
 void LayoutTestController::setMockGeolocationPosition(const CppArgumentList& arguments, CppVariant* result)
@@ -1528,7 +1532,9 @@ void LayoutTestController::setMockGeolocationPosition(const CppArgumentList& arg
     result->setNull();
     if (arguments.size() < 3 || !arguments[0].isNumber() || !arguments[1].isNumber() || !arguments[2].isNumber())
         return;
-    m_shell->webViewHost()->geolocationClientMock()->setPosition(arguments[0].toDouble(), arguments[1].toDouble(), arguments[2].toDouble());
+    Vector<WebViewHost*> windowList = m_shell->windowList();
+    for (size_t i = 0; i < windowList.size(); i++)
+        windowList[i]->geolocationClientMock()->setPosition(arguments[0].toDouble(), arguments[1].toDouble(), arguments[2].toDouble());
 }
 
 void LayoutTestController::setMockGeolocationError(const CppArgumentList& arguments, CppVariant* result)
@@ -1536,7 +1542,9 @@ void LayoutTestController::setMockGeolocationError(const CppArgumentList& argume
     result->setNull();
     if (arguments.size() < 2 || !arguments[0].isNumber() || !arguments[1].isString())
         return;
-    m_shell->webViewHost()->geolocationClientMock()->setError(arguments[0].toInt32(), cppVariantToWebString(arguments[1]));
+    Vector<WebViewHost*> windowList = m_shell->windowList();
+    for (size_t i = 0; i < windowList.size(); i++)
+        windowList[i]->geolocationClientMock()->setError(arguments[0].toInt32(), cppVariantToWebString(arguments[1]));
 }
 
 void LayoutTestController::abortModal(const CppArgumentList& arguments, CppVariant* result)
index 486f9ea..ef9be7f 100644 (file)
@@ -166,6 +166,9 @@ public:
 
     static const int virtualWindowBorder = 3;
 
+    typedef Vector<WebViewHost*> WindowList;
+    WindowList windowList() const { return m_windowList; }
+
 private:
     WebViewHost* createNewWindow(const WebKit::WebURL&, DRTDevToolsAgent*);
     void createMainWindow();
@@ -207,7 +210,6 @@ private:
 
     // List of all windows in this process.
     // The main window should be put into windowList[0].
-    typedef Vector<WebViewHost*> WindowList;
     WindowList m_windowList;
 
 #if defined(OS_WIN)