Changeset 269435 in webkit


Ignore:
Timestamp:
Nov 5, 2020 8:54:48 AM (3 years ago)
Author:
achristensen@apple.com
Message:

Store WeakPtr<Frame> instead of Frame*
​https://bugs.webkit.org/show_bug.cgi?id=218599

Reviewed by Youenn Fablet.

Source/WebCore:

No change in behavior, but this probably fixes some bugs.

  • bindings/js/WindowProxy.cpp:

(WebCore::WindowProxy::WindowProxy):
(WebCore::WindowProxy::frame const):

  • bindings/js/WindowProxy.h:

(WebCore::WindowProxy::frame const): Deleted.

  • dom/Document.cpp:

(WebCore::Document::willBeRemovedFromFrame):
(WebCore::Document::canNavigateInternal):
(WebCore::Document::setDesignMode):
(WebCore::Document::getDesignMode const): Deleted.

  • dom/Document.h:
  • html/HTMLFrameOwnerElement.cpp:

(WebCore::HTMLFrameOwnerElement::setContentFrame):
(WebCore::HTMLFrameOwnerElement::disconnectContentFrame):

  • html/HTMLFrameOwnerElement.h:

(WebCore::HTMLFrameOwnerElement::contentFrame const):

  • inspector/agents/InspectorPageAgent.cpp:

(WebCore::InspectorPageAgent::frameForId):
(WebCore::InspectorPageAgent::frameId):

  • inspector/agents/InspectorPageAgent.h:
  • loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::stopLoading):
(WebCore::DocumentLoader::willSendRequest):
(WebCore::DocumentLoader::commitLoad):
(WebCore::DocumentLoader::detachFromFrame):
(WebCore::DocumentLoader::removeSubresourceLoader):
(WebCore::DocumentLoader::subresourceLoaderFinishedLoadingOnePart):

  • loader/DocumentWriter.cpp:

(WebCore::DocumentWriter::createDocument):
(WebCore::DocumentWriter::decoder):
(WebCore::DocumentWriter::setFrame):

  • loader/DocumentWriter.h:

(WebCore::DocumentWriter::setFrame): Deleted.

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::detachFromAllOpenedFrames):
(WebCore::FrameLoader::opener):
(WebCore::FrameLoader::setOpener):
(WebCore::FrameLoader::hasOpenedFrames const):
(WebCore::FrameLoader::addExtraFieldsToRequest):

  • loader/FrameLoader.h:
  • loader/FrameNetworkingContext.h:

(WebCore::FrameNetworkingContext::FrameNetworkingContext):
(WebCore::FrameNetworkingContext::frame const):

  • loader/appcache/ApplicationCacheGroup.cpp:

(WebCore::ApplicationCacheGroup::update):
(WebCore::ApplicationCacheGroup::didFinishLoadingEntry):
(WebCore::ApplicationCacheGroup::didFailLoadingEntry):
(WebCore::ApplicationCacheGroup::didFailLoadingManifest):
(WebCore::ApplicationCacheGroup::startLoadingEntry):

  • loader/appcache/ApplicationCacheGroup.h:
  • page/AbstractFrame.h:
  • page/Frame.cpp:

(WebCore::Frame::Frame):
(WebCore::Frame::addDestructionObserver):
(WebCore::Frame::removeDestructionObserver):

  • page/Frame.h:
  • page/FrameDestructionObserver.cpp:

(WebCore::FrameDestructionObserver::frame const):
(WebCore::FrameDestructionObserver::observeFrame):

  • page/FrameDestructionObserver.h:

(WebCore::FrameDestructionObserver::frame const): Deleted.

  • page/FrameTree.cpp:

(WebCore::FrameTree::FrameTree):
(WebCore::FrameTree::parent const):
(WebCore::FrameTree::appendChild):
(WebCore::FrameTree::removeChild):
(WebCore::FrameTree::traverseNextInPostOrder const):

  • page/FrameTree.h:

(WebCore::FrameTree::previousSibling const):
(WebCore::FrameTree::lastChild const):
(WebCore::FrameTree::FrameTree): Deleted.

  • rendering/RenderScrollbar.cpp:

(WebCore::RenderScrollbar::RenderScrollbar):

  • rendering/RenderScrollbar.h:

Source/WebKit:

  • WebProcess/WebPage/WebFrame.cpp:

(WebKit::WebFrame::initWithCoreMainFrame):
(WebKit::WebFrame::createSubframe):
(WebKit::WebFrame::coreFrame const):
(WebKit::WebFrame::info const):
(WebKit::WebFrame::handlesPageScaleGesture const):
(WebKit::WebFrame::requiresUnifiedScaleFactor const):

  • WebProcess/WebPage/WebFrame.h:

(WebKit::WebFrame::coreFrame const): Deleted.

