1691861 - Simplify IconLoader code

Status: RESOLVED FIXED

(Core :: Widget, task, P3)

Description

[Markus Stange [:mstange]]

There's some code around mozilla::widget::IconLoader that's a little more complicated than it needs to be; there's a loader and a helper and a bunch of cycle collection.

In this bug I'm trying to remove the helper object and to stop using cycle collection for most of these objects by turning some owning references into non-owning references, so that there are fewer cycles.

Comment 1

[Markus Stange [:mstange]]

https://treeherder.mozilla.org/jobs?repo=try&revision=dea76b31397b9f9c35b5b4a832b5dc3b2cbdf7bf

Comment 2

[Markus Stange [:mstange]]

https://treeherder.mozilla.org/jobs?repo=try&revision=cb14db891630daf5f4ad64aaa540e974d923c736

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Unified build fixes. r=mconley

Comment 4

[Markus Stange [:mstange]]

Attachment: 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.

Depends on D104619

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Make the mHelper field of IconLoader a non-owning reference, and make sure it's always cleared with a call to Destroy before the helper goes away. r=mconley

This breaks another reference cycle.

Depends on D104620

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Make IconLoader not cycle-collected. r=mconley

It no longer needs to be cycle-collected because it doesn't have any strong
references to other cycle-collected objects anymore.

Depends on D104621

Comment 7

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Make the mLoadListener field of IconLoaderHelperCocoa a weak pointer. r=mconley

Depends on D104622

Comment 8

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Make the mLoadListener field of IconLoaderHelperWin a weak pointer. r=mconley

This breaks another strong reference cycle.

Depends on D104623

Comment 9

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Make IconLoaderHelperWin not cycle-collected. r=mconley

Depends on D104624

Comment 10

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - IconLoader::Helper no longer needs to inherit from nsISupports, use inline refcounting for each helper subclass instead. r=mconley

Depends on D104625

Comment 11

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Rename IconLoader::Helper to IconLoader::Listener. r=mconley

Depends on D104626

Comment 12

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Stop using IconLoaderHelperWin and remove it. r=mconley

Depends on D104627

Comment 13

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Move IconLoaderHelperCocoa functionality into a MOZIconHelper obj-c class. r=spohl

Depends on D104628

Comment 14

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Stop using IconLoaderHelperCocoa in nsMenuItemIconX. r=mconley

Depends on D104629

Comment 15

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Stop using IconLoaderHelperCocoa in nsTouchBarInputIcon. r=mconley

Depends on D104630

Comment 16

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Remove IconLoaderHelperCocoa. r=mconley

Depends on D104631

Comment 17

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Move image region handling out of IconLoader. r=mconley

On Windows we were passing an empty rect to signal "entire image", and on macOS we can move the logic into MOZIconHelper.

Depends on D104632

Comment 18

[Markus Stange [:mstange]]

Attachment: Bug 1691861 - Remove unused mContentType fields. r=mconley

This code seems to have been broken quite some time ago, but without adverse effect.
I have filed bug 1691868 so that a proper fix can be found.

In the meantime, this preserves the current behavior and is less confusing.

Depends on D104633

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c05e66763e2
Unified build fixes. r=mconley
https://hg.mozilla.org/integration/autoland/rev/b26ff99675eb
Remove IconLoader's mContent field, and pass the node to LoadURI instead. r=mconley
https://hg.mozilla.org/integration/autoland/rev/ae78f34ef0c3
Make the mHelper field of IconLoader a non-owning reference, and make sure it's always cleared with a call to Destroy before the helper goes away. r=mconley
https://hg.mozilla.org/integration/autoland/rev/7446edbd382c
Make IconLoader not cycle-collected. r=mconley
https://hg.mozilla.org/integration/autoland/rev/bcc29a743195
Make the mLoadListener field of IconLoaderHelperCocoa a weak pointer. r=mconley
https://hg.mozilla.org/integration/autoland/rev/84ba3baf8242
Make the mLoadListener field of IconLoaderHelperWin a weak pointer. r=mconley
https://hg.mozilla.org/integration/autoland/rev/c568b1b4d5a1
Make IconLoaderHelperWin not cycle-collected. r=mconley
https://hg.mozilla.org/integration/autoland/rev/ce0f21bd123b
IconLoader::Helper no longer needs to inherit from nsISupports, use inline refcounting for each helper subclass instead. r=mconley
https://hg.mozilla.org/integration/autoland/rev/aa072284a77b
Rename IconLoader::Helper to IconLoader::Listener. r=mconley
https://hg.mozilla.org/integration/autoland/rev/b5706477c6d2
Stop using IconLoaderHelperWin and remove it. r=mconley
https://hg.mozilla.org/integration/autoland/rev/ff63868de6d0
Move IconLoaderHelperCocoa functionality into a MOZIconHelper obj-c class. r=spohl
https://hg.mozilla.org/integration/autoland/rev/5603767d8f01
Stop using IconLoaderHelperCocoa in nsMenuItemIconX. r=mconley
https://hg.mozilla.org/integration/autoland/rev/30c26e68d0a5
Stop using IconLoaderHelperCocoa in nsTouchBarInputIcon. r=mconley
https://hg.mozilla.org/integration/autoland/rev/e300fc03ec3e
Remove IconLoaderHelperCocoa. r=mconley
https://hg.mozilla.org/integration/autoland/rev/0eddb7e6a636
Move image region handling out of IconLoader. r=mconley
https://hg.mozilla.org/integration/autoland/rev/bde601066f66
Remove unused mContentType fields. r=mconley

Comment 20

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/4c05e66763e2
https://hg.mozilla.org/mozilla-central/rev/b26ff99675eb
https://hg.mozilla.org/mozilla-central/rev/ae78f34ef0c3
https://hg.mozilla.org/mozilla-central/rev/7446edbd382c
https://hg.mozilla.org/mozilla-central/rev/bcc29a743195
https://hg.mozilla.org/mozilla-central/rev/84ba3baf8242
https://hg.mozilla.org/mozilla-central/rev/c568b1b4d5a1
https://hg.mozilla.org/mozilla-central/rev/ce0f21bd123b
https://hg.mozilla.org/mozilla-central/rev/aa072284a77b
https://hg.mozilla.org/mozilla-central/rev/b5706477c6d2
https://hg.mozilla.org/mozilla-central/rev/ff63868de6d0
https://hg.mozilla.org/mozilla-central/rev/5603767d8f01
https://hg.mozilla.org/mozilla-central/rev/30c26e68d0a5
https://hg.mozilla.org/mozilla-central/rev/e300fc03ec3e
https://hg.mozilla.org/mozilla-central/rev/0eddb7e6a636
https://hg.mozilla.org/mozilla-central/rev/bde601066f66