Bug 1691861 - Remove IconLoader's mContent field, and pass the node to LoadURI instead. r=mconley

Now IconLoader no longer has an unnecessary strong reference to the node.

Differential Revision: https://phabricator.services.mozilla.com/D104620
This commit is contained in:
Markus Stange 2021-02-12 00:32:01 +00:00
parent eacef4d046
commit f805574e1d
5 changed files with 20 additions and 30 deletions

View File

@ -16,7 +16,7 @@ using namespace mozilla;
namespace mozilla::widget {
NS_IMPL_CYCLE_COLLECTION(mozilla::widget::IconLoader, mContent, mHelper)
NS_IMPL_CYCLE_COLLECTION(mozilla::widget::IconLoader, mHelper)
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(mozilla::widget::IconLoader)
NS_INTERFACE_MAP_ENTRY(imgINotificationObserver)
NS_INTERFACE_MAP_ENTRY(nsISupports)
@ -24,10 +24,8 @@ NS_INTERFACE_MAP_END
NS_IMPL_CYCLE_COLLECTING_ADDREF(mozilla::widget::IconLoader)
NS_IMPL_CYCLE_COLLECTING_RELEASE(mozilla::widget::IconLoader)
IconLoader::IconLoader(Helper* aHelper, nsINode* aContent,
const nsIntRect& aImageRegionRect)
: mContent(aContent),
mContentType(nsIContentPolicy::TYPE_INTERNAL_IMAGE),
IconLoader::IconLoader(Helper* aHelper, const nsIntRect& aImageRegionRect)
: mContentType(nsIContentPolicy::TYPE_INTERNAL_IMAGE),
mImageRegionRect(aImageRegionRect),
mLoadedIcon(false),
mHelper(aHelper) {}
@ -44,7 +42,8 @@ void IconLoader::Destroy() {
}
}
nsresult IconLoader::LoadIcon(nsIURI* aIconURI, bool aIsInternalIcon) {
nsresult IconLoader::LoadIcon(nsIURI* aIconURI, nsINode* aNode,
bool aIsInternalIcon) {
if (mIconRequest) {
// Another icon request is already in flight. Kill it.
mIconRequest->Cancel(NS_BINDING_ABORTED);
@ -53,11 +52,11 @@ nsresult IconLoader::LoadIcon(nsIURI* aIconURI, bool aIsInternalIcon) {
mLoadedIcon = false;
if (!mContent) {
if (!aNode) {
return NS_ERROR_FAILURE;
}
RefPtr<mozilla::dom::Document> document = mContent->OwnerDoc();
RefPtr<mozilla::dom::Document> document = aNode->OwnerDoc();
nsCOMPtr<nsILoadGroup> loadGroup = document->GetDocumentLoadGroup();
if (!loadGroup) {
@ -78,9 +77,9 @@ nsresult IconLoader::LoadIcon(nsIURI* aIconURI, bool aIsInternalIcon) {
getter_AddRefs(mIconRequest));
} else {
rv = loader->LoadImage(
aIconURI, nullptr, nullptr, mContent->NodePrincipal(), 0, loadGroup,
this, mContent, document, nsIRequest::LOAD_NORMAL, nullptr,
mContentType, u""_ns, /* aUseUrgentStartForChannel */ false,
aIconURI, nullptr, nullptr, aNode->NodePrincipal(), 0, loadGroup, this,
aNode, document, nsIRequest::LOAD_NORMAL, nullptr, mContentType, u""_ns,
/* aUseUrgentStartForChannel */ false,
/* aLinkPreload */ false, getter_AddRefs(mIconRequest));
}
if (NS_FAILED(rv)) {

View File

@ -45,8 +45,7 @@ class IconLoader : public imgINotificationObserver {
virtual ~Helper() = default;
};
IconLoader(Helper* aHelper, nsINode* aContent,
const nsIntRect& aImageRegionRect);
IconLoader(Helper* aHelper, const nsIntRect& aImageRegionRect);
public:
NS_DECL_CYCLE_COLLECTING_ISUPPORTS
@ -57,9 +56,8 @@ class IconLoader : public imgINotificationObserver {
// The request may not complete until after LoadIcon returns.
// If aIsInternalIcon is true, the document and principal will not be
// used when loading.
nsresult LoadIcon(nsIURI* aIconURI, bool aIsInternalIcon = false);
void ReleaseJSObjects() { mContent = nullptr; }
nsresult LoadIcon(nsIURI* aIconURI, nsINode* aNode,
bool aIsInternalIcon = false);
void Destroy();
@ -69,7 +67,6 @@ class IconLoader : public imgINotificationObserver {
private:
nsresult OnFrameComplete(imgIRequest* aRequest);
nsCOMPtr<nsINode> mContent;
nsContentPolicyType mContentType;
RefPtr<imgRequestProxy> mIconRequest;
nsIntRect mImageRegionRect;

View File

@ -91,7 +91,7 @@ nsresult nsMenuItemIconX::SetupIcon() {
if (!mIconLoader) {
mIconLoaderHelper = new IconLoaderHelperCocoa(this, kIconSize, kIconSize);
mIconLoader = new IconLoader(mIconLoaderHelper, mContent, mImageRegionRect);
mIconLoader = new IconLoader(mIconLoaderHelper, mImageRegionRect);
if (!mIconLoader) {
return NS_ERROR_OUT_OF_MEMORY;
}
@ -101,7 +101,7 @@ nsresult nsMenuItemIconX::SetupIcon() {
[mNativeMenuItem setImage:mIconLoaderHelper->GetNativeIconImage()];
}
rv = mIconLoader->LoadIcon(iconURI);
rv = mIconLoader->LoadIcon(iconURI, mContent);
if (NS_FAILED(rv)) {
// There is no icon for this menu item, as an error occurred while loading it.
// An icon might have been set earlier or the place holder icon may have

View File

@ -83,7 +83,7 @@ nsresult nsTouchBarInputIcon::SetupIcon(nsCOMPtr<nsIURI> aIconURI) {
// We ask only for the HiDPI images since all Touch Bars are Retina
// displays and we have no need for icons @1x.
mIconLoaderHelper = new IconLoaderHelperCocoa(this, kIconSize, kIconSize, kHiDPIScalingFactor);
mIconLoader = new IconLoader(mIconLoaderHelper, mDocument, mImageRegionRect);
mIconLoader = new IconLoader(mIconLoaderHelper, mImageRegionRect);
if (!mIconLoader) {
return NS_ERROR_OUT_OF_MEMORY;
}
@ -96,7 +96,7 @@ nsresult nsTouchBarInputIcon::SetupIcon(nsCOMPtr<nsIURI> aIconURI) {
[mPopoverItem setCollapsedRepresentationImage:mIconLoaderHelper->GetNativeIconImage()];
}
nsresult rv = mIconLoader->LoadIcon(aIconURI, true /* aIsInternalIcon */);
nsresult rv = mIconLoader->LoadIcon(aIconURI, mDocument, true /* aIsInternalIcon */);
if (NS_FAILED(rv)) {
// There is no icon for this menu item, as an error occurred while loading it.
// An icon might have been set earlier or the place holder icon may have
@ -113,12 +113,7 @@ nsresult nsTouchBarInputIcon::SetupIcon(nsCOMPtr<nsIURI> aIconURI) {
NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT;
}
void nsTouchBarInputIcon::ReleaseJSObjects() {
if (mIconLoader) {
mIconLoader->ReleaseJSObjects();
}
mDocument = nil;
}
void nsTouchBarInputIcon::ReleaseJSObjects() { mDocument = nil; }
//
// mozilla::widget::IconLoaderListenerCocoa

View File

@ -133,10 +133,10 @@ nsresult StatusBarEntry::Init() {
mIconLoaderHelper = new IconLoaderHelperWin(this);
nsIntRect rect;
mIconLoader = new IconLoader(mIconLoaderHelper, mMenu, rect);
mIconLoader = new IconLoader(mIconLoaderHelper, rect);
if (iconURI) {
rv = mIconLoader->LoadIcon(iconURI);
rv = mIconLoader->LoadIcon(iconURI, mMenu);
}
HWND iconWindow;
@ -181,7 +181,6 @@ nsresult StatusBarEntry::OnComplete() {
// with this implementation. We can get rid of the IconLoader and Helper
// at this point, which will also free the allocated HICON.
mIconLoaderHelper->Destroy();
mIconLoader->ReleaseJSObjects();
mIconLoader->Destroy();
mIconLoader = nullptr;
mIconLoaderHelper = nullptr;