Closed Bug 1629588 Opened 4 years ago Closed 4 years ago

v2 personalization can trigger multiple instances of PersonalityProviderWorker.js

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 77
Iteration:
77.2 - Apr 20 - May 3
Tracking Status
firefox76 + verified
firefox77 + verified

People

(Reporter: thecount, Assigned: thecount)

References

Details

Attachments

(1 file)

This is a blocker to turning on this feature.

During the cache load processes there is a new object that is setup but doesn't tear down the old worker.

This new object then goes and sets up a new worker.

Pretty sure this is a memory leak of sorts.

The fix should be easy, either make sure, if there is a provider already available, use it and not make a new one.

Or teardown the old one.

I prefer the option to use the old one.

[Tracking Requested - why for this release]: I think we should uplift this if the fix is small, because it impact an experiment we want to do.

I'm working on a patch now.

Assignee: nobody → sdowne
Priority: -- → P1
Iteration: --- → 77.1 - Apr 6 - Apr 19

(In reply to Scott [:thecount] Downe from comment #0)

This is a blocker to turning on this feature.

Please set the dependency for this.

Blocks: 1629931

Hey Ryan,

I'm still waiting on the ticket for the experiment.

Once I have that, I'll update this as a blocker. Bug 1629931 is another one.

To test:

  1. Create a brand new profile
  2. If in beta set devtools.browsertoolbox.fission to true, in nightly it should already be true. If it's true in beta, you can just leave it true.
  3. Have the multi process debugger open.
  4. Set browser.newtabpage.activity-stream.asrouter.devtoolsEnabled to true
  5. Open about:newtab#devtools-ds
  6. Ensure "show_spocs" is checked
  7. Scroll down a bit and click "Enable V2 Personalization"
  8. Scroll up and click "Trigger Idle Daily"
  9. You should now have all the attachments from remote settings downloaded.
  10. Scroll up and click "Expire Cache"
  11. Scroll up and click "Trigger System Tick"
  12. Check the multi process debugger for sources, and look for PersonalityProviderWorker.js.
  13. Repeat steps 11 and 12.

Expected: Should see only 1 instance of PersonalityProviderWorker.js.

Actual: You can see a new version of PersonalityProviderWorker.js each time you do step 12.

Blocks: 1629999
Iteration: 77.1 - Apr 6 - Apr 19 → 77.2 - Apr 20 - May 3
Pushed by sdowne@getpocket.com:
https://hg.mozilla.org/integration/autoland/rev/8be654ca8d93
Ensure we don't setup a new provider if one already exists. r=gvn

Comment on attachment 9140483 [details]
Bug 1629588 - Ensure we don't setup a new provider if one already exists.

Beta/Release Uplift Approval Request

  • User impact if declined: Performance impact to the users in an experiment.

We would have to not run the experiment, if this patch wasn't so small.

  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: To test:
  1. Have the multi process debugger open.
  2. Create a brand new profile
  3. Set browser.newtabpage.activity-stream.asrouter.devtoolsEnabled to true
  4. Open about:newtab#devtools-ds
  5. Ensure "show_spocs" is checked
  6. Scroll down a bit and click "Enable V2 Personalization"
  7. Scroll up and click "Trigger Idle Daily"
  8. You should now have all the attachments from remote settings downloaded.
  9. Scroll up and click "Expire Cache"
  10. Scroll up and click "Trigger System Tick"
  11. Check the multi process debugger for sources, and look for PersonalityProviderWorker.js.
  12. Repeat steps 9 and 10.

Expected: Should see only 1 instance of PersonalityProviderWorker.js.

Actual: You can see a new version of PersonalityProviderWorker.js each time you do step 12.

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's really small and behind a pref, the impact to this is roughly 1% in an experiment.
  • String changes made/needed: None
Attachment #9140483 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 77
QA Whiteboard: [qa-triaged]

I have verified that this issue is no longer reproducible with the latest Firefox Nightly (77.0a1 Build ID - 20200421094220) installed, on Ubuntu 18.04 x64 and Mac 10.15.3 using the steps from here and I can confirm that only one instance of "PersonalityProviderWorker.js" is displayed. However, I was unable to test this issue on Windows 10 because the "Multiprocess Browser Toolbox" does not work on Windows due to bug 1611945.
Scott, do you think that there is another method to test this issue on Windows?

Flags: needinfo?(sdowne)

Hey Marius, I cannot reproduce 1611945 yet, but one thing that comes to mind.

Is it still an issue if you use --jsdebugger in the target input box for the windows shortcut? and launch from that shortcut?

Flags: needinfo?(sdowne)
Attachment #9140483 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I have verified that this issue is no longer reproducible with the latest Firefox Nightly (77.0a1 Build ID - 20200426215109) and latest Firefox Beta (76.0b8 Build ID - 20200424000239) installed, on Ubuntu 18.04 x64 and Mac 10.15.4 using the steps from here and I can confirm that only one instance of "PersonalityProviderWorker.js" is displayed.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: