Bug 1559243 - [AS only] After disabling Pocket section in via about:prefererences, Pocket section can't be re-enabled until restart. r=pdahiya, a=jcristau
authorEd Lee <edilee@mozilla.com>
Wed, 19 Jun 2019 02:00:39 +0000
changeset 537025 07204fabaf183f107167fa931e828827dc389458
parent 537024 c828ca470555167119b752f31be44e0bde92a17c
child 537026 3d0f2d47703954d9c19b8044e0ebc6e74c8bcfec
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspdahiya, jcristau
bugs1559243
milestone68.0
Bug 1559243 - [AS only] After disabling Pocket section in via about:prefererences, Pocket section can't be re-enabled until restart. r=pdahiya, a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D35294
browser/components/newtab/lib/TopStoriesFeed.jsm
browser/components/newtab/test/unit/lib/TopStoriesFeed.test.js
--- a/browser/components/newtab/lib/TopStoriesFeed.jsm
+++ b/browser/components/newtab/lib/TopStoriesFeed.jsm
@@ -31,17 +31,17 @@ const REC_IMPRESSION_TRACKING_PREF = "fe
 const OPTIONS_PREF = "feeds.section.topstories.options";
 const MAX_LIFETIME_CAP = 500; // Guard against misconfiguration on the server
 const DISCOVERY_STREAM_PREF = "discoverystream.config";
 
 this.TopStoriesFeed = class TopStoriesFeed {
   constructor(ds) {
     // Use discoverystream config pref default values for fast path and
     // if needed lazy load activity stream top stories feed based on
-    // actual user preference when PREFS_INITIAL_VALUES and PREF_CHANGED is invoked
+    // actual user preference when INIT and PREF_CHANGED is invoked
     this.discoveryStreamEnabled = ds && ds.value && JSON.parse(ds.value).enabled;
     if (!this.discoveryStreamEnabled) {
       this.initializeProperties();
     }
   }
 
   initializeProperties() {
     this.contentUpdateQueue = [];
@@ -660,18 +660,23 @@ this.TopStoriesFeed = class TopStoriesFe
         };
       }
       return true;
     }
     return false;
   }
 
   lazyLoadTopStories(dsPref) {
+    let _dsPref = dsPref;
+    if (!_dsPref) {
+      _dsPref = this.store.getState().Prefs.values[DISCOVERY_STREAM_PREF];
+    }
+
     try {
-      this.discoveryStreamEnabled = JSON.parse(dsPref).enabled;
+      this.discoveryStreamEnabled = JSON.parse(_dsPref).enabled;
     } catch (e) {
       // Load activity stream top stories if fail to determine discovery stream state
       this.discoveryStreamEnabled = false;
     }
 
     // Return without invoking initialization if top stories are loaded
     if (this.storiesLoaded) {
       return;
@@ -680,18 +685,18 @@ this.TopStoriesFeed = class TopStoriesFe
     if (!this.discoveryStreamEnabled && !this.propertiesInitialized) {
       this.initializeProperties();
     }
     this.init();
   }
 
   handleDisabled(action) {
     switch (action.type) {
-      case at.PREFS_INITIAL_VALUES:
-        this.lazyLoadTopStories(action.data[DISCOVERY_STREAM_PREF]);
+      case at.INIT:
+        this.lazyLoadTopStories();
         break;
       case at.PREF_CHANGED:
         if (action.data.name === DISCOVERY_STREAM_PREF) {
           this.lazyLoadTopStories(action.data.value);
         }
         break;
       case at.UNINIT:
         this.uninit();
@@ -700,21 +705,19 @@ this.TopStoriesFeed = class TopStoriesFe
   }
 
   async onAction(action) {
     if (this.discoveryStreamEnabled) {
       this.handleDisabled(action);
       return;
     }
     switch (action.type) {
-      // Check for pref initial values to lazy load activity stream top stories
-      // Here we are not using usual INIT and relying on PREFS_INITIAL_VALUES
-      // to check discoverystream pref and load activity stream top stories only if needed.
-      case at.PREFS_INITIAL_VALUES:
-        this.lazyLoadTopStories(action.data[DISCOVERY_STREAM_PREF]);
+      // Check discoverystream pref and load activity stream top stories only if needed
+      case at.INIT:
+        this.lazyLoadTopStories();
         break;
       case at.SYSTEM_TICK:
         let stories;
         let topics;
         if (Date.now() - this.storiesLastUpdated >= STORIES_UPDATE_TIME) {
           stories = await this.fetchStories();
         }
         if (Date.now() - this.topicsLastUpdated >= TOPICS_UPDATE_TIME) {
--- a/browser/components/newtab/test/unit/lib/TopStoriesFeed.test.js
+++ b/browser/components/newtab/test/unit/lib/TopStoriesFeed.test.js
@@ -93,21 +93,24 @@ describe("Top Stories Feed", () => {
   });
 
   describe("#lazyloading TopStories", () => {
     beforeEach(() => {
       instance.discoveryStreamEnabled = true;
     });
     it("should bind parseOptions to SectionsManager.onceInitialized when discovery stream is true", () => {
       instance.discoveryStreamEnabled = false;
-      instance.onAction({type: at.PREFS_INITIAL_VALUES, data: {"discoverystream.config": JSON.stringify({enabled: true})}});
+      instance.store.getState = () => ({Prefs: {values: {"discoverystream.config": JSON.stringify({enabled: true})}}});
+      instance.onAction({type: at.INIT, data: {}});
+
       assert.calledOnce(sectionsManagerStub.onceInitialized);
     });
     it("should bind parseOptions to SectionsManager.onceInitialized when discovery stream is false", () => {
-      instance.onAction({type: at.PREFS_INITIAL_VALUES, data: {"discoverystream.config": JSON.stringify({enabled: false})}});
+      instance.store.getState = () => ({Prefs: {values: {"discoverystream.config": JSON.stringify({enabled: false})}}});
+      instance.onAction({type: at.INIT, data: {}});
       assert.calledOnce(sectionsManagerStub.onceInitialized);
     });
     it("Should initialize properties once while lazy loading if not initialized earlier", () => {
       instance.discoveryStreamEnabled = false;
       instance.propertiesInitialized = false;
       sinon.stub(instance, "initializeProperties");
       instance.lazyLoadTopStories();
       assert.calledOnce(instance.initializeProperties);
@@ -132,27 +135,29 @@ describe("Top Stories Feed", () => {
       sinon.stub(instance, "doContentUpdate");
       await instance.onInit();
       assert.calledOnce(instance.doContentUpdate);
       assert.isTrue(instance.storiesLoaded);
     });
     it("should handle limited actions when discoverystream is enabled", async () => {
       sinon.spy(instance, "handleDisabled");
       sinon.stub(instance, "getPocketState");
-      instance.onAction({type: at.PREFS_INITIAL_VALUES, data: {"discoverystream.config": JSON.stringify({enabled: true})}});
+      instance.store.getState = () => ({Prefs: {values: {"discoverystream.config": JSON.stringify({enabled: true})}}});
+      instance.onAction({type: at.INIT, data: {}});
+
       assert.calledOnce(instance.handleDisabled);
-
       instance.onAction({type: at.NEW_TAB_REHYDRATED, meta: {fromTarget: {}}});
       assert.notCalled(instance.getPocketState);
     });
     it("should handle NEW_TAB_REHYDRATED when discoverystream is disabled", async () => {
       instance.discoveryStreamEnabled = false;
       sinon.spy(instance, "handleDisabled");
       sinon.stub(instance, "getPocketState");
-      instance.onAction({type: at.PREFS_INITIAL_VALUES, data: {"discoverystream.config": JSON.stringify({enabled: false})}});
+      instance.store.getState = () => ({Prefs: {values: {"discoverystream.config": JSON.stringify({enabled: false})}}});
+      instance.onAction({type: at.INIT, data: {}});
       assert.notCalled(instance.handleDisabled);
 
       instance.onAction({type: at.NEW_TAB_REHYDRATED, meta: {fromTarget: {}}});
       assert.calledOnce(instance.getPocketState);
     });
     it("should handle UNINIT when discoverystream is enabled", async () => {
       sinon.stub(instance, "uninit");
       instance.onAction({type: at.UNINIT});
@@ -188,33 +193,33 @@ describe("Top Stories Feed", () => {
     });
   });
 
   describe("#init", () => {
     it("should create a TopStoriesFeed", () => {
       assert.instanceOf(instance, TopStoriesFeed);
     });
     it("should bind parseOptions to SectionsManager.onceInitialized", () => {
-      instance.onAction({type: at.PREFS_INITIAL_VALUES, data: {}});
+      instance.onAction({type: at.INIT, data: {}});
       assert.calledOnce(sectionsManagerStub.onceInitialized);
     });
     it("should initialize endpoints based on options", async () => {
       await instance.onInit();
       assert.equal("https://somedomain.org/stories?key=test-api-key", instance.stories_endpoint);
       assert.equal("https://somedomain.org/referrer", instance.stories_referrer);
       assert.equal("https://somedomain.org/topics?key=test-api-key", instance.topics_endpoint);
     });
     it("should enable its section", () => {
-      instance.onAction({type: at.PREFS_INITIAL_VALUES, data: {}});
+      instance.onAction({type: at.INIT, data: {}});
       assert.calledOnce(sectionsManagerStub.enableSection);
       assert.calledWith(sectionsManagerStub.enableSection, SECTION_ID);
     });
     it("init should fire onInit", () => {
       instance.onInit = sinon.spy();
-      instance.onAction({type: at.PREFS_INITIAL_VALUES, data: {}});
+      instance.onAction({type: at.INIT, data: {}});
       assert.calledOnce(instance.onInit);
     });
     it("should fetch stories on init", async () => {
       instance.fetchStories = sinon.spy();
       await instance.onInit();
       assert.calledOnce(instance.fetchStories);
     });
     it("should fetch topics on init", async () => {