⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 248276 in webkit


Ignore:
Timestamp:
Aug 5, 2019, 4:00:26 PM (7 years ago)
Author:
Chris Dumez
Message:

navigator.geolocation wrapper should not become GC-collectable once its frame is detached
​https://bugs.webkit.org/show_bug.cgi?id=200436

Reviewed by Darin Adler.

Source/WebCore:

navigator.geolocation wrapper should not become GC-collectable once its frame is detached, given
that it can outlive the frame. Instead, tie the navigator.geolocation wrapper's lifetime to its
Navigator's.

Test: fast/dom/navigator-property-gc-after-frame-detach.html

  • Modules/geolocation/Geolocation.cpp:

(WebCore::Geolocation::create):
(WebCore::Geolocation::Geolocation):
(WebCore::Geolocation::navigator):
(WebCore::Geolocation::frame const):

  • Modules/geolocation/Geolocation.h:
  • Modules/geolocation/Geolocation.idl:
  • Modules/geolocation/NavigatorGeolocation.cpp:

(WebCore::NavigatorGeolocation::NavigatorGeolocation):
(WebCore::NavigatorGeolocation::from):
(WebCore::NavigatorGeolocation::geolocation):
(WebCore::NavigatorGeolocation::geolocation const):

  • Modules/geolocation/NavigatorGeolocation.h:
  • bindings/js/JSNavigatorCustom.cpp:

(WebCore::JSNavigator::visitAdditionalChildren):

  • bindings/js/JSWorkerNavigatorCustom.cpp:

(WebCore::JSWorkerNavigator::visitAdditionalChildren):

  • bindings/scripts/CodeGeneratorJS.pm:

(GenerateImplementation):

  • bindings/scripts/IDLAttributes.json:
  • page/Navigator.cpp:

(WebCore::Navigator::plugins):
(WebCore::Navigator::mimeTypes):

  • page/NavigatorBase.h:
  • plugins/DOMMimeTypeArray.cpp:

(WebCore::DOMMimeTypeArray::DOMMimeTypeArray):

  • plugins/DOMMimeTypeArray.h:
  • plugins/DOMMimeTypeArray.idl:
  • plugins/DOMPluginArray.cpp:

(WebCore::DOMPluginArray::DOMPluginArray):

  • plugins/DOMPluginArray.h:
  • plugins/DOMPluginArray.idl:
  • workers/service/ServiceWorkerContainer.h:
  • workers/service/ServiceWorkerContainer.idl:

LayoutTests:

Add layout test coverage.

  • fast/dom/navigator-property-gc-after-frame-detach-expected.txt: Added.
  • fast/dom/navigator-property-gc-after-frame-detach.html: Added.
