2010-09-07 Dimitri Glazkov <dglazkov@chromium.org>
authordglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Sep 2010 17:38:22 +0000 (17:38 +0000)
committerdglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Sep 2010 17:38:22 +0000 (17:38 +0000)
        Unreviewed, rolling out r66886.
        http://trac.webkit.org/changeset/66886
        https://bugs.webkit.org/show_bug.cgi?id=45112

        Made

        * platform/chromium/GeolocationServiceChromium.cpp:
        * platform/chromium/GeolocationServiceChromium.h:
2010-09-07  Dimitri Glazkov  <dglazkov@chromium.org>

        Unreviewed, rolling out r66886.
        http://trac.webkit.org/changeset/66886
        https://bugs.webkit.org/show_bug.cgi?id=45112

        Made

        * public/WebGeolocationService.h:
        * src/WebGeolocationServiceBridgeImpl.cpp:
        (WebKit::WebGeolocationServiceBridgeImpl::WebGeolocationServiceBridgeImpl):
        (WebKit::WebGeolocationServiceBridgeImpl::~WebGeolocationServiceBridgeImpl):
        (WebKit::WebGeolocationServiceBridgeImpl::startUpdating):
        (WebKit::WebGeolocationServiceBridgeImpl::stopUpdating):
        (WebKit::WebGeolocationServiceBridgeImpl::suspend):
        (WebKit::WebGeolocationServiceBridgeImpl::resume):
        (WebKit::WebGeolocationServiceBridgeImpl::attachBridgeIfNeeded):
        (WebKit::WebGeolocationServiceBridgeImpl::setIsAllowed):
        (WebKit::WebGeolocationServiceBridgeImpl::setLastPosition):
        (WebKit::WebGeolocationServiceBridgeImpl::setLastError):
        (WebKit::WebGeolocationServiceBridgeImpl::getWebViewClient):
        (WebKit::WebGeolocationServiceBridgeImpl::onWebGeolocationServiceDestroyed):
        * src/WebGeolocationServiceMock.cpp:

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

WebCore/ChangeLog
WebCore/platform/chromium/GeolocationServiceChromium.cpp
WebCore/platform/chromium/GeolocationServiceChromium.h
WebKit/chromium/ChangeLog
WebKit/chromium/public/WebGeolocationService.h
WebKit/chromium/src/WebGeolocationServiceBridgeImpl.cpp
WebKit/chromium/src/WebGeolocationServiceMock.cpp

index ad5355f..77e3aa4 100644 (file)
@@ -1,3 +1,14 @@
+2010-09-07  Dimitri Glazkov  <dglazkov@chromium.org>
+
+        Unreviewed, rolling out r66886.
+        http://trac.webkit.org/changeset/66886
+        https://bugs.webkit.org/show_bug.cgi?id=45112
+
+        Made
+
+        * platform/chromium/GeolocationServiceChromium.cpp:
+        * platform/chromium/GeolocationServiceChromium.h:
+
 2010-09-07  Martin Robinson  <mrobinson@igalia.com>
 
         Small build fix. Remove libWebCoreJS from CLEAN_FILES, as it
