Bug 1629588 - Ensure we don't setup a new provider if one already exists. r=gvn
authorScott <scott.downe@gmail.com>
Mon, 20 Apr 2020 18:12:37 +0000
changeset 524905 8be654ca8d934a3a7957ecf2f32a035bdc9e0c9a
parent 524904 8c9a6a498a944f34ac736deeb76d2c80ab2a2518
child 524906 0ff89c708ad7488fd4f156603be42c3009952ee9
push id113469
push usersdowne@getpocket.com
push dateMon, 20 Apr 2020 18:18:27 +0000
treeherderautoland@8be654ca8d93 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgvn
bugs1629588
milestone77.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1629588 - Ensure we don't setup a new provider if one already exists. r=gvn Differential Revision: https://phabricator.services.mozilla.com/D70890
browser/components/newtab/lib/RecommendationProviderSwitcher.jsm
browser/components/newtab/test/unit/lib/RecommendationProviderSwitcher.test.js
--- a/browser/components/newtab/lib/RecommendationProviderSwitcher.jsm
+++ b/browser/components/newtab/lib/RecommendationProviderSwitcher.jsm
@@ -52,16 +52,25 @@ this.RecommendationProviderSwitcher = cl
    *                 new provider.
    * @param {Object} scores This is used to re hydrate the provider based on cached results.
    * @returns {Object} A provider, either a PersonalityProvider or
    *                   UserDomainAffinityProvider.
    */
   setAffinityProvider(...args) {
     const { affinityProviderV2 } = this;
     if (affinityProviderV2 && affinityProviderV2.modelKeys) {
+      // A v2 provider is already set. This can happen when new stories come in
+      // and we need to update their scores.
+      // We can use the existing one, a fresh one is created after startup.
+      // Using the existing one might be a bit out of date,
+      // but it's fine for now. We can rely on restarts for updates.
+      // See bug 1629931 for improvements to this.
+      if (this.affinityProvider) {
+        return;
+      }
       // At this point we've determined we can successfully create a v2 personalization provider.
       this.affinityProvider = new PersonalityProvider(...args, {
         modelKeys: affinityProviderV2.modelKeys,
         dispatch: this.store.dispatch,
       });
       return;
     }
 
--- a/browser/components/newtab/test/unit/lib/RecommendationProviderSwitcher.test.js
+++ b/browser/components/newtab/test/unit/lib/RecommendationProviderSwitcher.test.js
@@ -26,30 +26,56 @@ describe("RecommendationProviderSwitcher
         undefined,
         undefined,
         undefined,
         undefined
       );
 
       assert.equal(feed.affinityProvider.modelKeys, undefined);
 
+      feed.affinityProvider = null;
       feed.affinityProviderV2 = {
         modelKeys: "1234",
       };
 
       feed.setAffinityProvider(
         undefined,
         undefined,
         undefined,
         undefined,
         undefined
       );
 
       assert.equal(feed.affinityProvider.modelKeys, "1234");
     });
+    it("should use old provider", async () => {
+      feed.setAffinityProvider(
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined
+      );
+
+      assert.equal(feed.affinityProvider.modelKeys, undefined);
+
+      feed.affinityProviderV2 = {
+        modelKeys: "1234",
+      };
+
+      feed.setAffinityProvider(
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined
+      );
+
+      assert.equal(feed.affinityProvider.modelKeys, undefined);
+    });
   });
 
   describe("#init", () => {
     it("should init affinityProvider then refreshContent", async () => {
       feed.affinityProvider = {
         init: sandbox.stub().resolves(),
       };
       await feed.init();