v2 personalization can trigger multiple instances of PersonalityProviderWorker.js
Categories
(Firefox :: New Tab Page, defect, P1)
Tracking
()
People
(Reporter: thecount, Assigned: thecount)
References
Details
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details | Review |
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.
Assignee | ||
Comment 1•4 years ago
|
||
[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.
Updated•4 years ago
|
Comment 2•4 years ago
|
||
(In reply to Scott [:thecount] Downe from comment #0)
This is a blocker to turning on this feature.
Please set the dependency for this.
Assignee | ||
Comment 3•4 years ago
|
||
Assignee | ||
Comment 4•4 years ago
•
|
||
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.
Assignee | ||
Comment 5•4 years ago
•
|
||
To test:
- Create a brand new profile
- 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.
- Have the multi process debugger open.
- Set
browser.newtabpage.activity-stream.asrouter.devtoolsEnabled
to true - Open about:newtab#devtools-ds
- Ensure "show_spocs" is checked
- Scroll down a bit and click "Enable V2 Personalization"
- Scroll up and click "Trigger Idle Daily"
- You should now have all the attachments from remote settings downloaded.
- Scroll up and click "Expire Cache"
- Scroll up and click "Trigger System Tick"
- Check the multi process debugger for sources, and look for PersonalityProviderWorker.js.
- 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.
Updated•4 years ago
|
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
Assignee | ||
Comment 7•4 years ago
|
||
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:
- Have the multi process debugger open.
- Create a brand new profile
- Set
browser.newtabpage.activity-stream.asrouter.devtoolsEnabled
to true - Open about:newtab#devtools-ds
- Ensure "show_spocs" is checked
- Scroll down a bit and click "Enable V2 Personalization"
- Scroll up and click "Trigger Idle Daily"
- You should now have all the attachments from remote settings downloaded.
- Scroll up and click "Expire Cache"
- Scroll up and click "Trigger System Tick"
- Check the multi process debugger for sources, and look for PersonalityProviderWorker.js.
- 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
Assignee | ||
Updated•4 years ago
|
Comment 8•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Comment 9•4 years ago
|
||
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?
Assignee | ||
Comment 10•4 years ago
|
||
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?
Updated•4 years ago
|
Comment 11•4 years ago
|
||
bugherder uplift |
Comment 12•4 years ago
|
||
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.
Description
•