738167 - [New Tab Page] Links keep newtab-site elements alive

Status: RESOLVED INVALID

(Firefox :: General, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

When pinning links or creating sites and directly copying those object to JSM structures, these can keep newtab-site divs alive.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

I think this is the cause of bug 723832. I couldn't reproduce the leak locally but the signature looks like this could be the culprit.

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 608266 [details] [diff] [review]
patch v1

>   finish: function () {
>     function cleanupAndFinish() {
>+      cw = cells = null;

shouldn't be needed

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Dão Gottwald [:dao] from comment #2)
> >     function cleanupAndFinish() {
> >+      cw = cells = null;
> 
> shouldn't be needed

Yeah, probably not, at least not directly for this bug. I noticed it fixed one of the document leaks that were reported when running the newtab test suite locally - but as these reports aren't too accurate anyway I guess we can leave this out.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

Removed the head.js change.

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 608271 [details] [diff] [review]
patch v2

> function Site(aNode, aLink) {
>   this._node = aNode;
>   this._node._newtabSite = this;
> 
>-  this._link = aLink;
>+  // Create a copy of the link so that it doesn't keep us alive.
>+  this._link = {title: aLink.title, url: aLink.url};
> 
>   this._render();
>   this._addEventHandlers();
> }

What's keeping the Site instances alive?

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Dão Gottwald [:dao] from comment #5)
> What's keeping the Site instances alive?

So I did some more investigations and turns out this bug is nonsense. Links don't really keep newtab-site divs alive.