Closed Bug 1579480 Opened 3 months ago Closed 3 months ago

newtab DS cards not removing idle callback

Categories

(Firefox :: New Tab Page, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 71
Iteration:
71.1 - Sept 2 - 15
Tracking Status
firefox70 + verified
firefox71 + verified

People

(Reporter: thecount, Assigned: thecount)

References

(Blocks 1 open bug)

Details

(Keywords: github-merged)

Attachments

(2 files)

In order to trigger this you need to open a tab and get it to unmount before the idle callback happens.

In those cases, it's creating a memory leak.

Assignee: nobody → sdowne

[Tracking Requested - why for this release]: There is a small potential memory leak caused by https://bugzilla.mozilla.org/show_bug.cgi?id=1576223 which is in 70, so we probably want an uplift for this.

Priority: -- → P1
Blocks: 1581195
Status: NEW → RESOLVED
Iteration: --- → 71.1 - Sept 2 - 15
Closed: 3 months ago
Keywords: github-merged
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

I have verified this issue with the steps provided in Comment 2 on Latest Firefox Nightly 71.0a1 (Build ID: 20190915214245) on Windows 10 x64, Mac 10.14.5 and Arch Linux 4.16.6 x64.
No error about setState on unmounted dscard are displayed in the console when toggling on and off the browser.newtabpage.activity-stream.discoverystream.enabled preference

Status: RESOLVED → VERIFIED

Comment on attachment 9093637 [details]
Bug 1579480 - uplift newtab DS cards not removing idle callback

Beta/Release Uplift Approval Request

  • User impact if declined: Potential memory leak, but otherwise an error message.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Managed to get better steps to test.
  1. Set browser.newtabpage.activity-stream.debug to true
  2. Open a new tab
  3. Open about:config
  4. Toggle browser.newtabpage.activity-stream.discoverystream.enabled on and off, a few times.
  5. Check console.

Expected, no errors about setState on unmounted dscard. "Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method."

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Verified on nightly, small change, and has unit tests.
  • String changes made/needed: none
Attachment #9093637 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9093637 [details]
Bug 1579480 - uplift newtab DS cards not removing idle callback

Fixes a possible memory leak. Approved for 70.0b8.

Attachment #9093637 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Comment on attachment 9093637 [details]
Bug 1579480 - uplift newtab DS cards not removing idle callback

Whoops, missed that this was still waiting on r+ in Phabricator.

Attachment #9093637 - Flags: approval-mozilla-beta+ → approval-mozilla-beta?

Scott, can you try to find a reviewer in the next couple of days so this can make it into beta 10 on Thursday? Thanks!

Flags: needinfo?(sdowne)

Comment on attachment 9093637 [details]
Bug 1579480 - uplift newtab DS cards not removing idle callback

Fix for memory leak, OK for uplift for beta 10.

Attachment #9093637 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Should this be landed on integration too ?

(In reply to Andreea Pavel [:apavel] from comment #11)

Should this be landed on integration too ?

No, that landed in more extensive way in comment 3.

Flags: needinfo?(sdowne)

I have verified this issue with the steps provided in Comment 6 on Latest Firefox Beta 70.0b10 (Build ID: 20190926005616) on Windows 10 x64, Mac 10.14.6 and Arch Linux 4.16.6 x64.
No error about setState on unmounted dscard are displayed in the console when toggling on and off the browser.newtabpage.activity-stream.discoverystream.enabled preference.

You need to log in before you can comment on or make changes to this bug.