Title: [109761] trunk/Source
Revision
109761
Author
aba...@webkit.org
Date
2012-03-05 10:17:58 -0800 (Mon, 05 Mar 2012)

Log Message

Geolocation should use a ScriptExecutionContext as its context object
https://bugs.webkit.org/show_bug.cgi?id=80248

Reviewed by Kentaro Hara.

Source/WebCore: 

This patch updates Geolocation to use some more modern WebCore
mechanisms.  Previously, Geolocation used a Frame as a context object,
which required a bunch of manual integration with the PageCache as well
as custom signaling for Geolocation::reset().  After this patch,
Geolocation subclasses ActiveDOMObject, which does all this work
automatically.

* Modules/geolocation/Geolocation.cpp:
(WebCore::Geolocation::create):
(WebCore):
(WebCore::Geolocation::Geolocation):
(WebCore::Geolocation::~Geolocation):
(WebCore::Geolocation::document):
(WebCore::Geolocation::frame):
(WebCore::Geolocation::page):
(WebCore::Geolocation::stop):
(WebCore::Geolocation::getCurrentPosition):
(WebCore::Geolocation::watchPosition):
(WebCore::Geolocation::requestPermission):
(WebCore::Geolocation::clearWatch):
(WebCore::Geolocation::setIsAllowed):
* Modules/geolocation/Geolocation.h:
(WebCore):
(Geolocation):
* Modules/geolocation/NavigatorGeolocation.cpp:
(WebCore):
(WebCore::NavigatorGeolocation::geolocation):
* Modules/geolocation/NavigatorGeolocation.h:
(NavigatorGeolocation):
* dom/Document.cpp:
(WebCore::Document::Document):
* dom/Document.h:
(Document):
* history/PageCache.cpp:
(WebCore::logCanCacheFrameDecision):
(WebCore::PageCache::canCachePageContainingThisFrame):

Source/WebKit/chromium: 

Rather than indirecting through the Frame to get the SecurityOrigin, we
should get the SecurityOrigin directly from ScriptExecutionContext.

* src/WebGeolocationPermissionRequest.cpp:
(WebKit::WebGeolocationPermissionRequest::securityOrigin):

Source/WebKit/mac: 

* WebView/WebFrame.mm:
(-[WebFrame _cacheabilityDictionary]):
    - We no longer special-case Geolocation.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (109760 => 109761)


--- trunk/Source/WebCore/ChangeLog	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/ChangeLog	2012-03-05 18:17:58 UTC (rev 109761)
@@ -1,3 +1,47 @@
+2012-03-05  Adam Barth  <aba...@webkit.org>
+
+        Geolocation should use a ScriptExecutionContext as its context object
+        https://bugs.webkit.org/show_bug.cgi?id=80248
+
+        Reviewed by Kentaro Hara.
+
+        This patch updates Geolocation to use some more modern WebCore
+        mechanisms.  Previously, Geolocation used a Frame as a context object,
+        which required a bunch of manual integration with the PageCache as well
+        as custom signaling for Geolocation::reset().  After this patch,
+        Geolocation subclasses ActiveDOMObject, which does all this work
+        automatically.
+
+        * Modules/geolocation/Geolocation.cpp:
+        (WebCore::Geolocation::create):
+        (WebCore):
+        (WebCore::Geolocation::Geolocation):
+        (WebCore::Geolocation::~Geolocation):
+        (WebCore::Geolocation::document):
+        (WebCore::Geolocation::frame):
+        (WebCore::Geolocation::page):
+        (WebCore::Geolocation::stop):
+        (WebCore::Geolocation::getCurrentPosition):
+        (WebCore::Geolocation::watchPosition):
+        (WebCore::Geolocation::requestPermission):
+        (WebCore::Geolocation::clearWatch):
+        (WebCore::Geolocation::setIsAllowed):
+        * Modules/geolocation/Geolocation.h:
+        (WebCore):
+        (Geolocation):
+        * Modules/geolocation/NavigatorGeolocation.cpp:
+        (WebCore):
+        (WebCore::NavigatorGeolocation::geolocation):
+        * Modules/geolocation/NavigatorGeolocation.h:
+        (NavigatorGeolocation):
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        * dom/Document.h:
+        (Document):
+        * history/PageCache.cpp:
+        (WebCore::logCanCacheFrameDecision):
+        (WebCore::PageCache::canCachePageContainingThisFrame):
+
 2012-03-05  Martin Robinson  <mrobin...@igalia.com>
 
         [soup] Crash while loading http://www.jusco.cn