Location:
trunk
Files:
4 added
23 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r248274 r248276  
     12019-08-05  Chris Dumez  <cdumez@apple.com>
     2
     3        navigator.geolocation wrapper should not become GC-collectable once its frame is detached
     4        https://bugs.webkit.org/show_bug.cgi?id=200436
     5
     6        Reviewed by Darin Adler.
     7
     8        Add layout test coverage.
     9
     10        * fast/dom/navigator-property-gc-after-frame-detach-expected.txt: Added.
     11        * fast/dom/navigator-property-gc-after-frame-detach.html: Added.
     12
    1132019-08-05  Devin Rousso  <drousso@apple.com>
    214

  • trunk/LayoutTests/platform/wk2/TestExpectations

    r248273 r248276  
    160160
    161161webkit.org/b/105952 fast/loader/submit-form-while-parsing-2.html [ Pass Failure ]
    162 
    163 webkit.org/b/141122 editing/selection/programmatic-selection-on-mac-is-directionless.html [ Pass Failure ]
    164162
    165163webkit.org/b/149087 http/tests/cache/disk-cache/disk-cache-cancel.html [ Pass Failure ]

  • trunk/Source/WebCore/ChangeLog

    r248275 r248276  
     12019-08-05  Chris Dumez  <cdumez@apple.com>
     2
     3        navigator.geolocation wrapper should not become GC-collectable once its frame is detached
     4        https://bugs.webkit.org/show_bug.cgi?id=200436
     5
     6        Reviewed by Darin Adler.
     7
     8        navigator.geolocation wrapper should not become GC-collectable once its frame is detached, given
     9        that it can outlive the frame. Instead, tie the navigator.geolocation wrapper's lifetime to its
     10        Navigator's.
     11
     12        Test: fast/dom/navigator-property-gc-after-frame-detach.html
     13
     14        * Modules/geolocation/Geolocation.cpp:
     15        (WebCore::Geolocation::create):
     16        (WebCore::Geolocation::Geolocation):
     17        (WebCore::Geolocation::navigator):
     18        (WebCore::Geolocation::frame const):
     19        * Modules/geolocation/Geolocation.h:
     20        * Modules/geolocation/Geolocation.idl:
     21        * Modules/geolocation/NavigatorGeolocation.cpp:
     22        (WebCore::NavigatorGeolocation::NavigatorGeolocation):
     23        (WebCore::NavigatorGeolocation::from):
     24        (WebCore::NavigatorGeolocation::geolocation):
     25        (WebCore::NavigatorGeolocation::geolocation const):
     26        * Modules/geolocation/NavigatorGeolocation.h:
     27        * bindings/js/JSNavigatorCustom.cpp:
     28        (WebCore::JSNavigator::visitAdditionalChildren):
     29        * bindings/js/JSWorkerNavigatorCustom.cpp:
     30        (WebCore::JSWorkerNavigator::visitAdditionalChildren):
     31        * bindings/scripts/CodeGeneratorJS.pm:
     32        (GenerateImplementation):
     33        * bindings/scripts/IDLAttributes.json:
     34        * page/Navigator.cpp:
     35        (WebCore::Navigator::plugins):
     36        (WebCore::Navigator::mimeTypes):
     37        * page/NavigatorBase.h:
     38        * plugins/DOMMimeTypeArray.cpp:
     39        (WebCore::DOMMimeTypeArray::DOMMimeTypeArray):
     40        * plugins/DOMMimeTypeArray.h:
     41        * plugins/DOMMimeTypeArray.idl:
     42        * plugins/DOMPluginArray.cpp:
     43        (WebCore::DOMPluginArray::DOMPluginArray):
     44        * plugins/DOMPluginArray.h:
     45        * plugins/DOMPluginArray.idl:
     46        * workers/service/ServiceWorkerContainer.h:
     47        * workers/service/ServiceWorkerContainer.idl:
     48
    1492019-08-05  Andy Estes  <aestes@apple.com>
    250

  • trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp

    r243887 r248276  
    3939#include "GeolocationPosition.h"
    4040#include "Geoposition.h"
     41#include "Navigator.h"
    4142#include "Page.h"
    4243#include "PositionError.h"
    … …  
    130131}
    131132
    132 Ref<Geolocation> Geolocation::create(ScriptExecutionContext* context)
    133 {
    134     auto geolocation = adoptRef(*new Geolocation(context));
     133Ref<Geolocation> Geolocation::create(Navigator& navigator)
     134{
     135    auto geolocation = adoptRef(*new Geolocation(navigator));
    135136    geolocation.get().suspendIfNeeded();
    136137    return geolocation;
    137138}
    138139
    139 Geolocation::Geolocation(ScriptExecutionContext* context)
    140     : ActiveDOMObject(context)
    141     , m_allowGeolocation(Unknown)
    142     , m_isSuspended(false)
    143     , m_hasChangedPosition(false)
     140Geolocation::Geolocation(Navigator& navigator)
     141    : ActiveDOMObject(navigator.scriptExecutionContext())
     142    , m_navigator(makeWeakPtr(navigator))
    144143    , m_resumeTimer(*this, &Geolocation::resumeTimerFired)
    145144{
    … …  
    732731}
    733732
     733Navigator* Geolocation::navigator()
     734{
     735    return m_navigator.get();
     736}
     737
     738Frame* Geolocation::frame() const
     739{
     740    return m_navigator ? m_navigator->frame() : nullptr;
     741}
     742
    734743} // namespace WebCore
    735744                                                       

  • trunk/Source/WebCore/Modules/geolocation/Geolocation.h

    r243887 r248276  
    4646class GeoNotifier;
    4747class GeolocationError;
     48class Navigator;
    4849class Page;
    4950class ScriptExecutionContext;
    … …  
    5556    friend class GeoNotifier;
    5657public:
    57     static Ref<Geolocation> create(ScriptExecutionContext*);
     58    static Ref<Geolocation> create(Navigator&);
    5859    WEBCORE_EXPORT ~Geolocation();
    5960
    6061    WEBCORE_EXPORT void resetAllGeolocationPermission();
    6162    Document* document() const { return downcast<Document>(scriptExecutionContext()); }
    62     Frame* frame() const { return document() ? document()->frame() : nullptr; }
    6363
    6464    void getCurrentPosition(Ref<PositionCallback>&&, RefPtr<PositionErrorCallback>&&, PositionOptions&&);
    … …  
    7474    bool shouldBlockGeolocationRequests();
    7575
     76    Navigator* navigator();
     77    WEBCORE_EXPORT Frame* frame() const;
     78
    7679private:
    77     explicit Geolocation(ScriptExecutionContext*);
     80    explicit Geolocation(Navigator&);
    7881
    7982    Geoposition* lastPosition();
    … …  
    145148    void makeCachedPositionCallbacks();
    146149
     150    void resumeTimerFired();
     151
     152    WeakPtr<Navigator> m_navigator;
    147153    GeoNotifierSet m_oneShots;
    148154    Watchers m_watchers;
    … …  
    150156    RefPtr<Geoposition> m_lastPosition;
    151157
    152     enum {
    153         Unknown,
    154         InProgress,
    155         Yes,
    156         No
    157     } m_allowGeolocation;
    158     bool m_isSuspended;
    159     bool m_resetOnResume;
    160     bool m_hasChangedPosition;
     158    enum { Unknown, InProgress, Yes, No } m_allowGeolocation { Unknown };
     159    bool m_isSuspended { false };
     160    bool m_resetOnResume { false };
     161    bool m_hasChangedPosition { false };
    161162    RefPtr<PositionError> m_errorWaitingForResume;
    162 
    163     void resumeTimerFired();
    164163    Timer m_resumeTimer;
    165 
    166164    GeoNotifierSet m_requestsAwaitingCachedPosition;
    167165};

  • trunk/Source/WebCore/Modules/geolocation/Geolocation.idl

    r209669 r248276  
    2828    NoInterfaceObject,
    2929    Conditional=GEOLOCATION,
    30     GenerateIsReachable=ImplFrame,
     30    GenerateIsReachable=ReachableFromNavigator,
    3131] interface Geolocation {
    3232    // FIXME: PositionErrorCallback should not be nullable

  • trunk/Source/WebCore/Modules/geolocation/NavigatorGeolocation.cpp

    r237266 r248276  
    3535namespace WebCore {
    3636
    37 NavigatorGeolocation::NavigatorGeolocation(DOMWindow* window)
    38     : DOMWindowProperty(window)
     37NavigatorGeolocation::NavigatorGeolocation(Navigator& navigator)
     38    : m_navigator(navigator)
    3939{
    4040}
    … …  
    4747}
    4848
    49 NavigatorGeolocation* NavigatorGeolocation::from(Navigator* navigator)
     49NavigatorGeolocation* NavigatorGeolocation::from(Navigator& navigator)
    5050{
    51     NavigatorGeolocation* supplement = static_cast<NavigatorGeolocation*>(Supplement<Navigator>::from(navigator, supplementName()));
     51    NavigatorGeolocation* supplement = static_cast<NavigatorGeolocation*>(Supplement<Navigator>::from(&navigator, supplementName()));
    5252    if (!supplement) {
    53         auto newSupplement = std::make_unique<NavigatorGeolocation>(navigator->window());
     53        auto newSupplement = std::make_unique<NavigatorGeolocation>(navigator);
    5454        supplement = newSupplement.get();
    55         provideTo(navigator, supplementName(), WTFMove(newSupplement));
     55        provideTo(&navigator, supplementName(), WTFMove(newSupplement));
    5656    }
    5757    return supplement;
    … …  
    6868Geolocation* NavigatorGeolocation::geolocation(Navigator& navigator)
    6969{
    70     return NavigatorGeolocation::from(&navigator)->geolocation();
     70    return NavigatorGeolocation::from(navigator)->geolocation();
    7171}
    7272
    … …  
    7474{
    7575    if (!m_geolocation)
    76         m_geolocation = Geolocation::create(window() ? window()->document() : nullptr);
     76        m_geolocation = Geolocation::create(m_navigator);
    7777    return m_geolocation.get();
    7878}

  • trunk/Source/WebCore/Modules/geolocation/NavigatorGeolocation.h

    r237266 r248276  
    3030class Navigator;
    3131
    32 class NavigatorGeolocation : public Supplement<Navigator>, public DOMWindowProperty {
     32class NavigatorGeolocation : public Supplement<Navigator> {
    3333    WTF_MAKE_FAST_ALLOCATED;
    3434public:
    35     explicit NavigatorGeolocation(DOMWindow*);
     35    explicit NavigatorGeolocation(Navigator&);
    3636    virtual ~NavigatorGeolocation();
    37     static NavigatorGeolocation* from(Navigator*);
     37    static NavigatorGeolocation* from(Navigator&);
    3838
    3939    static Geolocation* geolocation(Navigator&);
    … …  
    4848
    4949    mutable RefPtr<Geolocation> m_geolocation;
     50    Navigator& m_navigator;
    5051};
    5152

  • trunk/Source/WebCore/bindings/js/JSNavigatorCustom.cpp

    r244043 r248276  
    3131void JSNavigator::visitAdditionalChildren(JSC::SlotVisitor& visitor)
    3232{
    33 #if ENABLE(SERVICE_WORKER)
    34     visitor.addOpaqueRoot(wrapped().serviceWorkerIfExists());
    35 #else
    36     UNUSED_PARAM(visitor);
    37 #endif
     33    visitor.addOpaqueRoot(static_cast<NavigatorBase*>(&wrapped()));
    3834}
    3935

  • trunk/Source/WebCore/bindings/js/JSWorkerNavigatorCustom.cpp

    r244043 r248276  
    3131void JSWorkerNavigator::visitAdditionalChildren(JSC::SlotVisitor& visitor)
    3232{
    33 #if ENABLE(SERVICE_WORKER)
    34     visitor.addOpaqueRoot(wrapped().serviceWorkerIfExists());
    35 #else
    36     UNUSED_PARAM(visitor);
    37 #endif
     33    visitor.addOpaqueRoot(static_cast<NavigatorBase*>(&wrapped()));
    3834}
    3935

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r248275 r248276  
    46914691                $rootString .= "    if (UNLIKELY(reason))\n";
    46924692                $rootString .= "        *reason = \"Reachable from ${interfaceName}\";\n";
    4693             } elsif (GetGenerateIsReachable($interface) eq "ImplFrame") {
    4694                 $rootString  = "    Frame* root = WTF::getPtr(js${interfaceName}->wrapped().frame());\n";
    4695                 $rootString .= "    if (!root)\n";
    4696                 $rootString .= "        return false;\n";
    4697                 $rootString .= "    if (UNLIKELY(reason))\n";
    4698                 $rootString .= "        *reason = \"Reachable from Frame\";\n";
    46994693            } elsif (GetGenerateIsReachable($interface) eq "ReachableFromDOMWindow") {
    47004694                $rootString  = "    auto* root = WTF::getPtr(js${interfaceName}->wrapped().window());\n";
    … …  
    47034697                $rootString .= "    if (UNLIKELY(reason))\n";
    47044698                $rootString .= "        *reason = \"Reachable from Window\";\n";
     4699            } elsif (GetGenerateIsReachable($interface) eq "ReachableFromNavigator") {
     4700                $implIncludes{"Navigator.h"} = 1;
     4701                $implIncludes{"WorkerNavigator.h"} = 1;
     4702                $rootString  = "    NavigatorBase* root = WTF::getPtr(js${interfaceName}->wrapped().navigator());\n";
     4703                $rootString .= "    if (!root)\n";
     4704                $rootString .= "        return false;\n";
     4705                $rootString .= "    if (UNLIKELY(reason))\n";
     4706                $rootString .= "        *reason = \"Reachable from Navigator\";\n";
    47054707            } elsif (GetGenerateIsReachable($interface) eq "ImplDocument") {
    47064708                $rootString  = "    Document* root = WTF::getPtr(js${interfaceName}->wrapped().document());\n";

  • trunk/Source/WebCore/bindings/scripts/IDLAttributes.json

    r248155 r248276  
    227227        "GenerateIsReachable": {
    228228            "contextsAllowed": ["interface"],
    229             "values": ["", "Impl", "ImplWebGLRenderingContext", "ImplDocument", "ImplElementRoot", "ImplFrame", "ImplOwnerNodeRoot", "ImplScriptExecutionContext", "ReachableFromDOMWindow"]
     229            "values": ["", "Impl", "ImplWebGLRenderingContext", "ImplDocument", "ImplElementRoot", "ImplOwnerNodeRoot", "ImplScriptExecutionContext", "ReachableFromDOMWindow", "ReachableFromNavigator"]
    230230        },
    231231        "Global": {

  • trunk/Source/WebCore/page/DOMWindow.cpp

    r248193 r248276  
    20512051#if ENABLE(GEOLOCATION) && PLATFORM(IOS_FAMILY)
    20522052    if (m_navigator)
    2053         NavigatorGeolocation::from(m_navigator.get())->resetAllGeolocationPermission();
     2053        NavigatorGeolocation::from(*m_navigator)->resetAllGeolocationPermission();
    20542054#endif
    20552055}

  • trunk/Source/WebCore/page/Navigator.cpp

    r244927 r248276  
    154154    }
    155155    if (!m_plugins)
    156         m_plugins = DOMPluginArray::create(window());
     156        m_plugins = DOMPluginArray::create(*this);
    157157    return *m_plugins;
    158158}
    … …  
    165165    }
    166166    if (!m_mimeTypes)
    167         m_mimeTypes = DOMMimeTypeArray::create(window());
     167        m_mimeTypes = DOMMimeTypeArray::create(*this);
    168168    return *m_mimeTypes;
    169169}

  • trunk/Source/WebCore/page/NavigatorBase.h

    r244927 r248276  
    3232#include <wtf/UniqueRef.h>
    3333#include <wtf/Vector.h>
     34#include <wtf/WeakPtr.h>
    3435
    3536namespace WebCore {
    … …  
    3839class ServiceWorkerContainer;
    3940
    40 class NavigatorBase : public RefCounted<NavigatorBase>, public ContextDestructionObserver {
     41class NavigatorBase : public RefCounted<NavigatorBase>, public ContextDestructionObserver, public CanMakeWeakPtr<NavigatorBase> {
    4142public:
    4243    virtual ~NavigatorBase();

  • trunk/Source/WebCore/plugins/DOMMimeTypeArray.cpp

    r246490 r248276  
    3232WTF_MAKE_ISO_ALLOCATED_IMPL(DOMMimeTypeArray);
    3333
    34 DOMMimeTypeArray::DOMMimeTypeArray(DOMWindow* window)
    35     : DOMWindowProperty(window)
     34DOMMimeTypeArray::DOMMimeTypeArray(Navigator& navigator)
     35    : m_navigator(makeWeakPtr(navigator))
    3636{
    3737}

  • trunk/Source/WebCore/plugins/DOMMimeTypeArray.h

    r246490 r248276  
    2222
    2323#include "DOMMimeType.h"
    24 #include "DOMWindowProperty.h"
     24#include "Navigator.h"
    2525#include "ScriptWrappable.h"
    2626#include <wtf/RefCounted.h>
    … …  
    3030class PluginData;
    3131
    32 class DOMMimeTypeArray final : public ScriptWrappable, public RefCounted<DOMMimeTypeArray>, public DOMWindowProperty {
     32class DOMMimeTypeArray final : public ScriptWrappable, public RefCounted<DOMMimeTypeArray> {
    3333    WTF_MAKE_ISO_ALLOCATED(DOMMimeTypeArray);
    3434public:
    35     static Ref<DOMMimeTypeArray> create(DOMWindow* window) { return adoptRef(*new DOMMimeTypeArray(window)); }
     35    static Ref<DOMMimeTypeArray> create(Navigator& navigator) { return adoptRef(*new DOMMimeTypeArray(navigator)); }
    3636    ~DOMMimeTypeArray();
    3737
    … …  
    4040    RefPtr<DOMMimeType> namedItem(const AtomString& propertyName);
    4141    Vector<AtomString> supportedPropertyNames();
     42   
     43    Navigator* navigator() { return m_navigator.get(); }
    4244
    4345private:
    44     explicit DOMMimeTypeArray(DOMWindow*);
     46    explicit DOMMimeTypeArray(Navigator&);
    4547    PluginData* getPluginData() const;
     48    Frame* frame() const { return m_navigator ? m_navigator->frame() : nullptr; }
     49   
     50    WeakPtr<Navigator> m_navigator;
    4651};
    4752

  • trunk/Source/WebCore/plugins/DOMMimeTypeArray.idl

    r248155 r248276  
    2020
    2121[
    22     GenerateIsReachable=ReachableFromDOMWindow,
     22    GenerateIsReachable=ReachableFromNavigator,
    2323    LegacyUnenumerableNamedProperties,
    2424    ImplementationLacksVTable,

  • trunk/Source/WebCore/plugins/DOMPluginArray.cpp

    r246490 r248276  
    3232WTF_MAKE_ISO_ALLOCATED_IMPL(DOMPluginArray);
    3333
    34 DOMPluginArray::DOMPluginArray(DOMWindow* window)
    35     : DOMWindowProperty(window)
     34DOMPluginArray::DOMPluginArray(Navigator& navigator)
     35    : m_navigator(makeWeakPtr(navigator))
    3636{
    3737}

  • trunk/Source/WebCore/plugins/DOMPluginArray.h

    r246490 r248276  
    2323#include "DOMPlugin.h"
    2424#include "DOMWindowProperty.h"
     25#include "Navigator.h"
    2526#include "ScriptWrappable.h"
    2627#include <wtf/RefCounted.h>
    … …  
    3031class PluginData;
    3132
    32 class DOMPluginArray final : public ScriptWrappable, public RefCounted<DOMPluginArray>, public DOMWindowProperty {
     33class DOMPluginArray final : public ScriptWrappable, public RefCounted<DOMPluginArray> {
    3334    WTF_MAKE_ISO_ALLOCATED(DOMPluginArray);
    3435public:
    35     static Ref<DOMPluginArray> create(DOMWindow* window) { return adoptRef(*new DOMPluginArray(window)); }
     36    static Ref<DOMPluginArray> create(Navigator& navigator) { return adoptRef(*new DOMPluginArray(navigator)); }
    3637    ~DOMPluginArray();
    3738
    … …  
    4243
    4344    void refresh(bool reloadPages);
     45   
     46    Navigator* navigator() { return m_navigator.get(); }
    4447
    4548private:
    46     explicit DOMPluginArray(DOMWindow*);
     49    explicit DOMPluginArray(Navigator&);
     50
    4751    PluginData* pluginData() const;
     52    Frame* frame() const { return m_navigator ? m_navigator->frame() : nullptr; }
     53   
     54    WeakPtr<Navigator> m_navigator;
    4855};
    4956

  • trunk/Source/WebCore/plugins/DOMPluginArray.idl

    r248155 r248276  
    2020
    2121[
    22     GenerateIsReachable=ReachableFromDOMWindow,
     22    GenerateIsReachable=ReachableFromNavigator,
    2323    LegacyUnenumerableNamedProperties,
    2424    ImplementationLacksVTable,

  • trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h

    r245873 r248276  
    8888    bool isAlwaysOnLoggingAllowed() const;
    8989
     90    NavigatorBase* navigator() { return &m_navigator; }
     91
    9092private:
    9193    void scheduleJob(std::unique_ptr<ServiceWorkerJob>&&);

  • trunk/Source/WebCore/workers/service/ServiceWorkerContainer.idl

    r225221 r248276  
    3232    Conditional=SERVICE_WORKER,
    3333    EnabledAtRuntime=ServiceWorker,
    34     GenerateIsReachable=Impl,
     34    GenerateIsReachable=ReachableFromNavigator
    3535] interface ServiceWorkerContainer : EventTarget {
    3636    readonly attribute ServiceWorker? controller;
Note: See TracChangeset for help on using the changeset viewer.