index b64f5eb..9333999 100644 (file)
 
 namespace WebCore {
 
-GeolocationServiceBridge::~GeolocationServiceBridge()
-{
-}
-
 GeolocationServiceChromium::GeolocationServiceChromium(GeolocationServiceClient* c)
         : GeolocationService(c),
           m_geolocation(static_cast<Geolocation*>(c)),
index 7e6f633..f139220 100644 (file)
@@ -42,7 +42,6 @@ namespace WebCore {
 // Provides an interface for GeolocationServiceChromium to call into the embedder.
 class GeolocationServiceBridge {
 public:
-    virtual ~GeolocationServiceBridge();
     // Called by GeolocationServiceChromium.
     virtual bool startUpdating(PositionOptions*) = 0;
     virtual void stopUpdating() = 0;
index 9edfc01..023e1d7 100644 (file)
@@ -1,3 +1,27 @@
+2010-09-07  Dimitri Glazkov  <dglazkov@chromium.org>
+
+        Unreviewed, rolling out r66886.
+        http://trac.webkit.org/changeset/66886
+        https://bugs.webkit.org/show_bug.cgi?id=45112
+
+        Made
+
+        * public/WebGeolocationService.h:
+        * src/WebGeolocationServiceBridgeImpl.cpp:
+        (WebKit::WebGeolocationServiceBridgeImpl::WebGeolocationServiceBridgeImpl):
+        (WebKit::WebGeolocationServiceBridgeImpl::~WebGeolocationServiceBridgeImpl):
+        (WebKit::WebGeolocationServiceBridgeImpl::startUpdating):
+        (WebKit::WebGeolocationServiceBridgeImpl::stopUpdating):
+        (WebKit::WebGeolocationServiceBridgeImpl::suspend):
+        (WebKit::WebGeolocationServiceBridgeImpl::resume):
+        (WebKit::WebGeolocationServiceBridgeImpl::attachBridgeIfNeeded):
+        (WebKit::WebGeolocationServiceBridgeImpl::setIsAllowed):
+        (WebKit::WebGeolocationServiceBridgeImpl::setLastPosition):
+        (WebKit::WebGeolocationServiceBridgeImpl::setLastError):
+        (WebKit::WebGeolocationServiceBridgeImpl::getWebViewClient):
+        (WebKit::WebGeolocationServiceBridgeImpl::onWebGeolocationServiceDestroyed):
+        * src/WebGeolocationServiceMock.cpp:
+
 2010-09-07  Jonathan Dixon  <joth@chromium.org>
         
         Reviewed by Jeremy Orlow.
index ea91b58..ed8c4e8 100644 (file)
 #ifndef WebGeolocationService_h
 #define WebGeolocationService_h
 
+#include "WebGeolocationServiceBridge.h"
+
 namespace WebKit {
 
-class WebGeolocationServiceBridge;
 class WebString;
 class WebURL;
 
@@ -49,14 +50,10 @@ public:
 
     // Attaches the WebGeolocationServiceBridge to the embedder and returns its
     // id, which should be used on subsequent calls for the methods above.
-    // An ID of zero indicates the attach failed.
     virtual int attachBridge(WebGeolocationServiceBridge*) { return 0; }
 
     // Detaches the WebGeolocationServiceBridge from the embedder.
     virtual void detachBridge(int bridgeId) { }
-
-protected:
-    virtual ~WebGeolocationService() {}
 };
 
 } // namespace WebKit
index 4e7a4b6..bbb7162 100644 (file)
@@ -82,12 +82,10 @@ public:
     virtual void onWebGeolocationServiceDestroyed();
 
 private:
-    bool isAttached() const;
-    // Pointer back to the WebKit geolocation client. We obtain this via the frame's page, but need to cache it
-    // as it may still be alive after the page has detached from the frame.
-    WebGeolocationService* m_webGeolocationService;
+    WebViewClient* getWebViewClient();
+
     // GeolocationServiceChromium owns us, we only have a pointer back to it.
-    GeolocationServiceChromium* m_geolocationServiceChromium;
+    GeolocationServiceChromium* m_GeolocationServiceChromium;
     int m_bridgeId;
 };
 
@@ -97,47 +95,49 @@ GeolocationServiceBridge* createGeolocationServiceBridgeImpl(GeolocationServiceC
 }
 
 WebGeolocationServiceBridgeImpl::WebGeolocationServiceBridgeImpl(GeolocationServiceChromium* geolocationServiceChromium)
-    : m_webGeolocationService(0)
-    , m_geolocationServiceChromium(geolocationServiceChromium)
+    : m_GeolocationServiceChromium(geolocationServiceChromium)
     , m_bridgeId(0)
 {
 }
 
 WebGeolocationServiceBridgeImpl::~WebGeolocationServiceBridgeImpl()
 {
-    if (isAttached())
-        m_webGeolocationService->detachBridge(m_bridgeId);
+    WebKit::WebViewClient* webViewClient = getWebViewClient();
+    // Geolocation has an OwnPtr to us, and it's destroyed after the frame has
+    // been potentially disconnected. In this case, it calls stopUpdating()
+    // has been called and we have already detached ourselves.
+    if (!webViewClient)
+        ASSERT(!m_bridgeId);
+    else if (m_bridgeId)
+        webViewClient->geolocationService()->detachBridge(m_bridgeId);
 }
 
 bool WebGeolocationServiceBridgeImpl::startUpdating(PositionOptions* positionOptions)
 {
     attachBridgeIfNeeded();
-    if (!isAttached())
-        return false;
-    m_webGeolocationService->startUpdating(m_bridgeId, m_geolocationServiceChromium->frame()->document()->url(), positionOptions->enableHighAccuracy());
+    getWebViewClient()->geolocationService()->startUpdating(m_bridgeId, m_GeolocationServiceChromium->frame()->document()->url(), positionOptions->enableHighAccuracy());
     return true;
 }
 
 void WebGeolocationServiceBridgeImpl::stopUpdating()
 {
-    if (isAttached()) {
-        m_webGeolocationService->stopUpdating(m_bridgeId);
-        m_webGeolocationService->detachBridge(m_bridgeId);
-        m_bridgeId = 0;
-        m_webGeolocationService = 0;
+    WebViewClient* webViewClient = getWebViewClient();
+    if (m_bridgeId && webViewClient) {
+        WebGeolocationService* geolocationService = webViewClient->geolocationService();
+        geolocationService->stopUpdating(m_bridgeId);
+        geolocationService->detachBridge(m_bridgeId);
     }
+    m_bridgeId = 0;
 }
 
 void WebGeolocationServiceBridgeImpl::suspend()
 {
-    if (isAttached())
-        m_webGeolocationService->suspend(m_bridgeId);
+    getWebViewClient()->geolocationService()->suspend(m_bridgeId);
 }
 
 void WebGeolocationServiceBridgeImpl::resume()
 {
-    if (isAttached())
-        m_webGeolocationService->resume(m_bridgeId);
+    getWebViewClient()->geolocationService()->resume(m_bridgeId);
 }
 
 int WebGeolocationServiceBridgeImpl::getBridgeId() const
@@ -147,54 +147,38 @@ int WebGeolocationServiceBridgeImpl::getBridgeId() const
 
 void WebGeolocationServiceBridgeImpl::attachBridgeIfNeeded()
 {
-    if (isAttached())
-        return;
-    // Lazy attach to the geolocation service of the associated page if there is one.
-    Frame* frame = m_geolocationServiceChromium->frame();
-    if (!frame || !frame->page())
-        return;
-    WebKit::ChromeClientImpl* chromeClientImpl = static_cast<WebKit::ChromeClientImpl*>(frame->page()->chrome()->client());
-    WebKit::WebViewClient* webViewClient = chromeClientImpl->webView()->client();
-    m_webGeolocationService = webViewClient->geolocationService();
-    ASSERT(m_webGeolocationService);
-    m_bridgeId = m_webGeolocationService->attachBridge(this);
-    if (!m_bridgeId) {
-        // Attach failed. Release association with this service.
-        m_webGeolocationService = 0;
-    }
+    if (!m_bridgeId)
+        m_bridgeId = getWebViewClient()->geolocationService()->attachBridge(this);
 }
 
 void WebGeolocationServiceBridgeImpl::setIsAllowed(bool allowed)
 {
-    m_geolocationServiceChromium->setIsAllowed(allowed);
+    m_GeolocationServiceChromium->setIsAllowed(allowed);
 }
 
 void WebGeolocationServiceBridgeImpl::setLastPosition(double latitude, double longitude, bool providesAltitude, double altitude, double accuracy, bool providesAltitudeAccuracy, double altitudeAccuracy, bool providesHeading, double heading, bool providesSpeed, double speed, long long timestamp)
 {
     RefPtr<Geoposition> geoposition = Geoposition::create(Coordinates::create(latitude, longitude, providesAltitude, altitude, accuracy, providesAltitudeAccuracy, altitudeAccuracy, providesHeading, heading, providesSpeed, speed), timestamp);
-    m_geolocationServiceChromium->setLastPosition(geoposition);
+    m_GeolocationServiceChromium->setLastPosition(geoposition);
 }
 
 void WebGeolocationServiceBridgeImpl::setLastError(int errorCode, const WebString& message)
 {
-    m_geolocationServiceChromium->setLastError(errorCode, message);
+    m_GeolocationServiceChromium->setLastError(errorCode, message);
 }
 
-void WebGeolocationServiceBridgeImpl::onWebGeolocationServiceDestroyed()
+WebViewClient* WebGeolocationServiceBridgeImpl::getWebViewClient()
 {
-    m_bridgeId = 0;
-    m_webGeolocationService = 0;
+    Frame* frame = m_GeolocationServiceChromium->frame();
+    if (!frame || !frame->page())
+        return 0;
+    WebKit::ChromeClientImpl* chromeClientImpl = static_cast<WebKit::ChromeClientImpl*>(frame->page()->chrome()->client());
+    WebKit::WebViewClient* webViewClient = chromeClientImpl->webView()->client();
+    return webViewClient;
 }
 
-bool WebGeolocationServiceBridgeImpl::isAttached() const
+void WebGeolocationServiceBridgeImpl::onWebGeolocationServiceDestroyed()
 {
-    // Test the class invariant.
-    if (m_webGeolocationService)
-        ASSERT(m_bridgeId);
-    else     
-        ASSERT(!m_bridgeId);
-
-    return m_webGeolocationService;
 }
 
 } // namespace WebKit
index 0562fab..2a10c96 100644 (file)
@@ -34,7 +34,6 @@
 #include "GeolocationService.h"
 #include "GeolocationServiceChromium.h"
 #include "GeolocationServiceMock.h"
-#include "WebGeolocationServiceBridge.h"
 #include "WebString.h"
 #include <wtf/CurrentTime.h>
 #include <wtf/HashMap.h>