Modified: trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp (109760 => 109761)


--- trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp	2012-03-05 18:17:58 UTC (rev 109761)
@@ -225,38 +225,57 @@
     copyValuesToVector(m_idToNotifierMap, copy);
 }
 
-Geolocation::Geolocation(Frame* frame)
-    : DOMWindowProperty(frame)
+PassRefPtr<Geolocation> Geolocation::create(ScriptExecutionContext* context)
+{
+    RefPtr<Geolocation> geolocation = adoptRef(new Geolocation(context));
+    geolocation->suspendIfNeeded();
+    return geolocation.release();
+}
+
+Geolocation::Geolocation(ScriptExecutionContext* context)
+    : ActiveDOMObject(context, this)
 #if !ENABLE(CLIENT_BASED_GEOLOCATION)
     , m_service(GeolocationService::create(this))
 #endif
     , m_allowGeolocation(Unknown)
 {
-    if (!m_frame)
-        return;
-    ASSERT(m_frame->document());
-    m_frame->document()->setUsingGeolocation(true);
 }
 
 Geolocation::~Geolocation()
 {
     ASSERT(m_allowGeolocation != InProgress);
-    ASSERT(!m_frame);
 }
 
+Document* Geolocation::document() const
+{
+    ASSERT(!scriptExecutionContext() || scriptExecutionContext()->isDocument());
+    return static_cast<Document*>(scriptExecutionContext());
+}
+
+Frame* Geolocation::frame() const
+{
+    return document() ? document()->frame() : 0;
+}
+
 Page* Geolocation::page() const
 {
-    return m_frame ? m_frame->page() : 0;
+    return document() ? document()->page() : 0;
 }
 
