Diff
Modified: trunk/Source/WebCore/ChangeLog (269434 => 269435)
--- trunk/Source/WebCore/ChangeLog 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/ChangeLog 2020-11-05 16:54:48 UTC (rev 269435)
@@ -1,5 +1,89 @@
2020-11-05 Alex Christensen <[email protected]>
+ Store WeakPtr<Frame> instead of Frame*
+ https://bugs.webkit.org/show_bug.cgi?id=218599
+
+ Reviewed by Youenn Fablet.
+
+ 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:
+
+2020-11-05 Alex Christensen <[email protected]>
+
Use fewer raw pointers and more const correctness in Frame.h
https://bugs.webkit.org/show_bug.cgi?id=218598
Modified: trunk/Source/WebCore/bindings/js/WindowProxy.cpp (269434 => 269435)
--- trunk/Source/WebCore/bindings/js/WindowProxy.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/bindings/js/WindowProxy.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -53,7 +53,7 @@
}
WindowProxy::WindowProxy(AbstractFrame& frame)
- : m_frame(&frame)
+ : m_frame(makeWeakPtr(frame))
, m_jsWindowProxies(makeUniqueRef<ProxyMap>())
{
}
@@ -64,6 +64,11 @@
ASSERT(m_jsWindowProxies->isEmpty());
}
+AbstractFrame* WindowProxy::frame() const
+{
+ return m_frame.get();
+}
+
void WindowProxy::detachFromFrame()
{
ASSERT(m_frame);
Modified: trunk/Source/WebCore/bindings/js/WindowProxy.h (269434 => 269435)
--- trunk/Source/WebCore/bindings/js/WindowProxy.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/bindings/js/WindowProxy.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -24,6 +24,7 @@
#include <wtf/HashMap.h>
#include <wtf/RefCounted.h>
#include <wtf/UniqueRef.h>
+#include <wtf/WeakPtr.h>
namespace JSC {
class Debugger;
@@ -49,7 +50,7 @@
WEBCORE_EXPORT ~WindowProxy();
- AbstractFrame* frame() const { return m_frame; }
+ WEBCORE_EXPORT AbstractFrame* frame() const;
void detachFromFrame();
void destroyJSWindowProxy(DOMWrapperWorld&);
@@ -94,7 +95,7 @@
JSWindowProxy& createJSWindowProxy(DOMWrapperWorld&);
WEBCORE_EXPORT JSWindowProxy& createJSWindowProxyWithInitializedScript(DOMWrapperWorld&);
- AbstractFrame* m_frame;
+ WeakPtr<AbstractFrame> m_frame;
UniqueRef<ProxyMap> m_jsWindowProxies;
};
Modified: trunk/Source/WebCore/dom/Document.cpp (269434 => 269435)
--- trunk/Source/WebCore/dom/Document.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/dom/Document.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -2558,7 +2558,7 @@
#endif
{
- NavigationDisabler navigationDisabler(m_frame);
+ NavigationDisabler navigationDisabler(m_frame.get());
disconnectDescendantFrames();
}
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!m_frame || !m_frame->tree().childCount());
@@ -3466,13 +3466,13 @@
return true;
// iii. A sandboxed frame can always navigate its descendants.
- if (isSandboxed(SandboxNavigation) && targetFrame.tree().isDescendantOf(m_frame))
+ if (isSandboxed(SandboxNavigation) && targetFrame.tree().isDescendantOf(m_frame.get()))
return true;
// From https://html.spec.whatwg.org/multipage/browsers.html#allowed-to-navigate.
// 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
// flag set has its sandboxed navigation browsing context flag set, then abort these steps negatively.
- if (m_frame != &targetFrame && isSandboxed(SandboxNavigation) && targetFrame.tree().parent() && !targetFrame.tree().isDescendantOf(m_frame)) {
+ if (m_frame != &targetFrame && isSandboxed(SandboxNavigation) && targetFrame.tree().parent() && !targetFrame.tree().isDescendantOf(m_frame.get())) {
printNavigationErrorMessage(targetFrame, url(), "The frame attempting navigation is sandboxed, and is therefore disallowed from navigating its ancestors."_s);
return false;
}
@@ -5700,7 +5700,7 @@
void Document::setDesignMode(InheritedBool value)
{
m_designMode = value;
- for (Frame* frame = m_frame; frame && frame->document(); frame = frame->tree().traverseNext(m_frame))
+ for (auto* frame = m_frame.get(); frame && frame->document(); frame = frame->tree().traverseNext(m_frame.get()))
frame->document()->scheduleFullStyleRebuild();
}
@@ -5721,11 +5721,6 @@
setDesignMode(mode);
}
-auto Document::getDesignMode() const -> InheritedBool
-{
- return m_designMode;
-}
-
bool Document::inDesignMode() const
{
for (const Document* d = this; d; d = d->parentDocument()) {
Modified: trunk/Source/WebCore/dom/Document.h (269434 => 269435)
--- trunk/Source/WebCore/dom/Document.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/dom/Document.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -1007,7 +1007,6 @@
// designMode support
enum InheritedBool { off = false, on = true, inherit };
void setDesignMode(InheritedBool value);
- InheritedBool getDesignMode() const;
bool inDesignMode() const;
WEBCORE_EXPORT String designMode() const;
WEBCORE_EXPORT void setDesignMode(const String&);
Modified: trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp (269434 => 269435)
--- trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -50,14 +50,13 @@
return downcast<RenderWidget>(renderer());
}
-void HTMLFrameOwnerElement::setContentFrame(Frame* frame)
+void HTMLFrameOwnerElement::setContentFrame(Frame& frame)
{
// Make sure we will not end up with two frames referencing the same owner element.
ASSERT(!m_contentFrame || m_contentFrame->ownerElement() != this);
- ASSERT(frame);
// Disconnected frames should not be allowed to load.
ASSERT(isConnected());
- m_contentFrame = frame;
+ m_contentFrame = makeWeakPtr(frame);
for (RefPtr<ContainerNode> node = this; node; node = node->parentOrShadowHostNode())
node->incrementConnectedSubframeCount();
@@ -76,7 +75,7 @@
void HTMLFrameOwnerElement::disconnectContentFrame()
{
- if (RefPtr<Frame> frame = m_contentFrame) {
+ if (RefPtr<Frame> frame = m_contentFrame.get()) {
frame->loader().frameDetached();
frame->disconnectOwnerElement();
}
Modified: trunk/Source/WebCore/html/HTMLFrameOwnerElement.h (269434 => 269435)
--- trunk/Source/WebCore/html/HTMLFrameOwnerElement.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/html/HTMLFrameOwnerElement.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -35,11 +35,11 @@
public:
virtual ~HTMLFrameOwnerElement();
- Frame* contentFrame() const { return m_contentFrame; }
+ Frame* contentFrame() const { return m_contentFrame.get(); }
WEBCORE_EXPORT WindowProxy* contentWindow() const;
WEBCORE_EXPORT Document* contentDocument() const;
- void setContentFrame(Frame*);
+ void setContentFrame(Frame&);
void clearContentFrame();
void disconnectContentFrame();
@@ -73,7 +73,7 @@
bool isKeyboardFocusable(KeyboardEvent*) const override;
bool isFrameOwnerElement() const final { return true; }
- Frame* m_contentFrame { nullptr };
+ WeakPtr<Frame> m_contentFrame;
SandboxFlags m_sandboxFlags { SandboxNone };
};
Modified: trunk/Source/WebCore/inspector/agents/InspectorPageAgent.cpp (269434 => 269435)
--- trunk/Source/WebCore/inspector/agents/InspectorPageAgent.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/inspector/agents/InspectorPageAgent.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -828,7 +828,7 @@
Frame* InspectorPageAgent::frameForId(const Protocol::Network::FrameId& frameId)
{
- return frameId.isEmpty() ? nullptr : m_identifierToFrame.get(frameId);
+ return frameId.isEmpty() ? nullptr : m_identifierToFrame.get(frameId).get();
}
String InspectorPageAgent::frameId(Frame* frame)
@@ -837,7 +837,7 @@
return emptyString();
return m_frameToIdentifier.ensure(frame, [this, frame] {
auto identifier = IdentifiersFactory::createIdentifier();
- m_identifierToFrame.set(identifier, frame);
+ m_identifierToFrame.set(identifier, makeWeakPtr(frame));
return identifier;
}).iterator->value;
}
Modified: trunk/Source/WebCore/inspector/agents/InspectorPageAgent.h (269434 => 269435)
--- trunk/Source/WebCore/inspector/agents/InspectorPageAgent.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/inspector/agents/InspectorPageAgent.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -165,8 +165,9 @@
InspectorClient* m_client { nullptr };
InspectorOverlay* m_overlay { nullptr };
+ // FIXME: Make a WeakHashMap and use it for m_frameToIdentifier and m_loaderToIdentifier.
HashMap<Frame*, String> m_frameToIdentifier;
- HashMap<String, Frame*> m_identifierToFrame;
+ HashMap<String, WeakPtr<Frame>> m_identifierToFrame;
HashMap<DocumentLoader*, String> m_loaderToIdentifier;
String m_userAgentOverride;
String m_emulatedMedia;
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (269434 => 269435)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -283,7 +283,7 @@
// but not loads initiated by child frames' data sources -- that's the WebFrame's job.
void DocumentLoader::stopLoading()
{
- RefPtr<Frame> protectedFrame(m_frame);
+ RefPtr<Frame> protectedFrame(m_frame.get());
Ref<DocumentLoader> protectedThis(*this);
// In some rare cases, calling FrameLoader::stopLoading could cause isLoading() to return false.
@@ -599,7 +599,7 @@
Ref<SecurityOrigin> redirectingOrigin(SecurityOrigin::create(redirectResponse.url()));
if (!redirectingOrigin.get().canDisplay(newRequest.url())) {
RELEASE_LOG_IF_ALLOWED("willSendRequest: canceling - redirecting URL not allowed to display content from target");
- FrameLoader::reportLocalLoadFailed(m_frame, newRequest.url().string());
+ FrameLoader::reportLocalLoadFailed(m_frame.get(), newRequest.url().string());
cancelMainResourceLoad(frameLoader()->cancelledError(newRequest));
return completionHandler(WTFMove(newRequest));
}
@@ -1048,7 +1048,7 @@
{
// Both unloading the old page and parsing the new page may execute _javascript_ which destroys the datasource
// by starting a new load, so retain temporarily.
- RefPtr<Frame> protectedFrame(m_frame);
+ RefPtr<Frame> protectedFrame(m_frame.get());
Ref<DocumentLoader> protectedThis(*this);
commitIfReady();
@@ -1302,7 +1302,7 @@
else
ASSERT_WITH_MESSAGE(m_frame, "detachFromFrame() is being called on a DocumentLoader that has never attached to any Frame");
#endif
- RefPtr<Frame> protectedFrame(m_frame);
+ RefPtr<Frame> protectedFrame(m_frame.get());
Ref<DocumentLoader> protectedThis(*this);
// It never makes sense to have a document loader that is detached from its
@@ -1787,8 +1787,8 @@
if (!m_subresourceLoaders.remove(loader->identifier()))
return;
checkLoadComplete();
- if (Frame* frame = m_frame)
- frame->loader().subresourceLoadDone(type);
+ if (m_frame)
+ m_frame->loader().subresourceLoadDone(type);
}
void DocumentLoader::addPlugInStreamLoader(ResourceLoader& loader)
@@ -2073,8 +2073,8 @@
}
checkLoadComplete();
- if (Frame* frame = m_frame)
- frame->loader().checkLoadComplete();
+ if (m_frame)
+ m_frame->loader().checkLoadComplete();
}
void DocumentLoader::maybeFinishLoadingMultipartContent()
Modified: trunk/Source/WebCore/loader/DocumentWriter.cpp (269434 => 269435)
--- trunk/Source/WebCore/loader/DocumentWriter.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/loader/DocumentWriter.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -117,7 +117,7 @@
#endif
if (!m_frame->loader().client().hasHTMLView())
return Document::createNonRenderedPlaceholder(*m_frame, url);
- return DOMImplementation::createDocument(m_mimeType, m_frame, m_frame->settings(), url);
+ return DOMImplementation::createDocument(m_mimeType, m_frame.get(), m_frame->settings(), url);
}
bool DocumentWriter::begin(const URL& urlReference, bool dispatch, Document* ownerDocument)
@@ -222,10 +222,10 @@
// an attack vector.
// FIXME: This might be too cautious for non-7bit-encodings and
// we may consider relaxing this later after testing.
- if (canReferToParentFrameEncoding(m_frame, parentFrame))
+ if (canReferToParentFrameEncoding(m_frame.get(), parentFrame))
m_decoder->setHintEncoding(parentFrame->document()->decoder());
if (m_encoding.isEmpty()) {
- if (canReferToParentFrameEncoding(m_frame, parentFrame))
+ if (canReferToParentFrameEncoding(m_frame.get(), parentFrame))
m_decoder->setEncoding(parentFrame->document()->textEncoding(), TextResourceDecoder::EncodingFromParentFrame);
} else {
m_decoder->setEncoding(m_encoding,
@@ -297,6 +297,11 @@
m_encodingWasChosenByUser = userChosen;
}
+void DocumentWriter::setFrame(Frame& frame)
+{
+ m_frame = makeWeakPtr(frame);
+}
+
void DocumentWriter::setDocumentWasLoadedAsPartOfNavigation()
{
ASSERT(m_parser && !m_parser->isStopped());
Modified: trunk/Source/WebCore/loader/DocumentWriter.h (269434 => 269435)
--- trunk/Source/WebCore/loader/DocumentWriter.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/loader/DocumentWriter.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -50,7 +50,7 @@
void insertDataSynchronously(const String&); // For an internal use only to prevent the parser from yielding.
WEBCORE_EXPORT void end();
- void setFrame(Frame& frame) { m_frame = &frame; }
+ void setFrame(Frame&);
WEBCORE_EXPORT void setEncoding(const String& encoding, bool userChosen);
@@ -67,7 +67,7 @@
Ref<Document> createDocument(const URL&);
void clear();
- Frame* m_frame { nullptr };
+ WeakPtr<Frame> m_frame;
bool m_hasReceivedSomeData { false };
String m_mimeType;
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (269434 => 269435)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -330,7 +330,7 @@
void FrameLoader::detachFromAllOpenedFrames()
{
for (auto& frame : m_openedFrames)
- frame->loader().m_opener = nullptr;
+ frame.loader().m_opener = nullptr;
m_openedFrames.clear();
}
@@ -384,6 +384,16 @@
return client().frameID();
}
+Frame* FrameLoader::opener()
+{
+ return m_opener;
+}
+
+const Frame* FrameLoader::opener() const
+{
+ return m_opener;
+}
+
void FrameLoader::setDefersLoading(bool defers)
{
if (m_documentLoader)
@@ -1050,7 +1060,7 @@
if (m_opener) {
// When setOpener is called in ~FrameLoader, opener's m_frameLoader is already cleared.
auto& openerFrameLoader = m_opener == &m_frame ? *this : m_opener->loader();
- openerFrameLoader.m_openedFrames.remove(&m_frame);
+ openerFrameLoader.m_openedFrames.remove(m_frame);
}
if (opener) {
opener->loader().m_openedFrames.add(&m_frame);
@@ -1057,7 +1067,7 @@
if (auto* page = m_frame.page())
page->setOpenedByDOMWithOpener();
}
- m_opener = opener;
+ m_opener = makeWeakPtr(opener);
if (m_frame.document())
m_frame.document()->initSecurityContext();
@@ -1639,6 +1649,11 @@
setProvisionalDocumentLoader(nullptr);
}
+bool FrameLoader::hasOpenedFrames() const
+{
+ return !m_openedFrames.computesEmpty();
+}
+
void FrameLoader::reportLocalLoadFailed(Frame* frame, const String& url)
{
ASSERT(!url.isEmpty());
@@ -2891,7 +2906,7 @@
if (isMainResource) {
auto* ownerFrame = m_frame.tree().parent();
if (!ownerFrame && m_stateMachine.isDisplayingInitialEmptyDocument())
- ownerFrame = m_opener;
+ ownerFrame = m_opener.get();
if (ownerFrame)
initiator = ownerFrame->document();
ASSERT(ownerFrame || m_frame.isMainFrame());
Modified: trunk/Source/WebCore/loader/FrameLoader.h (269434 => 269435)
--- trunk/Source/WebCore/loader/FrameLoader.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/loader/FrameLoader.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -53,6 +53,7 @@
#include <wtf/Optional.h>
#include <wtf/UniqueRef.h>
#include <wtf/WallTime.h>
+#include <wtf/WeakHashSet.h>
namespace WebCore {
@@ -248,8 +249,8 @@
bool checkIfFormActionAllowedByCSP(const URL&, bool didReceiveRedirectResponse) const;
- Frame* opener() { return m_opener; }
- const Frame* opener() const { return m_opener; }
+ WEBCORE_EXPORT Frame* opener();
+ WEBCORE_EXPORT const Frame* opener() const;
WEBCORE_EXPORT void setOpener(Frame*);
WEBCORE_EXPORT void detachFromAllOpenedFrames();
@@ -426,7 +427,7 @@
// PolicyChecker specific.
void clearProvisionalLoadForPolicyCheck();
- bool hasOpenedFrames() const { return !m_openedFrames.isEmpty(); }
+ bool hasOpenedFrames() const;
bool preventsParentFromBeingComplete(const Frame&) const;
@@ -482,8 +483,8 @@
bool m_shouldCallCheckCompleted;
bool m_shouldCallCheckLoadComplete;
- Frame* m_opener;
- HashSet<Frame*> m_openedFrames;
+ WeakPtr<Frame> m_opener;
+ WeakHashSet<Frame> m_openedFrames;
bool m_loadingFromCachedPage;
Modified: trunk/Source/WebCore/loader/FrameNetworkingContext.h (269434 => 269435)
--- trunk/Source/WebCore/loader/FrameNetworkingContext.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/loader/FrameNetworkingContext.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -44,16 +44,16 @@
protected:
explicit FrameNetworkingContext(Frame* frame)
- : m_frame(frame)
+ : m_frame(makeWeakPtr(frame))
{
}
- Frame* frame() const { return m_frame; }
+ Frame* frame() const { return m_frame.get(); }
private:
- bool isValid() const override { return m_frame; }
+ bool isValid() const override { return !!m_frame; }
- Frame* m_frame;
+ WeakPtr<Frame> m_frame;
};
}
Modified: trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp (269434 => 269435)
--- trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -416,7 +416,7 @@
}
ASSERT(!m_frame);
- m_frame = &frame;
+ m_frame = makeWeakPtr(frame);
setUpdateStatus(Checking);
@@ -437,7 +437,7 @@
auto request = createRequest(URL { m_manifestURL }, m_newestCache ? m_newestCache->manifestResource() : nullptr);
m_currentResourceIdentifier = m_frame->page()->progress().createUniqueIdentifier();
- InspectorInstrumentation::willSendRequest(m_frame, m_currentResourceIdentifier, m_frame->loader().documentLoader(), request, ResourceResponse { });
+ InspectorInstrumentation::willSendRequest(m_frame.get(), m_currentResourceIdentifier, m_frame->loader().documentLoader(), request, ResourceResponse { });
m_manifestLoader = ApplicationCacheResourceLoader::create(ApplicationCacheResource::Type::Manifest, documentLoader.cachedResourceLoader(), WTFMove(request), [this] (auto&& resourceOrError) {
// 'this' is only valid if returned value is not Error::Abort.
@@ -447,7 +447,7 @@
return;
if (error == ApplicationCacheResourceLoader::Error::CannotCreateResource) {
// FIXME: We should get back the error from ApplicationCacheResourceLoader level.
- InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, ResourceError { ResourceError::Type::AccessControl });
+ InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, ResourceError { ResourceError::Type::AccessControl });
this->cacheUpdateFailed();
return;
}
@@ -494,7 +494,7 @@
{
// FIXME: We should have NetworkLoadMetrics for ApplicationCache loads.
NetworkLoadMetrics emptyMetrics;
- InspectorInstrumentation::didFinishLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, emptyMetrics, nullptr);
+ InspectorInstrumentation::didFinishLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, emptyMetrics, nullptr);
ASSERT(m_pendingEntries.contains(entryURL.string()));
@@ -543,7 +543,7 @@
// FIXME: We should get back the error from ApplicationCacheResourceLoader level.
ResourceError resourceError { error == ApplicationCacheResourceLoader::Error::CannotCreateResource ? ResourceError::Type::AccessControl : ResourceError::Type::General };
- InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, resourceError);
+ InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, resourceError);
URL url(entryURL);
url.removeFragmentIdentifier();
@@ -663,17 +663,17 @@
cacheUpdateFailed();
break;
case ApplicationCacheResourceLoader::Error::NotFound:
- InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
+ InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
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."));
manifestNotFound();
break;
case ApplicationCacheResourceLoader::Error::NotOK:
- InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
+ InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
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."));
cacheUpdateFailed();
break;
case ApplicationCacheResourceLoader::Error::RedirectForbidden:
- InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
+ InspectorInstrumentation::didFailLoading(m_frame.get(), m_frame->loader().documentLoader(), m_currentResourceIdentifier, m_frame->loader().cancelledError(m_manifestLoader->resource()->resourceRequest()));
m_frame->document()->addConsoleMessage(MessageSource::AppCache, MessageLevel::Error, "Application Cache manifest could not be fetched, because a redirection was attempted."_s);
cacheUpdateFailed();
break;
@@ -899,7 +899,7 @@
auto request = createRequest(URL { { }, firstPendingEntryURL }, m_newestCache ? m_newestCache->resourceForURL(firstPendingEntryURL) : nullptr);
m_currentResourceIdentifier = m_frame->page()->progress().createUniqueIdentifier();
- InspectorInstrumentation::willSendRequest(m_frame, m_currentResourceIdentifier, m_frame->loader().documentLoader(), request, ResourceResponse { });
+ InspectorInstrumentation::willSendRequest(m_frame.get(), m_currentResourceIdentifier, m_frame->loader().documentLoader(), request, ResourceResponse { });
auto& documentLoader = *m_frame->loader().documentLoader();
auto requestURL = request.url();
Modified: trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.h (269434 => 269435)
--- trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -159,7 +159,7 @@
// Frame used for fetching resources when updating.
// 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.
- Frame* m_frame { nullptr };
+ WeakPtr<Frame> m_frame;
// An obsolete cache group is never stored, but the opposite is not true - storing may fail for multiple reasons, such as exceeding disk quota.
unsigned m_storageID { 0 };
Modified: trunk/Source/WebCore/page/AbstractFrame.h (269434 => 269435)
--- trunk/Source/WebCore/page/AbstractFrame.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/page/AbstractFrame.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -27,6 +27,7 @@
#include <wtf/Ref.h>
#include <wtf/ThreadSafeRefCounted.h>
+#include <wtf/WeakPtr.h>
namespace WebCore {
@@ -34,7 +35,7 @@
class WindowProxy;
// FIXME: Rename Frame to LocalFrame and AbstractFrame to Frame.
-class AbstractFrame : public ThreadSafeRefCounted<AbstractFrame> {
+class AbstractFrame : public ThreadSafeRefCounted<AbstractFrame>, public CanMakeWeakPtr<AbstractFrame> {
public:
virtual ~AbstractFrame();
Modified: trunk/Source/WebCore/page/Frame.cpp (269434 => 269435)
--- trunk/Source/WebCore/page/Frame.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/page/Frame.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -163,7 +163,7 @@
if (ownerElement) {
m_mainFrame.selfOnlyRef();
- ownerElement->setContentFrame(this);
+ ownerElement->setContentFrame(*this);
}
#ifndef NDEBUG
@@ -217,14 +217,14 @@
return m_ownerElement.get();
}
-void Frame::addDestructionObserver(FrameDestructionObserver* observer)
+void Frame::addDestructionObserver(FrameDestructionObserver& observer)
{
- m_destructionObservers.add(observer);
+ m_destructionObservers.add(&observer);
}
-void Frame::removeDestructionObserver(FrameDestructionObserver* observer)
+void Frame::removeDestructionObserver(FrameDestructionObserver& observer)
{
- m_destructionObservers.remove(observer);
+ m_destructionObservers.remove(&observer);
}
void Frame::setView(RefPtr<FrameView>&& view)
Modified: trunk/Source/WebCore/page/Frame.h (269434 => 269435)
--- trunk/Source/WebCore/page/Frame.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/page/Frame.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -148,8 +148,8 @@
WEBCORE_EXPORT DOMWindow* window() const;
- void addDestructionObserver(FrameDestructionObserver*);
- void removeDestructionObserver(FrameDestructionObserver*);
+ void addDestructionObserver(FrameDestructionObserver&);
+ void removeDestructionObserver(FrameDestructionObserver&);
WEBCORE_EXPORT void willDetachPage();
void detachFromPage();
Modified: trunk/Source/WebCore/page/FrameDestructionObserver.cpp (269434 => 269435)
--- trunk/Source/WebCore/page/FrameDestructionObserver.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/page/FrameDestructionObserver.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -42,15 +42,20 @@
}
+Frame* FrameDestructionObserver::frame() const
+{
+ return m_frame.get();
+}
+
void FrameDestructionObserver::observeFrame(Frame* frame)
{
if (m_frame)
- m_frame->removeDestructionObserver(this);
+ m_frame->removeDestructionObserver(*this);
- m_frame = frame;
+ m_frame = makeWeakPtr(frame);
if (m_frame)
- m_frame->addDestructionObserver(this);
+ m_frame->addDestructionObserver(*this);
}
void FrameDestructionObserver::frameDestroyed()
Modified: trunk/Source/WebCore/page/FrameDestructionObserver.h (269434 => 269435)
--- trunk/Source/WebCore/page/FrameDestructionObserver.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/page/FrameDestructionObserver.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -25,6 +25,8 @@
#pragma once
+#include <wtf/WeakPtr.h>
+
namespace WebCore {
class Frame;
@@ -36,13 +38,13 @@
WEBCORE_EXPORT virtual void frameDestroyed();
WEBCORE_EXPORT virtual void willDetachPage();
- Frame* frame() const { return m_frame; }
+ WEBCORE_EXPORT Frame* frame() const;
protected:
WEBCORE_EXPORT virtual ~FrameDestructionObserver();
WEBCORE_EXPORT void observeFrame(Frame*);
- Frame* m_frame;
+ WeakPtr<Frame> m_frame;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/page/FrameTree.cpp (269434 => 269435)
--- trunk/Source/WebCore/page/FrameTree.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/page/FrameTree.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -36,6 +36,12 @@
namespace WebCore {
+FrameTree::FrameTree(Frame& thisFrame, Frame* parentFrame)
+ : m_thisFrame(thisFrame)
+ , m_parent(makeWeakPtr(parentFrame))
+{
+}
+
FrameTree::~FrameTree()
{
for (Frame* child = firstChild(); child; child = child->tree().nextSibling())
@@ -61,15 +67,15 @@
Frame* FrameTree::parent() const
{
- return m_parent;
+ return m_parent.get();
}
void FrameTree::appendChild(Frame& child)
{
ASSERT(child.page() == m_thisFrame.page());
- child.tree().m_parent = &m_thisFrame;
- Frame* oldLast = m_lastChild;
- m_lastChild = &child;
+ child.tree().m_parent = makeWeakPtr(m_thisFrame);
+ WeakPtr<Frame> oldLast = m_lastChild;
+ m_lastChild = makeWeakPtr(child);
if (oldLast) {
child.tree().m_previousSibling = oldLast;
@@ -84,7 +90,7 @@
void FrameTree::removeChild(Frame& child)
{
- Frame*& newLocationForPrevious = m_lastChild == &child ? m_lastChild : child.tree().m_nextSibling->tree().m_previousSibling;
+ WeakPtr<Frame>& newLocationForPrevious = m_lastChild == &child ? m_lastChild : child.tree().m_nextSibling->tree().m_previousSibling;
RefPtr<Frame>& newLocationForNext = m_firstChild == &child ? m_firstChild : child.tree().m_previousSibling->tree().m_nextSibling;
child.tree().m_parent = nullptr;
@@ -423,7 +429,7 @@
if (m_nextSibling)
return m_nextSibling->tree().deepFirstChild();
if (m_parent)
- return m_parent;
+ return m_parent.get();
if (canWrap == CanWrap::Yes)
return deepFirstChild();
return nullptr;
Modified: trunk/Source/WebCore/page/FrameTree.h (269434 => 269435)
--- trunk/Source/WebCore/page/FrameTree.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/page/FrameTree.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -34,14 +34,7 @@
public:
static constexpr unsigned invalidCount = static_cast<unsigned>(-1);
- FrameTree(Frame& thisFrame, Frame* parentFrame)
- : m_thisFrame(thisFrame)
- , m_parent(parentFrame)
- , m_previousSibling(nullptr)
- , m_lastChild(nullptr)
- , m_scopedChildCount(invalidCount)
- {
- }
+ FrameTree(Frame& thisFrame, Frame* parentFrame);
~FrameTree();
@@ -52,9 +45,9 @@
WEBCORE_EXPORT Frame* parent() const;
Frame* nextSibling() const { return m_nextSibling.get(); }
- Frame* previousSibling() const { return m_previousSibling; }
+ Frame* previousSibling() const { return m_previousSibling.get(); }
Frame* firstChild() const { return m_firstChild.get(); }
- Frame* lastChild() const { return m_lastChild; }
+ Frame* lastChild() const { return m_lastChild.get(); }
Frame* firstRenderedChild() const;
Frame* nextRenderedSibling() const;
@@ -100,15 +93,15 @@
Frame& m_thisFrame;
- Frame* m_parent;
+ WeakPtr<Frame> m_parent;
AtomString m_name; // The actual frame name (may be empty).
AtomString m_uniqueName;
RefPtr<Frame> m_nextSibling;
- Frame* m_previousSibling;
+ WeakPtr<Frame> m_previousSibling;
RefPtr<Frame> m_firstChild;
- Frame* m_lastChild;
- mutable unsigned m_scopedChildCount;
+ WeakPtr<Frame> m_lastChild;
+ mutable unsigned m_scopedChildCount { invalidCount };
mutable uint64_t m_frameIDGenerator { 0 };
};
Modified: trunk/Source/WebCore/rendering/RenderScrollbar.cpp (269434 => 269435)
--- trunk/Source/WebCore/rendering/RenderScrollbar.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/rendering/RenderScrollbar.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -44,7 +44,7 @@
RenderScrollbar::RenderScrollbar(ScrollableArea& scrollableArea, ScrollbarOrientation orientation, Element* ownerElement, Frame* owningFrame)
: Scrollbar(scrollableArea, orientation, ScrollbarControlSize::Regular, RenderScrollbarTheme::renderScrollbarTheme(), true)
, m_ownerElement(ownerElement)
- , m_owningFrame(owningFrame)
+ , m_owningFrame(makeWeakPtr(owningFrame))
{
ASSERT(ownerElement || owningFrame);
Modified: trunk/Source/WebCore/rendering/RenderScrollbar.h (269434 => 269435)
--- trunk/Source/WebCore/rendering/RenderScrollbar.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebCore/rendering/RenderScrollbar.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -85,7 +85,7 @@
// FrameView which this Element pointer can in no way keep alive. See webkit bug 80610.
RefPtr<Element> m_ownerElement;
- Frame* m_owningFrame;
+ WeakPtr<Frame> m_owningFrame;
HashMap<unsigned, RenderPtr<RenderScrollbarPart>> m_parts;
};
Modified: trunk/Source/WebKit/ChangeLog (269434 => 269435)
--- trunk/Source/WebKit/ChangeLog 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebKit/ChangeLog 2020-11-05 16:54:48 UTC (rev 269435)
@@ -1,5 +1,22 @@
2020-11-05 Alex Christensen <[email protected]>
+ Store WeakPtr<Frame> instead of Frame*
+ https://bugs.webkit.org/show_bug.cgi?id=218599
+
+ Reviewed by Youenn Fablet.
+
+ * 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.
+
+2020-11-05 Alex Christensen <[email protected]>
+
Use fewer raw pointers and more const correctness in Frame.h
https://bugs.webkit.org/show_bug.cgi?id=218598
Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/DOMObjectCache.cpp (269434 => 269435)
--- trunk/Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/DOMObjectCache.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/DOMObjectCache.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -170,7 +170,7 @@
void frameDestroyed() override
{
clear();
- WebCore::Frame* frame = m_frame;
+ WebCore::Frame* frame = m_frame.get();
FrameDestructionObserver::frameDestroyed();
domObjectCacheFrameObservers().remove(frame);
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp (269434 => 269435)
--- trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp 2020-11-05 16:54:48 UTC (rev 269435)
@@ -106,7 +106,7 @@
{
page.send(Messages::WebPageProxy::DidCreateMainFrame(frameID()));
- m_coreFrame = &coreFrame;
+ m_coreFrame = makeWeakPtr(coreFrame);
m_coreFrame->tree().setName(String());
m_coreFrame->init();
}
@@ -117,7 +117,7 @@
page->send(Messages::WebPageProxy::DidCreateSubframe(frame->frameID()));
auto coreFrame = Frame::create(page->corePage(), ownerElement, makeUniqueRef<WebFrameLoaderClient>(frame.get()));
- frame->m_coreFrame = coreFrame.ptr();
+ frame->m_coreFrame = makeWeakPtr(coreFrame.get());
coreFrame->tree().setName(frameName);
if (ownerElement) {
@@ -177,6 +177,11 @@
return &webFrameLoaderClient->webFrame();
}
+WebCore::Frame* WebFrame::coreFrame() const
+{
+ return m_coreFrame.get();
+}
+
FrameInfoData WebFrame::info() const
{
auto* parent = parentFrame();
@@ -185,7 +190,7 @@
isMainFrame(),
// FIXME: This should use the full request.
ResourceRequest(url()),
- SecurityOriginData::fromFrame(m_coreFrame),
+ SecurityOriginData::fromFrame(m_coreFrame.get()),
m_frameID,
parent ? Optional<WebCore::FrameIdentifier> { parent->frameID() } : WTF::nullopt,
};
@@ -533,13 +538,13 @@
bool WebFrame::handlesPageScaleGesture() const
{
- auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame);
+ auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame.get());
return pluginView && pluginView->handlesPageScaleFactor();
}
bool WebFrame::requiresUnifiedScaleFactor() const
{
- auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame);
+ auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame.get());
return pluginView && pluginView->requiresUnifiedScaleFactor();
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h (269434 => 269435)
--- trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h 2020-11-05 16:46:12 UTC (rev 269434)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h 2020-11-05 16:54:48 UTC (rev 269435)
@@ -78,7 +78,7 @@
WebPage* page() const;
static WebFrame* fromCoreFrame(const WebCore::Frame&);
- WebCore::Frame* coreFrame() const { return m_coreFrame; }
+ WebCore::Frame* coreFrame() const;
FrameInfoData info() const;
WebCore::FrameIdentifier frameID() const { return m_frameID; }
@@ -184,7 +184,7 @@
private:
WebFrame();
- WebCore::Frame* m_coreFrame { nullptr };
+ WeakPtr<WebCore::Frame> m_coreFrame;
uint64_t m_policyListenerID { 0 };
Optional<WebCore::PolicyCheckIdentifier> m_policyIdentifier;
