Skip to content

Commit

Permalink
Bug 1867945 - Use WeakPtr<Document> instead of nsWeakPtr. r=dom-core,…
Browse files Browse the repository at this point in the history 
…farre

Differential Revision: https://phabricator.services.mozilla.com/D195342
  • Loading branch information
@petervanderbeken
petervanderbeken committed Dec 7, 2023
1 parent 786fbf7 commit 76d5680
Show file tree
Hide file tree
Showing 14 changed files with 58 additions and 103 deletions.
51 changes: 11 additions & 40 deletions dom/base/Document.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14158,9 +14158,7 @@ class FullscreenRoots {
MOZ_COUNTED_DEFAULT_CTOR(FullscreenRoots)
MOZ_COUNTED_DTOR(FullscreenRoots)

enum : uint32_t { NotFound = uint32_t(-1) };
// Looks in mRoots for aRoot. Returns the index if found, otherwise NotFound.
static uint32_t Find(Document* aRoot);
using RootsArray = nsTArray<WeakPtr<Document>>;

// Returns true if aRoot is in the list of fullscreen roots.
static bool Contains(Document* aRoot);
Expand All @@ -14170,7 +14168,7 @@ class FullscreenRoots {
static FullscreenRoots* sInstance;

// List of weak pointers to roots.
nsTArray<nsWeakPtr> mRoots;
RootsArray mRoots;
};

FullscreenRoots* FullscreenRoots::sInstance = nullptr;
Expand All @@ -14182,10 +14180,10 @@ void FullscreenRoots::ForEach(void (*aFunction)(Document* aDoc)) {
}
// Create a copy of the roots array, and iterate over the copy. This is so
// that if an element is removed from mRoots we don't mess up our iteration.
nsTArray<nsWeakPtr> roots(sInstance->mRoots.Clone());
RootsArray roots(sInstance->mRoots.Clone());
// Call aFunction on all entries.
for (uint32_t i = 0; i < roots.Length(); i++) {
nsCOMPtr<Document> root = do_QueryReferent(roots[i]);
nsCOMPtr<Document> root(roots[i]);
// Check that the root isn't in the manager. This is so that new additions
// while we were running don't get traversed.
if (root && FullscreenRoots::Contains(root)) {
Expand All @@ -14196,7 +14194,7 @@ void FullscreenRoots::ForEach(void (*aFunction)(Document* aDoc)) {

/* static */
bool FullscreenRoots::Contains(Document* aRoot) {
return FullscreenRoots::Find(aRoot) != NotFound;
return sInstance && sInstance->mRoots.Contains(aRoot);
}

/* static */
Expand All @@ -14207,36 +14205,18 @@ void FullscreenRoots::Add(Document* aDoc) {
if (!sInstance) {
sInstance = new FullscreenRoots();
}
sInstance->mRoots.AppendElement(do_GetWeakReference(root));
sInstance->mRoots.AppendElement(root);
}
}

/* static */
uint32_t FullscreenRoots::Find(Document* aRoot) {
if (!sInstance) {
return NotFound;
}
nsTArray<nsWeakPtr>& roots = sInstance->mRoots;
for (uint32_t i = 0; i < roots.Length(); i++) {
nsCOMPtr<Document> otherRoot(do_QueryReferent(roots[i]));
if (otherRoot == aRoot) {
return i;
}
}
return NotFound;
}

/* static */
void FullscreenRoots::Remove(Document* aDoc) {
nsCOMPtr<Document> root =
nsContentUtils::GetInProcessSubtreeRootDocument(aDoc);
uint32_t index = Find(root);
NS_ASSERTION(index != NotFound,
"Should only try to remove roots which are still added!");
if (index == NotFound || !sInstance) {
if (!sInstance || !sInstance->mRoots.RemoveElement(root)) {
NS_ERROR("Should only try to remove roots which are still added!");
return;
}
sInstance->mRoots.RemoveElementAt(index);
if (sInstance->mRoots.IsEmpty()) {
delete sInstance;
sInstance = nullptr;
Expand Down Expand Up @@ -14354,11 +14334,6 @@ class PendingFullscreenChangeList {
/* static */
LinkedList<FullscreenChange> PendingFullscreenChangeList::sList;

Document* Document::GetFullscreenRoot() {
nsCOMPtr<Document> root = do_QueryReferent(mFullscreenRoot);
return root;
}

size_t Document::CountFullscreenElements() const {
size_t count = 0;
for (const nsWeakPtr& ptr : mTopLayer) {
Expand All @@ -14371,10 +14346,6 @@ size_t Document::CountFullscreenElements() const {
return count;
}

void Document::SetFullscreenRoot(Document* aRoot) {
mFullscreenRoot = do_GetWeakReference(aRoot);
}

// https://github.com/whatwg/html/issues/9143
// We need to consider the precedence between active modal dialog, topmost auto
// popover and fullscreen element once it's specified.
Expand Down Expand Up @@ -16962,7 +16933,7 @@ class UserInteractionTimer final : public Runnable,
explicit UserInteractionTimer(Document* aDocument)
: Runnable("UserInteractionTimer"),
mPrincipal(aDocument->NodePrincipal()),
mDocument(do_GetWeakReference(aDocument)) {
mDocument(aDocument) {
static int32_t userInteractionTimerId = 0;
// Blocker names must be unique. Let's create it now because when needed,
// the document could be already gone.
Expand Down Expand Up @@ -17039,7 +17010,7 @@ class UserInteractionTimer final : public Runnable,
}

// If the document is not gone, let's reset its timer flag.
nsCOMPtr<Document> document = do_QueryReferent(mDocument);
nsCOMPtr<Document> document(mDocument);
if (document) {
ContentBlockingUserInteraction::Observe(mPrincipal);
document->ResetUserInteractionTimer();
Expand Down Expand Up @@ -17067,7 +17038,7 @@ class UserInteractionTimer final : public Runnable,
}

nsCOMPtr<nsIPrincipal> mPrincipal;
nsWeakPtr mDocument;
WeakPtr<Document> mDocument;

nsCOMPtr<nsITimer> mTimer;

Expand Down
6 changes: 3 additions & 3 deletions dom/base/Document.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1901,15 +1901,15 @@ class Document : public nsINode,
* in the in-process document tree. Returns nullptr if the document isn't
* fullscreen.
*/
Document* GetFullscreenRoot();
Document* GetFullscreenRoot() const { return mFullscreenRoot; }

size_t CountFullscreenElements() const;

/**
* Sets the fullscreen root to aRoot. This stores a weak reference to aRoot
* in this document.
*/
void SetFullscreenRoot(Document* aRoot);
void SetFullscreenRoot(Document* aRoot) { mFullscreenRoot = aRoot; }

/**
* Synchronously cleans up the fullscreen state on the given document.
Expand Down Expand Up @@ -5133,7 +5133,7 @@ class Document : public nsINode,

// The root of the doc tree in which this document is in. This is only
// non-null when this document is in fullscreen mode.
nsWeakPtr mFullscreenRoot;
WeakPtr<Document> mFullscreenRoot;

RefPtr<DOMImplementation> mDOMImplementation;

Expand Down
10 changes: 4 additions & 6 deletions dom/base/Navigator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,10 +680,8 @@ namespace {

class VibrateWindowListener : public nsIDOMEventListener {
public:
VibrateWindowListener(nsPIDOMWindowInner* aWindow, Document* aDocument) {
mWindow = do_GetWeakReference(aWindow);
mDocument = do_GetWeakReference(aDocument);

VibrateWindowListener(nsPIDOMWindowInner* aWindow, Document* aDocument)
: mWindow(do_GetWeakReference(aWindow)), mDocument(aDocument) {
constexpr auto visibilitychange = u"visibilitychange"_ns;
aDocument->AddSystemEventListener(visibilitychange, this, /* listener */
true, /* use capture */
Expand All @@ -699,7 +697,7 @@ class VibrateWindowListener : public nsIDOMEventListener {
virtual ~VibrateWindowListener() = default;

nsWeakPtr mWindow;
nsWeakPtr mDocument;
WeakPtr<Document> mDocument;
};

NS_IMPL_ISUPPORTS(VibrateWindowListener, nsIDOMEventListener)
Expand Down Expand Up @@ -731,7 +729,7 @@ VibrateWindowListener::HandleEvent(Event* aEvent) {
}

void VibrateWindowListener::RemoveListener() {
nsCOMPtr<EventTarget> target = do_QueryReferent(mDocument);
nsCOMPtr<Document> target(mDocument);
if (!target) {
return;
}
Expand Down
13 changes: 6 additions & 7 deletions dom/base/nsGlobalWindowInner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6721,21 +6721,20 @@ void nsGlobalWindowInner::AddSizeOfIncludingThis(
void nsGlobalWindowInner::RegisterDataDocumentForMemoryReporting(
Document* aDocument) {
aDocument->SetAddedToMemoryReportAsDataDocument();
mDataDocumentsForMemoryReporting.AppendElement(
do_GetWeakReference(aDocument));
mDataDocumentsForMemoryReporting.AppendElement(aDocument);
}

void nsGlobalWindowInner::UnregisterDataDocumentForMemoryReporting(
Document* aDocument) {
nsWeakPtr doc = do_GetWeakReference(aDocument);
MOZ_ASSERT(mDataDocumentsForMemoryReporting.Contains(doc));
mDataDocumentsForMemoryReporting.RemoveElement(doc);
DebugOnly<bool> found =
mDataDocumentsForMemoryReporting.RemoveElement(aDocument);
MOZ_ASSERT(found);
}

void nsGlobalWindowInner::CollectDOMSizesForDataDocuments(
nsWindowSizes& aSize) const {
for (const nsWeakPtr& ptr : mDataDocumentsForMemoryReporting) {
if (nsCOMPtr<Document> doc = do_QueryReferent(ptr)) {
for (Document* doc : mDataDocumentsForMemoryReporting) {
if (doc) {
doc->DocAddSizeOfIncludingThis(aSize);
}
}
Expand Down
3 changes: 2 additions & 1 deletion dom/base/nsGlobalWindowInner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

// Local Includes
// Helper Classes
#include "mozilla/WeakPtr.h"
#include "nsCOMPtr.h"
#include "nsWeakReference.h"
#include "nsTHashMap.h"
Expand Down Expand Up @@ -1446,7 +1447,7 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,

nsTArray<uint32_t> mScrollMarks;

nsTArray<nsWeakPtr> mDataDocumentsForMemoryReporting;
nsTArray<mozilla::WeakPtr<Document>> mDataDocumentsForMemoryReporting;

static InnerWindowByIdTable* sInnerWindowsById;

Expand Down
6 changes: 3 additions & 3 deletions dom/websocket/WebSocket.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class WebSocketImpl final : public nsIInterfaceRequestor,
nsCString mURI;
nsCString mRequestedProtocolList;

nsWeakPtr mOriginDocument;
WeakPtr<Document> mOriginDocument;

// Web Socket owner information:
// - the script file name, UTF8 encoded.
Expand Down Expand Up @@ -1643,7 +1643,7 @@ nsresult WebSocketImpl::Init(JSContext* aCx, bool aIsSecure,
rv = mWebSocket->CheckCurrentGlobalCorrectness();
NS_ENSURE_SUCCESS(rv, rv);
}
mOriginDocument = do_GetWeakReference(originDoc);
mOriginDocument = originDoc;

if (!mIsServerSide) {
nsCOMPtr<nsIURI> uri;
Expand Down Expand Up @@ -1883,7 +1883,7 @@ nsresult WebSocketImpl::InitializeConnection(

// manually adding loadinfo to the channel since it
// was not set during channel creation.
nsCOMPtr<Document> doc = do_QueryReferent(mOriginDocument);
nsCOMPtr<Document> doc(mOriginDocument);

// mOriginDocument has to be release on main-thread because WeakReferences
// are not thread-safe.
Expand Down
7 changes: 3 additions & 4 deletions dom/xslt/xpath/XPathEvaluator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,22 @@ class XPathEvaluatorParseContext : public txIParseContext {
bool mIsCaseSensitive;
};

XPathEvaluator::XPathEvaluator(Document* aDocument)
: mDocument(do_GetWeakReference(aDocument)) {}
XPathEvaluator::XPathEvaluator(Document* aDocument) : mDocument(aDocument) {}

XPathEvaluator::~XPathEvaluator() = default;

UniquePtr<XPathExpression> XPathEvaluator::CreateExpression(
const nsAString& aExpression, XPathNSResolver* aResolver,
ErrorResult& aRv) {
nsCOMPtr<Document> doc = do_QueryReferent(mDocument);
nsCOMPtr<Document> doc(mDocument);
XPathEvaluatorParseContext pContext(aResolver,
!(doc && doc->IsHTMLDocument()));
return CreateExpression(aExpression, &pContext, doc, aRv);
}

UniquePtr<XPathExpression> XPathEvaluator::CreateExpression(
const nsAString& aExpression, nsINode* aResolver, ErrorResult& aRv) {
nsCOMPtr<Document> doc = do_QueryReferent(mDocument);
nsCOMPtr<Document> doc(mDocument);
XPathEvaluatorParseContext pContext(aResolver,
!(doc && doc->IsHTMLDocument()));
return CreateExpression(aExpression, &pContext, doc, aRv);
Expand Down
7 changes: 2 additions & 5 deletions dom/xslt/xpath/XPathEvaluator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ class XPathEvaluator final : public NonRefcountedDOMObject {
// WebIDL API
bool WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto,
JS::MutableHandle<JSObject*> aReflector);
Document* GetParentObject() {
nsCOMPtr<Document> doc = do_QueryReferent(mDocument);
return doc;
}
Document* GetParentObject() { return mDocument; }
static UniquePtr<XPathEvaluator> Constructor(const GlobalObject& aGlobal);
UniquePtr<XPathExpression> CreateExpression(const nsAString& aExpression,
XPathNSResolver* aResolver,
Expand All @@ -58,7 +55,7 @@ class XPathEvaluator final : public NonRefcountedDOMObject {
ErrorResult& rv);

private:
nsWeakPtr mDocument;
WeakPtr<Document> mDocument;
RefPtr<txResultRecycler> mRecycler;
};

Expand Down
29 changes: 11 additions & 18 deletions dom/xul/nsXULContentSink.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(XULContentSinkImpl)

NS_IMETHODIMP
XULContentSinkImpl::DidBuildModel(bool aTerminated) {
nsCOMPtr<Document> doc = do_QueryReferent(mDocument);
nsCOMPtr<Document> doc(mDocument);
if (doc) {
mPrototype->NotifyLoadDone();
mDocument = nullptr;
Expand Down Expand Up @@ -200,16 +200,13 @@ XULContentSinkImpl::SetParser(nsParserBase* aParser) {

void XULContentSinkImpl::SetDocumentCharset(
NotNull<const Encoding*> aEncoding) {
nsCOMPtr<Document> doc = do_QueryReferent(mDocument);
nsCOMPtr<Document> doc(mDocument);
if (doc) {
doc->SetDocumentCharacterSet(aEncoding);
}
}

nsISupports* XULContentSinkImpl::GetTarget() {
nsCOMPtr<Document> doc = do_QueryReferent(mDocument);
return ToSupports(doc);
}
nsISupports* XULContentSinkImpl::GetTarget() { return ToSupports(mDocument); }

//----------------------------------------------------------------------

Expand All @@ -218,7 +215,7 @@ nsresult XULContentSinkImpl::Init(Document* aDocument,
MOZ_ASSERT(aDocument != nullptr, "null ptr");
if (!aDocument) return NS_ERROR_NULL_POINTER;

mDocument = do_GetWeakReference(aDocument);
mDocument = aDocument;
mPrototype = aPrototype;

mDocumentURL = mPrototype->GetURI();
Expand Down Expand Up @@ -412,7 +409,7 @@ XULContentSinkImpl::HandleEndElement(const char16_t* aName) {

// If given a src= attribute, we must ignore script tag content.
if (!script->mSrcURI && !script->HasStencil()) {
nsCOMPtr<Document> doc = do_QueryReferent(mDocument);
nsCOMPtr<Document> doc(mDocument);

script->mOutOfLine = false;
if (doc) {
Expand Down Expand Up @@ -548,7 +545,7 @@ XULContentSinkImpl::ReportError(const char16_t* aErrorText,

// return leaving the document empty if we're asked to not add a <parsererror>
// root node
nsCOMPtr<Document> idoc = do_QueryReferent(mDocument);
nsCOMPtr<Document> idoc(mDocument);
if (idoc && idoc->SuppressParserErrorElement()) {
return NS_OK;
};
Expand Down Expand Up @@ -725,7 +722,7 @@ nsresult XULContentSinkImpl::OpenScript(const char16_t** aAttributes,
return NS_OK;
}

nsCOMPtr<Document> doc(do_QueryReferent(mDocument));
nsCOMPtr<Document> doc(mDocument);
nsCOMPtr<nsIScriptGlobalObject> globalObject;
if (doc) globalObject = do_QueryInterface(doc->GetWindow());
RefPtr<nsXULPrototypeScript> script = new nsXULPrototypeScript(aLineNumber);
Expand All @@ -741,14 +738,10 @@ nsresult XULContentSinkImpl::OpenScript(const char16_t** aAttributes,
if (NS_SUCCEEDED(rv)) {
if (!mSecMan)
mSecMan = do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
if (NS_SUCCEEDED(rv)) {
nsCOMPtr<Document> doc = do_QueryReferent(mDocument, &rv);

if (NS_SUCCEEDED(rv)) {
rv = mSecMan->CheckLoadURIWithPrincipal(
doc->NodePrincipal(), script->mSrcURI,
nsIScriptSecurityManager::ALLOW_CHROME, doc->InnerWindowID());
}
if (NS_SUCCEEDED(rv) && doc) {
rv = mSecMan->CheckLoadURIWithPrincipal(
doc->NodePrincipal(), script->mSrcURI,
nsIScriptSecurityManager::ALLOW_CHROME, doc->InnerWindowID());
}
}

Expand Down
Loading

0 comments on commit 76d5680

Please sign in to comment.