-void Geolocation::reset()
+void Geolocation::stop()
 {
+    // FIXME: We should ideally allow existing Geolocation activities to continue
+    // when the Geolocation's iframe is reparented. (Assuming we continue to
+    // support reparenting iframes.)
+    // See https://bugs.webkit.org/show_bug.cgi?id=55577
+    // and https://bugs.webkit.org/show_bug.cgi?id=52877
+
     Page* page = this->page();
     if (page && m_allowGeolocation == InProgress) {
 #if ENABLE(CLIENT_BASED_GEOLOCATION)
         page->geolocationController()->cancelPermissionRequest(this);
 #else
-        page->chrome()->client()->cancelGeolocationPermissionRequestForFrame(m_frame, this);
+        page->chrome()->client()->cancelGeolocationPermissionRequestForFrame(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.
@@ -268,15 +287,6 @@
 #endif
 }
 
-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);
-    DOMWindowProperty::disconnectFrame();
-}
-
 Geoposition* Geolocation::lastPosition()
 {
 #if ENABLE(CLIENT_BASED_GEOLOCATION)
@@ -294,7 +304,7 @@
 
 void Geolocation::getCurrentPosition(PassRefPtr<PositionCallback> successCallback, PassRefPtr<PositionErrorCallback> errorCallback, PassRefPtr<PositionOptions> options)
 {
-    if (!m_frame)
+    if (!frame())
         return;
 
     RefPtr<GeoNotifier> notifier = startRequest(successCallback, errorCallback, options);
@@ -305,7 +315,7 @@
 
 int Geolocation::watchPosition(PassRefPtr<PositionCallback> successCallback, PassRefPtr<PositionErrorCallback> errorCallback, PassRefPtr<PositionOptions> options)
 {
-    if (!m_frame)
+    if (!frame())
         return 0;
 
     RefPtr<GeoNotifier> notifier = startRequest(successCallback, errorCallback, options);
@@ -620,7 +630,7 @@
 #if ENABLE(CLIENT_BASED_GEOLOCATION)
     page->geolocationController()->requestPermission(this);
 #else
-    page->chrome()->client()->requestGeolocationPermissionForFrame(m_frame, this);
+    page->chrome()->client()->requestGeolocationPermissionForFrame(frame(), this);
 #endif
 }
 
@@ -757,18 +767,12 @@
 
 namespace WebCore {
 
-void Geolocation::clearWatch(int) {}
+void Geolocation::clearWatch(int) { }
+void Geolocation::stop() { }
+Geolocation::Geolocation(ScriptExecutionContext* context) : ActiveDOMObject(context, this) { }
+Geolocation::~Geolocation() { }
+void Geolocation::setIsAllowed(bool) { }
 
-void Geolocation::reset() {}
-
-void Geolocation::disconnectFrame() {}
-
-Geolocation::Geolocation(Frame*) : DOMWindowProperty(0) {}
-
-Geolocation::~Geolocation() {}
-
-void Geolocation::setIsAllowed(bool) {}
-
 }
                                                         
 #endif // ENABLE(GEOLOCATION)

Modified: trunk/Source/WebCore/Modules/geolocation/Geolocation.h (109760 => 109761)


--- trunk/Source/WebCore/Modules/geolocation/Geolocation.h	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/Modules/geolocation/Geolocation.h	2012-03-05 18:17:58 UTC (rev 109761)
@@ -27,7 +27,7 @@
 #ifndef Geolocation_h
 #define Geolocation_h
 
-#include "DOMWindowProperty.h"
+#include "ActiveDOMObject.h"
 #include "Geoposition.h"
 #include "PositionCallback.h"
 #include "PositionError.h"
@@ -41,24 +41,27 @@
 
 namespace WebCore {
 
+class Document;
 class Frame;
 #if ENABLE(CLIENT_BASED_GEOLOCATION)
 class GeolocationPosition;
 class GeolocationError;
 #endif
 class Page;
+class ScriptExecutionContext;
 
-class Geolocation : public RefCounted<Geolocation>, public DOMWindowProperty
+class Geolocation : public RefCounted<Geolocation>, public ActiveDOMObject
 #if !ENABLE(CLIENT_BASED_GEOLOCATION) && ENABLE(GEOLOCATION)
     , public GeolocationServiceClient
 #endif
 {
 public:
-    static PassRefPtr<Geolocation> create(Frame* frame) { return adoptRef(new Geolocation(frame)); }
+    static PassRefPtr<Geolocation> create(ScriptExecutionContext*);
     ~Geolocation();
 
-    virtual void disconnectFrame() OVERRIDE;
-    void reset();
+    virtual void stop() OVERRIDE;
+    Document* document() const;
+    Frame* frame() const;
 
     void getCurrentPosition(PassRefPtr<PositionCallback>, PassRefPtr<PositionErrorCallback>, PassRefPtr<PositionOptions>);
     int watchPosition(PassRefPtr<PositionCallback>, PassRefPtr<PositionErrorCallback>, PassRefPtr<PositionOptions>);
@@ -79,7 +82,7 @@
     bool isAllowed() const { return m_allowGeolocation == Yes; }
     bool isDenied() const { return m_allowGeolocation == No; }
 
-    explicit Geolocation(Frame*);
+    explicit Geolocation(ScriptExecutionContext*);
 
     Page* page() const;
 

Modified: trunk/Source/WebCore/Modules/geolocation/NavigatorGeolocation.cpp (109760 => 109761)


--- trunk/Source/WebCore/Modules/geolocation/NavigatorGeolocation.cpp	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/Modules/geolocation/NavigatorGeolocation.cpp	2012-03-05 18:17:58 UTC (rev 109761)
@@ -23,6 +23,8 @@
 #include "config.h"
 #include "NavigatorGeolocation.h"
 
+#include "Document.h"
+#include "Frame.h"
 #include "Geolocation.h"
 #include "Navigator.h"
 
@@ -37,17 +39,6 @@
 {
 }
 
-void NavigatorGeolocation::willDetachPage()
-{
-    // FIXME: We should ideally allow existing Geolocation activities to continue
-    // when the Geolocation's iframe is reparented. (Assuming we continue to
-    // support reparenting iframes.)
-    // See https://bugs.webkit.org/show_bug.cgi?id=55577
-    // and https://bugs.webkit.org/show_bug.cgi?id=52877
-    if (m_geolocation)
-        m_geolocation->reset();
-}
-
 NavigatorGeolocation* NavigatorGeolocation::from(Navigator* navigator)
 {
     DEFINE_STATIC_LOCAL(AtomicString, name, ("NavigatorGeolocation"));
@@ -66,8 +57,8 @@
 
 Geolocation* NavigatorGeolocation::geolocation() const
 {
-    if (!m_geolocation)
-        m_geolocation = Geolocation::create(frame());
+    if (!m_geolocation && frame())
+        m_geolocation = Geolocation::create(frame()->document());
     return m_geolocation.get();
 }
 

Modified: trunk/Source/WebCore/Modules/geolocation/NavigatorGeolocation.h (109760 => 109761)


--- trunk/Source/WebCore/Modules/geolocation/NavigatorGeolocation.h	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/Modules/geolocation/NavigatorGeolocation.h	2012-03-05 18:17:58 UTC (rev 109761)
@@ -40,8 +40,6 @@
 private:
     NavigatorGeolocation(Frame*);
 
-    virtual void willDetachPage() OVERRIDE;
-
     mutable RefPtr<Geolocation> m_geolocation;
 };
 

Modified: trunk/Source/WebCore/WebCore.exp.in (109760 => 109761)


--- trunk/Source/WebCore/WebCore.exp.in	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/WebCore.exp.in	2012-03-05 18:17:58 UTC (rev 109761)
@@ -1173,6 +1173,7 @@
 __ZNK7WebCore11FrameLoader20activeDocumentLoaderEv
 __ZNK7WebCore11FrameLoader27numPendingOrLoadingRequestsEb
 __ZNK7WebCore11FrameLoader8loadTypeEv
+__ZNK7WebCore11Geolocation5frameEv
 __ZNK7WebCore11HistoryItem10visitCountEv
 __ZNK7WebCore11HistoryItem11hasChildrenEv
 __ZNK7WebCore11HistoryItem11originalURLEv

Modified: trunk/Source/WebCore/dom/Document.cpp (109760 => 109761)


--- trunk/Source/WebCore/dom/Document.cpp	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/dom/Document.cpp	2012-03-05 18:17:58 UTC (rev 109761)
@@ -419,7 +419,6 @@
     , m_isHTML(isHTML)
     , m_isViewSource(false)
     , m_sawElementsInKnownNamespaces(false)
-    , m_usingGeolocation(false)
     , m_eventQueue(DocumentEventQueue::create(this))
     , m_weakReference(DocumentWeakReference::create(this))
     , m_idAttributeName(idAttr)

Modified: trunk/Source/WebCore/dom/Document.h (109760 => 109761)


--- trunk/Source/WebCore/dom/Document.h	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/dom/Document.h	2012-03-05 18:17:58 UTC (rev 109761)
@@ -1031,9 +1031,6 @@
     virtual bool isContextThread() const;
     virtual bool isJSExecutionForbidden() const { return false; }
 
-    void setUsingGeolocation(bool f) { m_usingGeolocation = f; }
-    bool usingGeolocation() const { return m_usingGeolocation; };
-
     bool containsValidityStyleRules() const { return m_containsValidityStyleRules; }
     void setContainsValidityStyleRules() { m_containsValidityStyleRules = true; }
 
@@ -1414,8 +1411,6 @@
     bool m_isViewSource;
     bool m_sawElementsInKnownNamespaces;
 
-    bool m_usingGeolocation;
-
     RefPtr<DocumentEventQueue> m_eventQueue;
 
     RefPtr<DocumentWeakReference> m_weakReference;

Modified: trunk/Source/WebCore/history/PageCache.cpp (109760 => 109761)


--- trunk/Source/WebCore/history/PageCache.cpp	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebCore/history/PageCache.cpp	2012-03-05 18:17:58 UTC (rev 109761)
@@ -116,10 +116,6 @@
             cannotCache = true;
         }
 #endif
-        if (frame->document()->usingGeolocation()) {
-            PCLOG("   -Frame uses Geolocation");
-            cannotCache = true;
-        }
         if (!frame->loader()->history()->currentItem()) {
             PCLOG("   -No current history item");
             cannotCache = true;
@@ -253,7 +249,6 @@
 #if ENABLE(SHARED_WORKERS)
         && !SharedWorkerRepository::hasSharedWorkers(document)
 #endif
-        && !document->usingGeolocation()
         && frameLoader->history()->currentItem()
         && !frameLoader->quickRedirectComing()
         && !documentLoader->isLoadingInAPISense()

Modified: trunk/Source/WebKit/chromium/ChangeLog (109760 => 109761)


--- trunk/Source/WebKit/chromium/ChangeLog	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebKit/chromium/ChangeLog	2012-03-05 18:17:58 UTC (rev 109761)
@@ -1,3 +1,16 @@
+2012-03-05  Adam Barth  <aba...@webkit.org>
+
+        Geolocation should use a ScriptExecutionContext as its context object
+        https://bugs.webkit.org/show_bug.cgi?id=80248
+
+        Reviewed by Kentaro Hara.
+
+        Rather than indirecting through the Frame to get the SecurityOrigin, we
+        should get the SecurityOrigin directly from ScriptExecutionContext.
+
+        * src/WebGeolocationPermissionRequest.cpp:
+        (WebKit::WebGeolocationPermissionRequest::securityOrigin):
+
 2012-03-02  Andrey Kosyakov  <ca...@chromium.org>
 
         Add instrumentation for frame start/end on timeline.

Modified: trunk/Source/WebKit/chromium/src/WebGeolocationPermissionRequest.cpp (109760 => 109761)


--- trunk/Source/WebKit/chromium/src/WebGeolocationPermissionRequest.cpp	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebKit/chromium/src/WebGeolocationPermissionRequest.cpp	2012-03-05 18:17:58 UTC (rev 109761)
@@ -39,7 +39,7 @@
 
 WebSecurityOrigin WebGeolocationPermissionRequest::securityOrigin() const
 {
-    return WebSecurityOrigin(m_private->frame()->document()->securityOrigin());
+    return WebSecurityOrigin(m_private->scriptExecutionContext()->securityOrigin());
 }
 
 void WebGeolocationPermissionRequest::setIsAllowed(bool allowed)

Modified: trunk/Source/WebKit/mac/ChangeLog (109760 => 109761)


--- trunk/Source/WebKit/mac/ChangeLog	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebKit/mac/ChangeLog	2012-03-05 18:17:58 UTC (rev 109761)
@@ -1,3 +1,14 @@
+2012-03-05  Adam Barth  <aba...@webkit.org>
+
+        Geolocation should use a ScriptExecutionContext as its context object
+        https://bugs.webkit.org/show_bug.cgi?id=80248
+
+        Reviewed by Kentaro Hara.
+
+        * WebView/WebFrame.mm:
+        (-[WebFrame _cacheabilityDictionary]):
+            - We no longer special-case Geolocation.
+
 2012-03-03  Benjamin Poulain  <benja...@webkit.org>
 
         Remove the redundant method KURL::protocolInHTTPFamily()

Modified: trunk/Source/WebKit/mac/WebView/WebFrame.mm (109760 => 109761)


--- trunk/Source/WebKit/mac/WebView/WebFrame.mm	2012-03-05 18:15:32 UTC (rev 109760)
+++ trunk/Source/WebKit/mac/WebView/WebFrame.mm	2012-03-05 18:17:58 UTC (rev 109761)
@@ -1124,10 +1124,6 @@
         if (DatabaseContext::hasOpenDatabases(document))
             [result setObject:[NSNumber numberWithBool:YES] forKey:WebFrameUsesDatabases];
 #endif
-            
-        if (document->usingGeolocation())
-            [result setObject:[NSNumber numberWithBool:YES] forKey:WebFrameUsesGeolocation];
-            
         if (!document->canSuspendActiveDOMObjects())
             [result setObject:[NSNumber numberWithBool:YES] forKey:WebFrameCanSuspendActiveDOMObjects];
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to