Location:
trunk/Source
Files:
30 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r269434 r269435  
     12020-11-05  Alex Christensen  <achristensen@webkit.org>
     2
     3        Store WeakPtr<Frame> instead of Frame*
     4        https://bugs.webkit.org/show_bug.cgi?id=218599
     5
     6        Reviewed by Youenn Fablet.
     7
     8        No change in behavior, but this probably fixes some bugs.
     9
     10        * bindings/js/WindowProxy.cpp:
     11        (WebCore::WindowProxy::WindowProxy):
     12        (WebCore::WindowProxy::frame const):
     13        * bindings/js/WindowProxy.h:
     14        (WebCore::WindowProxy::frame const): Deleted.
     15        * dom/Document.cpp:
     16        (WebCore::Document::willBeRemovedFromFrame):
     17        (WebCore::Document::canNavigateInternal):
     18        (WebCore::Document::setDesignMode):
     19        (WebCore::Document::getDesignMode const): Deleted.
     20        * dom/Document.h:
     21        * html/HTMLFrameOwnerElement.cpp:
     22        (WebCore::HTMLFrameOwnerElement::setContentFrame):
     23        (WebCore::HTMLFrameOwnerElement::disconnectContentFrame):
     24        * html/HTMLFrameOwnerElement.h:
     25        (WebCore::HTMLFrameOwnerElement::contentFrame const):
     26        * inspector/agents/InspectorPageAgent.cpp:
     27        (WebCore::InspectorPageAgent::frameForId):
     28        (WebCore::InspectorPageAgent::frameId):
     29        * inspector/agents/InspectorPageAgent.h:
     30        * loader/DocumentLoader.cpp:
     31        (WebCore::DocumentLoader::stopLoading):
     32        (WebCore::DocumentLoader::willSendRequest):
     33        (WebCore::DocumentLoader::commitLoad):
     34        (WebCore::DocumentLoader::detachFromFrame):
     35        (WebCore::DocumentLoader::removeSubresourceLoader):
     36        (WebCore::DocumentLoader::subresourceLoaderFinishedLoadingOnePart):
     37        * loader/DocumentWriter.cpp:
     38        (WebCore::DocumentWriter::createDocument):
     39        (WebCore::DocumentWriter::decoder):
     40        (WebCore::DocumentWriter::setFrame):
     41        * loader/DocumentWriter.h:
     42        (WebCore::DocumentWriter::setFrame): Deleted.
     43        * loader/FrameLoader.cpp:
     44        (WebCore::FrameLoader::detachFromAllOpenedFrames):
     45        (WebCore::FrameLoader::opener):
     46        (WebCore::FrameLoader::setOpener):
     47        (WebCore::FrameLoader::hasOpenedFrames const):
     48        (WebCore::FrameLoader::addExtraFieldsToRequest):
     49        * loader/FrameLoader.h:
     50        * loader/FrameNetworkingContext.h:
     51        (WebCore::FrameNetworkingContext::FrameNetworkingContext):
     52        (WebCore::FrameNetworkingContext::frame const):
     53        * loader/appcache/ApplicationCacheGroup.cpp:
     54        (WebCore::ApplicationCacheGroup::update):
     55        (WebCore::ApplicationCacheGroup::didFinishLoadingEntry):
     56        (WebCore::ApplicationCacheGroup::didFailLoadingEntry):
     57        (WebCore::ApplicationCacheGroup::didFailLoadingManifest):
     58        (WebCore::ApplicationCacheGroup::startLoadingEntry):
     59        * loader/appcache/ApplicationCacheGroup.h:
     60        * page/AbstractFrame.h:
     61        * page/Frame.cpp:
     62        (WebCore::Frame::Frame):
     63        (WebCore::Frame::addDestructionObserver):
     64        (WebCore::Frame::removeDestructionObserver):
     65        * page/Frame.h:
     66        * page/FrameDestructionObserver.cpp:
     67        (WebCore::FrameDestructionObserver::frame const):
     68        (WebCore::FrameDestructionObserver::observeFrame):
     69        * page/FrameDestructionObserver.h:
     70        (WebCore::FrameDestructionObserver::frame const): Deleted.
     71        * page/FrameTree.cpp:
     72        (WebCore::FrameTree::FrameTree):
     73        (WebCore::FrameTree::parent const):
     74        (WebCore::FrameTree::appendChild):
     75        (WebCore::FrameTree::removeChild):
     76        (WebCore::FrameTree::traverseNextInPostOrder const):
     77        * page/FrameTree.h:
     78        (WebCore::FrameTree::previousSibling const):
     79        (WebCore::FrameTree::lastChild const):
     80        (WebCore::FrameTree::FrameTree): Deleted.
     81        * rendering/RenderScrollbar.cpp:
     82        (WebCore::RenderScrollbar::RenderScrollbar):
     83        * rendering/RenderScrollbar.h:
     84
    1852020-11-05  Alex Christensen  <achristensen@webkit.org>
    286

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

    r262751 r269435  
    5454
    5555WindowProxy::WindowProxy(AbstractFrame& frame)
    56     : m_frame(&frame)
     56    : m_frame(makeWeakPtr(frame))
    5757    , m_jsWindowProxies(makeUniqueRef<ProxyMap>())
    5858{
    … …  
    6363    ASSERT(!m_frame);
    6464    ASSERT(m_jsWindowProxies->isEmpty());
     65}
     66
     67AbstractFrame* WindowProxy::frame() const
     68{
     69    return m_frame.get();
    6570}
    6671

  • trunk/Source/WebCore/bindings/js/WindowProxy.h

    r251361 r269435  
    2525#include <wtf/RefCounted.h>
    2626#include <wtf/UniqueRef.h>
     27#include <wtf/WeakPtr.h>
    2728
    2829namespace JSC {
    … …  
    5051    WEBCORE_EXPORT ~WindowProxy();
    5152
    52     AbstractFrame* frame() const { return m_frame; }
     53    WEBCORE_EXPORT AbstractFrame* frame() const;
    5354    void detachFromFrame();
    5455
    … …  
    9596    WEBCORE_EXPORT JSWindowProxy& createJSWindowProxyWithInitializedScript(DOMWrapperWorld&);
    9697
    97     AbstractFrame* m_frame;
     98    WeakPtr<AbstractFrame> m_frame;
    9899    UniqueRef<ProxyMap> m_jsWindowProxies;
    99100};

  • trunk/Source/WebCore/dom/Document.cpp

    r269295 r269435  
    25592559
    25602560    {
    2561         NavigationDisabler navigationDisabler(m_frame);
     2561        NavigationDisabler navigationDisabler(m_frame.get());
    25622562        disconnectDescendantFrames();
    25632563    }
    … …  
    34673467
    34683468    // iii. A sandboxed frame can always navigate its descendants.
    3469     if (isSandboxed(SandboxNavigation) && targetFrame.tree().isDescendantOf(m_frame))
     3469    if (isSandboxed(SandboxNavigation) && targetFrame.tree().isDescendantOf(m_frame.get()))
    34703470        return true;
    34713471
    … …  
    34733473    // 1. If A is not the same browsing context as B, and A is not one of the ancestor browsing contexts of B, and B is not a top-level browsing context, and A's active document's active sandboxing
    34743474    // flag set has its sandboxed navigation browsing context flag set, then abort these steps negatively.
    3475     if (m_frame != &targetFrame && isSandboxed(SandboxNavigation) && targetFrame.tree().parent() && !targetFrame.tree().isDescendantOf(m_frame)) {
     3475    if (m_frame != &targetFrame && isSandboxed(SandboxNavigation) && targetFrame.tree().parent() && !targetFrame.tree().isDescendantOf(m_frame.get())) {
    34763476        printNavigationErrorMessage(targetFrame, url(), "The frame attempting navigation is sandboxed, and is therefore disallowed from navigating its ancestors."_s);
    34773477        return false;
    … …  
    57015701{
    57025702    m_designMode = value;
    5703     for (Frame* frame = m_frame; frame && frame->document(); frame = frame->tree().traverseNext(m_frame))
     5703    for (auto* frame = m_frame.get(); frame && frame->document(); frame = frame->tree().traverseNext(m_frame.get()))
    57045704        frame->document()->scheduleFullStyleRebuild();
    57055705}
    … …  
    57205720        mode = inherit;
    57215721    setDesignMode(mode);
    5722 }
    5723 
    5724 auto Document::getDesignMode() const -> InheritedBool
    5725 {
    5726     return m_designMode;
    57275722}
    57285723

  • trunk/Source/WebCore/dom/Document.h

    r269295 r269435  
    10081008    enum InheritedBool { off = false, on = true, inherit };   
    10091009    void setDesignMode(InheritedBool value);
    1010     InheritedBool getDesignMode() const;
    10111010    bool inDesignMode() const;
    10121011    WEBCORE_EXPORT String designMode() const;

  • trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp

    r247924 r269435  
    5151}
    5252
    53 void HTMLFrameOwnerElement::setContentFrame(Frame* frame)
     53void HTMLFrameOwnerElement::setContentFrame(Frame& frame)
    5454{
    5555    // Make sure we will not end up with two frames referencing the same owner element.
    5656    ASSERT(!m_contentFrame || m_contentFrame->ownerElement() != this);
    57     ASSERT(frame);
    5857    // Disconnected frames should not be allowed to load.
    5958    ASSERT(isConnected());
    60     m_contentFrame = frame;
     59    m_contentFrame = makeWeakPtr(frame);
    6160
    6261    for (RefPtr<ContainerNode> node = this; node; node = node->parentOrShadowHostNode())
    … …  
    7776void HTMLFrameOwnerElement::disconnectContentFrame()
    7877{
    79     if (RefPtr<Frame> frame = m_contentFrame) {
     78    if (RefPtr<Frame> frame = m_contentFrame.get()) {
    8079        frame->loader().frameDetached();
    8180        frame->disconnectOwnerElement();

  • trunk/Source/WebCore/html/HTMLFrameOwnerElement.h

    r267449 r269435  
    3636    virtual ~HTMLFrameOwnerElement();
    3737
    38     Frame* contentFrame() const { return m_contentFrame; }
     38    Frame* contentFrame() const { return m_contentFrame.get(); }
    3939    WEBCORE_EXPORT WindowProxy* contentWindow() const;
    4040    WEBCORE_EXPORT Document* contentDocument() const;
    4141
    42     void setContentFrame(Frame*);
     42    void setContentFrame(Frame&);
    4343    void clearContentFrame();
    4444
    … …  
    7474    bool isFrameOwnerElement() const final { return true; }
    7575
    76     Frame* m_contentFrame { nullptr };
     76    WeakPtr<Frame> m_contentFrame;
    7777    SandboxFlags m_sandboxFlags { SandboxNone };
    7878};

  • trunk/Source/WebCore/inspector/agents/InspectorPageAgent.cpp

    r269323 r269435  
    829829Frame* InspectorPageAgent::frameForId(const Protocol::Network::FrameId& frameId)
    830830{
    831     return frameId.isEmpty() ? nullptr : m_identifierToFrame.get(frameId);
     831    return frameId.isEmpty() ? nullptr : m_identifierToFrame.get(frameId).get();
    832832}
    833833
    … …  
    838838    return m_frameToIdentifier.ensure(frame, [this, frame] {
    839839        auto identifier = IdentifiersFactory::createIdentifier();
    840         m_identifierToFrame.set(identifier, frame);
     840        m_identifierToFrame.set(identifier, makeWeakPtr(frame));
    841841        return identifier;
    842842    }).iterator->value;

  • trunk/Source/WebCore/inspector/agents/InspectorPageAgent.h

    r268716 r269435  
    166166    InspectorOverlay* m_overlay { nullptr };
    167167
     168    // FIXME: Make a WeakHashMap and use it for m_frameToIdentifier and m_loaderToIdentifier.
    168169    HashMap<Frame*, String> m_frameToIdentifier;
    169     HashMap<String, Frame*> m_identifierToFrame;
     170    HashMap<String, WeakPtr<Frame>> m_identifierToFrame;
    170171    HashMap<DocumentLoader*, String> m_loaderToIdentifier;
    171172    String m_userAgentOverride;

  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r266148 r269435  
    284284void DocumentLoader::stopLoading()
    285285{
    286     RefPtr<Frame> protectedFrame(m_frame);
     286    RefPtr<Frame> protectedFrame(m_frame.get());
    287287    Ref<DocumentLoader> protectedThis(*this);
    288288
    … …  
    600600        if (!redirectingOrigin.get().canDisplay(newRequest.url())) {
    601601            RELEASE_LOG_IF_ALLOWED("willSendRequest: canceling - redirecting URL not allowed to display content from target");
    602             FrameLoader::reportLocalLoadFailed(m_frame, newRequest.url().string());
     602            FrameLoader::reportLocalLoadFailed(m_frame.get(), newRequest.url().string());
    603603            cancelMainResourceLoad(frameLoader()->cancelledError(newRequest));
    604604            return completionHandler(WTFMove(newRequest));
    … …  
    10491049    // Both unloading the old page and parsing the new page may execute JavaScript which destroys the datasource
    10501050    // by starting a new load, so retain temporarily.
    1051     RefPtr<Frame> protectedFrame(m_frame);
     1051    RefPtr<Frame> protectedFrame(m_frame.get());
    10521052    Ref<DocumentLoader> protectedThis(*this);
    10531053
    … …  
    13031303        ASSERT_WITH_MESSAGE(m_frame, "detachFromFrame() is being called on a DocumentLoader that has never attached to any Frame");
    13041304#endif
    1305     RefPtr<Frame> protectedFrame(m_frame);
     1305    RefPtr<Frame> protectedFrame(m_frame.get());
    13061306    Ref<DocumentLoader> protectedThis(*this);
    13071307
    … …  
    17881788        return;
    17891789    checkLoadComplete();
    1790     if (Frame* frame = m_frame)
    1791         frame->loader().subresourceLoadDone(type);
     1790    if (m_frame)
     1791        m_frame->loader().subresourceLoadDone(type);
    17921792}
    17931793
    … …  
    20742074
    20752075    checkLoadComplete();
    2076     if (Frame* frame = m_frame)
    2077         frame->loader().checkLoadComplete();   
     2076    if (m_frame)
     2077        m_frame->loader().checkLoadComplete();
    20782078}
    20792079

  • trunk/Source/WebCore/loader/DocumentWriter.cpp

    r268114 r269435  
    118118    if (!m_frame->loader().client().hasHTMLView())
    119119        return Document::createNonRenderedPlaceholder(*m_frame, url);
    120     return DOMImplementation::createDocument(m_mimeType, m_frame, m_frame->settings(), url);
     120    return DOMImplementation::createDocument(m_mimeType, m_frame.get(), m_frame->settings(), url);
    121121}
    122122
    … …  
    223223        // FIXME: This might be too cautious for non-7bit-encodings and
    224224        // we may consider relaxing this later after testing.
    225         if (canReferToParentFrameEncoding(m_frame, parentFrame))
     225        if (canReferToParentFrameEncoding(m_frame.get(), parentFrame))
    226226            m_decoder->setHintEncoding(parentFrame->document()->decoder());
    227227        if (m_encoding.isEmpty()) {
    228             if (canReferToParentFrameEncoding(m_frame, parentFrame))
     228            if (canReferToParentFrameEncoding(m_frame.get(), parentFrame))
    229229                m_decoder->setEncoding(parentFrame->document()->textEncoding(), TextResourceDecoder::EncodingFromParentFrame);
    230230        } else {
    … …  
    298298}
    299299
     300void DocumentWriter::setFrame(Frame& frame)
     301{
     302    m_frame = makeWeakPtr(frame);
     303}
     304
    300305void DocumentWriter::setDocumentWasLoadedAsPartOfNavigation()
    301306{

  • trunk/Source/WebCore/loader/DocumentWriter.h

    r247017 r269435  
    5151    WEBCORE_EXPORT void end();
    5252
    53     void setFrame(Frame& frame) { m_frame = &frame; }
     53    void setFrame(Frame&);
    5454
    5555    WEBCORE_EXPORT void setEncoding(const String& encoding, bool userChosen);
    … …  
    6868    void clear();
    6969
    70     Frame* m_frame { nullptr };
     70    WeakPtr<Frame> m_frame;
    7171
    7272    bool m_hasReceivedSomeData { false };

  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r269434 r269435  
    331331{
    332332    for (auto& frame : m_openedFrames)
    333         frame->loader().m_opener = nullptr;
     333        frame.loader().m_opener = nullptr;
    334334    m_openedFrames.clear();
    335335}
    … …  
    383383{
    384384    return client().frameID();
     385}
     386
     387Frame* FrameLoader::opener()
     388{
     389    return m_opener;
     390}
     391
     392const Frame* FrameLoader::opener() const
     393{
     394    return m_opener;
    385395}
    386396
    … …  
    10511061        // When setOpener is called in ~FrameLoader, opener's m_frameLoader is already cleared.
    10521062        auto& openerFrameLoader = m_opener == &m_frame ? *this : m_opener->loader();
    1053         openerFrameLoader.m_openedFrames.remove(&m_frame);
     1063        openerFrameLoader.m_openedFrames.remove(m_frame);
    10541064    }
    10551065    if (opener) {
    … …  
    10581068            page->setOpenedByDOMWithOpener();
    10591069    }
    1060     m_opener = opener;
     1070    m_opener = makeWeakPtr(opener);
    10611071
    10621072    if (m_frame.document())
    … …  
    16401650}
    16411651
     1652bool FrameLoader::hasOpenedFrames() const
     1653{
     1654    return !m_openedFrames.computesEmpty();
     1655}
     1656
    16421657void FrameLoader::reportLocalLoadFailed(Frame* frame, const String& url)
    16431658{
    … …  
    28922907            auto* ownerFrame = m_frame.tree().parent();
    28932908            if (!ownerFrame && m_stateMachine.isDisplayingInitialEmptyDocument())
    2894                 ownerFrame = m_opener;
     2909                ownerFrame = m_opener.get();
    28952910            if (ownerFrame)
    28962911                initiator = ownerFrame->document();

  • trunk/Source/WebCore/loader/FrameLoader.h

    r269434 r269435  
    5454#include <wtf/UniqueRef.h>
    5555#include <wtf/WallTime.h>
     56#include <wtf/WeakHashSet.h>
    5657
    5758namespace WebCore {
    … …  
    249250    bool checkIfFormActionAllowedByCSP(const URL&, bool didReceiveRedirectResponse) const;
    250251
    251     Frame* opener() { return m_opener; }
    252     const Frame* opener() const { return m_opener; }
     252    WEBCORE_EXPORT Frame* opener();
     253    WEBCORE_EXPORT const Frame* opener() const;
    253254    WEBCORE_EXPORT void setOpener(Frame*);
    254255    WEBCORE_EXPORT void detachFromAllOpenedFrames();
    … …  
    427428    // PolicyChecker specific.
    428429    void clearProvisionalLoadForPolicyCheck();
    429     bool hasOpenedFrames() const { return !m_openedFrames.isEmpty(); }
     430    bool hasOpenedFrames() const;
    430431
    431432    bool preventsParentFromBeingComplete(const Frame&) const;
    … …  
    483484    bool m_shouldCallCheckLoadComplete;
    484485
    485     Frame* m_opener;
    486     HashSet<Frame*> m_openedFrames;
     486    WeakPtr<Frame> m_opener;
     487    WeakHashSet<Frame> m_openedFrames;
    487488
    488489    bool m_loadingFromCachedPage;

  • trunk/Source/WebCore/loader/FrameNetworkingContext.h

    r220208 r269435  
    4545protected:
    4646    explicit FrameNetworkingContext(Frame* frame)
    47         : m_frame(frame)
     47        : m_frame(makeWeakPtr(frame))
    4848    {
    4949    }
    5050
    51     Frame* frame() const { return m_frame; }
     51    Frame* frame() const { return m_frame.get(); }
    5252
    5353private:
    54     bool isValid() const override { return m_frame; }
     54    bool isValid() const override { return !!m_frame; }
    5555
    56     Frame* m_frame;
     56    WeakPtr<Frame> m_frame;
    5757};
    5858

  • trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp

    r263775 r269435  
    417417
    418418    ASSERT(!m_frame);
    419     m_frame = &frame;
     419    m_frame = makeWeakPtr(frame);
    420420
    421421    setUpdateStatus(Checking);
    … …  
    438438
    439439    m_currentResourceIdentifier = m_frame->page()->progress().createUniqueIdentifier();
    440     InspectorInstrumentation::willSendRequest(m_frame, m_currentResourceIdentifier, m_frame->loader().documentLoader(), request, ResourceResponse { });
     440    InspectorInstrumentation::willSendRequest(m_frame.get(), m_currentResourceIdentifier, m_frame->loader().documentLoader(), request, ResourceResponse { });
    441441
    442442    m_manifestLoader = ApplicationCacheResourceLoader::create(ApplicationCacheResource::Type::Manifest, documentLoader.cachedResourceLoader(), WTFMove(request), [this] (auto&& resourceOrError) {
    … …  
    448448            if (error == ApplicationCacheResourceLoader::Error::CannotCreateResource) {
    449449                // FIXME: We should get back the error from ApplicationCacheResourceLoader level.
    450                 InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, ResourceError { ResourceError::Type::AccessControl });
     450                InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, ResourceError { ResourceError::Type::AccessControl });
    451451                this->cacheUpdateFailed();
    452452                return;
    … …  
    495495    // FIXME: We should have NetworkLoadMetrics for ApplicationCache loads.
    496496    NetworkLoadMetrics emptyMetrics;
    497     InspectorInstrumentation::didFinishLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, emptyMetrics, nullptr);
     497    InspectorInstrumentation::didFinishLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, emptyMetrics, nullptr);
    498498
    499499    ASSERT(m_pendingEntries.contains(entryURL.string()));
    … …  
    544544    ResourceError resourceError { error == ApplicationCacheResourceLoader::Error::CannotCreateResource ? ResourceError::Type::AccessControl : ResourceError::Type::General };
    545545
    546     InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, resourceError);
     546    InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, resourceError);
    547547
    548548    URL url(entryURL);
    … …  
    664664        break;
    665665    case ApplicationCacheResourceLoader::Error::NotFound:
    666         InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
     666        InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
    667667        m_frame->document()->addConsoleMessage(MessageSource::AppCache, MessageLevel::Error, makeString("Application Cache manifest could not be fetched, because the manifest had a ", m_manifestLoader->resource()->response().httpStatusCode(), " response."));
    668668        manifestNotFound();
    669669        break;
    670670    case ApplicationCacheResourceLoader::Error::NotOK:
    671         InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
     671        InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
    672672        m_frame->document()->addConsoleMessage(MessageSource::AppCache, MessageLevel::Error, makeString("Application Cache manifest could not be fetched, because the manifest had a ", m_manifestLoader->resource()->response().httpStatusCode(), " response."));
    673673        cacheUpdateFailed();
    674674        break;
    675675    case ApplicationCacheResourceLoader::Error::RedirectForbidden:
    676         InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
     676        InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
    677677        m_frame->document()->addConsoleMessage(MessageSource::AppCache, MessageLevel::Error, "Application Cache manifest could not be fetched, because a redirection was attempted."_s);
    678678        cacheUpdateFailed();
    … …  
    900900
    901901    m_currentResourceIdentifier = m_frame->page()->progress().createUniqueIdentifier();
    902     InspectorInstrumentation::willSendRequest(m_frame, m_currentResourceIdentifier, m_frame->loader().documentLoader(), request, ResourceResponse { });
     902    InspectorInstrumentation::willSendRequest(m_frame.get(), m_currentResourceIdentifier, m_frame->loader().documentLoader(), request, ResourceResponse { });
    903903
    904904    auto& documentLoader = *m_frame->loader().documentLoader();

  • trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.h

    r250283 r269435  
    160160    // Frame used for fetching resources when updating.
    161161    // FIXME: An update started by a particular frame should not stop if it is destroyed, but there are other frames associated with the same cache group.
    162     Frame* m_frame { nullptr };
     162    WeakPtr<Frame> m_frame;
    163163 
    164164    // An obsolete cache group is never stored, but the opposite is not true - storing may fail for multiple reasons, such as exceeding disk quota.

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

    r231963 r269435  
    2828#include <wtf/Ref.h>
    2929#include <wtf/ThreadSafeRefCounted.h>
     30#include <wtf/WeakPtr.h>
    3031
    3132namespace WebCore {
    … …  
    3536
    3637// FIXME: Rename Frame to LocalFrame and AbstractFrame to Frame.
    37 class AbstractFrame : public ThreadSafeRefCounted<AbstractFrame> {
     38class AbstractFrame : public ThreadSafeRefCounted<AbstractFrame>, public CanMakeWeakPtr<AbstractFrame> {
    3839public:
    3940    virtual ~AbstractFrame();

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

    r269434 r269435  
    164164    if (ownerElement) {
    165165        m_mainFrame.selfOnlyRef();
    166         ownerElement->setContentFrame(this);
     166        ownerElement->setContentFrame(*this);
    167167    }
    168168
    … …  
    218218}
    219219
    220 void Frame::addDestructionObserver(FrameDestructionObserver* observer)
    221 {
    222     m_destructionObservers.add(observer);
    223 }
    224 
    225 void Frame::removeDestructionObserver(FrameDestructionObserver* observer)
    226 {
    227     m_destructionObservers.remove(observer);
     220void Frame::addDestructionObserver(FrameDestructionObserver& observer)
     221{
     222    m_destructionObservers.add(&observer);
     223}
     224
     225void Frame::removeDestructionObserver(FrameDestructionObserver& observer)
     226{
     227    m_destructionObservers.remove(&observer);
    228228}
    229229

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

    r269434 r269435  
    149149    WEBCORE_EXPORT DOMWindow* window() const;
    150150
    151     void addDestructionObserver(FrameDestructionObserver*);
    152     void removeDestructionObserver(FrameDestructionObserver*);
     151    void addDestructionObserver(FrameDestructionObserver&);
     152    void removeDestructionObserver(FrameDestructionObserver&);
    153153
    154154    WEBCORE_EXPORT void willDetachPage();

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

    r185231 r269435  
    4343}
    4444
     45Frame* FrameDestructionObserver::frame() const
     46{
     47    return m_frame.get();
     48}
     49
    4550void FrameDestructionObserver::observeFrame(Frame* frame)
    4651{
    4752    if (m_frame)
    48         m_frame->removeDestructionObserver(this);
     53        m_frame->removeDestructionObserver(*this);
    4954
    50     m_frame = frame;
     55    m_frame = makeWeakPtr(frame);
    5156
    5257    if (m_frame)
    53         m_frame->addDestructionObserver(this);
     58        m_frame->addDestructionObserver(*this);
    5459}
    5560

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

    r228483 r269435  
    2626#pragma once
    2727
     28#include <wtf/WeakPtr.h>
     29
    2830namespace WebCore {
    2931
    … …  
    3739    WEBCORE_EXPORT virtual void willDetachPage();
    3840
    39     Frame* frame() const { return m_frame; }
     41    WEBCORE_EXPORT Frame* frame() const;
    4042
    4143protected:
    … …  
    4345    WEBCORE_EXPORT void observeFrame(Frame*);
    4446
    45     Frame* m_frame;
     47    WeakPtr<Frame> m_frame;
    4648};
    4749

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

    r264372 r269435  
    3737namespace WebCore {
    3838
     39FrameTree::FrameTree(Frame& thisFrame, Frame* parentFrame)
     40    : m_thisFrame(thisFrame)
     41    , m_parent(makeWeakPtr(parentFrame))
     42{
     43}
     44
    3945FrameTree::~FrameTree()
    4046{
    … …  
    6268Frame* FrameTree::parent() const
    6369{
    64     return m_parent;
     70    return m_parent.get();
    6571}
    6672
    … …  
    6874{
    6975    ASSERT(child.page() == m_thisFrame.page());
    70     child.tree().m_parent = &m_thisFrame;
    71     Frame* oldLast = m_lastChild;
    72     m_lastChild = &child;
     76    child.tree().m_parent = makeWeakPtr(m_thisFrame);
     77    WeakPtr<Frame> oldLast = m_lastChild;
     78    m_lastChild = makeWeakPtr(child);
    7379
    7480    if (oldLast) {
    … …  
    8591void FrameTree::removeChild(Frame& child)
    8692{
    87     Frame*& newLocationForPrevious = m_lastChild == &child ? m_lastChild : child.tree().m_nextSibling->tree().m_previousSibling;
     93    WeakPtr<Frame>& newLocationForPrevious = m_lastChild == &child ? m_lastChild : child.tree().m_nextSibling->tree().m_previousSibling;
    8894    RefPtr<Frame>& newLocationForNext = m_firstChild == &child ? m_firstChild : child.tree().m_previousSibling->tree().m_nextSibling;
    8995
    … …  
    424430        return m_nextSibling->tree().deepFirstChild();
    425431    if (m_parent)
    426         return m_parent;
     432        return m_parent.get();
    427433    if (canWrap == CanWrap::Yes)
    428434        return deepFirstChild();

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

    r264372 r269435  
    3535    static constexpr unsigned invalidCount = static_cast<unsigned>(-1);
    3636
    37     FrameTree(Frame& thisFrame, Frame* parentFrame)
    38         : m_thisFrame(thisFrame)
    39         , m_parent(parentFrame)
    40         , m_previousSibling(nullptr)
    41         , m_lastChild(nullptr)
    42         , m_scopedChildCount(invalidCount)
    43     {
    44     }
     37    FrameTree(Frame& thisFrame, Frame* parentFrame);
    4538
    4639    ~FrameTree();
    … …  
    5346   
    5447    Frame* nextSibling() const { return m_nextSibling.get(); }
    55     Frame* previousSibling() const { return m_previousSibling; }
     48    Frame* previousSibling() const { return m_previousSibling.get(); }
    5649    Frame* firstChild() const { return m_firstChild.get(); }
    57     Frame* lastChild() const { return m_lastChild; }
     50    Frame* lastChild() const { return m_lastChild.get(); }
    5851
    5952    Frame* firstRenderedChild() const;
    … …  
    10194    Frame& m_thisFrame;
    10295
    103     Frame* m_parent;
     96    WeakPtr<Frame> m_parent;
    10497    AtomString m_name; // The actual frame name (may be empty).
    10598    AtomString m_uniqueName;
    10699
    107100    RefPtr<Frame> m_nextSibling;
    108     Frame* m_previousSibling;
     101    WeakPtr<Frame> m_previousSibling;
    109102    RefPtr<Frame> m_firstChild;
    110     Frame* m_lastChild;
    111     mutable unsigned m_scopedChildCount;
     103    WeakPtr<Frame> m_lastChild;
     104    mutable unsigned m_scopedChildCount { invalidCount };
    112105    mutable uint64_t m_frameIDGenerator { 0 };
    113106};

  • trunk/Source/WebCore/rendering/RenderScrollbar.cpp

    r264525 r269435  
    4545    : Scrollbar(scrollableArea, orientation, ScrollbarControlSize::Regular, RenderScrollbarTheme::renderScrollbarTheme(), true)
    4646    , m_ownerElement(ownerElement)
    47     , m_owningFrame(owningFrame)
     47    , m_owningFrame(makeWeakPtr(owningFrame))
    4848{
    4949    ASSERT(ownerElement || owningFrame);

  • trunk/Source/WebCore/rendering/RenderScrollbar.h

    r264525 r269435  
    8686    RefPtr<Element> m_ownerElement;
    8787
    88     Frame* m_owningFrame;
     88    WeakPtr<Frame> m_owningFrame;
    8989    HashMap<unsigned, RenderPtr<RenderScrollbarPart>> m_parts;
    9090};

  • trunk/Source/WebKit/ChangeLog

    r269434 r269435  
     12020-11-05  Alex Christensen  <achristensen@webkit.org>
     2
     3        Store WeakPtr<Frame> instead of Frame*
     4        https://bugs.webkit.org/show_bug.cgi?id=218599
     5
     6        Reviewed by Youenn Fablet.
     7
     8        * WebProcess/WebPage/WebFrame.cpp:
     9        (WebKit::WebFrame::initWithCoreMainFrame):
     10        (WebKit::WebFrame::createSubframe):
     11        (WebKit::WebFrame::coreFrame const):
     12        (WebKit::WebFrame::info const):
     13        (WebKit::WebFrame::handlesPageScaleGesture const):
     14        (WebKit::WebFrame::requiresUnifiedScaleFactor const):
     15        * WebProcess/WebPage/WebFrame.h:
     16        (WebKit::WebFrame::coreFrame const): Deleted.
     17
    1182020-11-05  Alex Christensen  <achristensen@webkit.org>
    219

  • trunk/Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/DOMObjectCache.cpp

    r248846 r269435  
    171171    {
    172172        clear();
    173         WebCore::Frame* frame = m_frame;
     173        WebCore::Frame* frame = m_frame.get();
    174174        FrameDestructionObserver::frameDestroyed();
    175175        domObjectCacheFrameObservers().remove(frame);

  • trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp

    r269323 r269435  
    107107    page.send(Messages::WebPageProxy::DidCreateMainFrame(frameID()));
    108108
    109     m_coreFrame = &coreFrame;
     109    m_coreFrame = makeWeakPtr(coreFrame);
    110110    m_coreFrame->tree().setName(String());
    111111    m_coreFrame->init();
    … …  
    118118
    119119    auto coreFrame = Frame::create(page->corePage(), ownerElement, makeUniqueRef<WebFrameLoaderClient>(frame.get()));
    120     frame->m_coreFrame = coreFrame.ptr();
     120    frame->m_coreFrame = makeWeakPtr(coreFrame.get());
    121121
    122122    coreFrame->tree().setName(frameName);
    … …  
    178178}
    179179
     180WebCore::Frame* WebFrame::coreFrame() const
     181{
     182    return m_coreFrame.get();
     183}
     184
    180185FrameInfoData WebFrame::info() const
    181186{
    … …  
    186191        // FIXME: This should use the full request.
    187192        ResourceRequest(url()),
    188         SecurityOriginData::fromFrame(m_coreFrame),
     193        SecurityOriginData::fromFrame(m_coreFrame.get()),
    189194        m_frameID,
    190195        parent ? Optional<WebCore::FrameIdentifier> { parent->frameID() } : WTF::nullopt,
    … …  
    534539bool WebFrame::handlesPageScaleGesture() const
    535540{
    536     auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame);
     541    auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame.get());
    537542    return pluginView && pluginView->handlesPageScaleFactor();
    538543}
    … …  
    540545bool WebFrame::requiresUnifiedScaleFactor() const
    541546{
    542     auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame);
     547    auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame.get());
    543548    return pluginView && pluginView->requiresUnifiedScaleFactor();
    544549}

  • trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h

    r268989 r269435  
    7979
    8080    static WebFrame* fromCoreFrame(const WebCore::Frame&);
    81     WebCore::Frame* coreFrame() const { return m_coreFrame; }
     81    WebCore::Frame* coreFrame() const;
    8282
    8383    FrameInfoData info() const;
    … …  
    185185    WebFrame();
    186186
    187     WebCore::Frame* m_coreFrame { nullptr };
     187    WeakPtr<WebCore::Frame> m_coreFrame;
    188188
    189189    uint64_t m_policyListenerID { 0 };
Note: See TracChangeset for help on using the changeset viewer.