will ammend - review comments

mozilla · Apr 25, 2016 · 62e4a63 · 62e4a63
1 parent 6ff41ef
commit 62e4a63
Show file tree

Hide file tree

Showing 9 changed files with 13 additions and 43 deletions.
diff --git a/common/action-manager.js b/common/action-manager.js
@@ -11,8 +11,6 @@ const am = new ActionManager([
   "RECEIVE_BOOKMARKS_CHANGES",
   "RECENT_LINKS_REQUEST",
   "RECENT_LINKS_RESPONSE",
-  "FRECENT_LINKS_REQUEST",
-  "FRECENT_LINKS_RESPONSE",
   "HIGHLIGHTS_LINKS_REQUEST",
   "HIGHLIGHTS_LINKS_RESPONSE",
   "BLOCK_URL",
@@ -33,7 +31,6 @@ am.ACTIONS_WITH_SITES = new Set([
   "TOP_FRECENT_SITES_RESPONSE",
   "RECENT_BOOKMARKS_RESPONSE",
   "RECENT_LINKS_RESPONSE",
-  "FRECENT_LINKS_RESPONSE",
   "HIGHLIGHTS_LINKS_RESPONSE",
 ].map(type => am.type(type)));
 
@@ -102,10 +99,6 @@ function RequestMoreRecentLinks(beforeDate) {
   });
 }
 
-function RequestFrecentLinks() {
-  return RequestExpect("FRECENT_LINKS_REQUEST", "FRECENT_LINKS_RESPONSE");
-}
-
 function RequestHighlightsLinks() {
   return RequestExpect("HIGHLIGHTS_LINKS_REQUEST", "HIGHLIGHTS_LINKS_RESPONSE");
 }
@@ -164,7 +157,6 @@ am.defineActions({
   RequestMoreBookmarks,
   RequestRecentLinks,
   RequestMoreRecentLinks,
-  RequestFrecentLinks,
   RequestHighlightsLinks,
   RequestSearchState,
   BlockUrl,

diff --git a/content-src/components/Base/Base.js b/content-src/components/Base/Base.js
@@ -9,14 +9,12 @@ const Base = React.createClass({
 
     this.props.dispatch(actions.RequestRecentLinks());
 
-    this.props.dispatch(actions.RequestFrecentLinks());
+    this.props.dispatch(actions.RequestHighlightsLinks());
 
     this.props.dispatch(actions.RequestBookmarks());
 
     this.props.dispatch(actions.RequestSearchState());
 
-    this.props.dispatch(actions.RequestHighlightsLinks());
-
     this.props.dispatch(actions.NotifyPerf("BASE_MOUNTED"));
   },
   render() {

diff --git a/content-src/components/DebugPage/DebugPage.js b/content-src/components/DebugPage/DebugPage.js
@@ -28,7 +28,7 @@ const DebugPage = React.createClass({
   getInitialState() {
     return {
       component: "Spotlight",
-      dataSource: "Highlights"
+      dataSource: "Spotlight"
     };
   },
   render() {
@@ -44,7 +44,7 @@ const DebugPage = React.createClass({
           <div className="form-group">
             <label>UI Component</label>
             <select value={this.state.component} onChange={e => this.setState({component: e.target.value})}>
-              <option value={"Spotlight"}>Highlight</option>
+              <option value={"Spotlight"}>Spotlight</option>
               <option value={"TopSites"}>Top Sites</option>
               <option value={"ActivityFeed"}>Activity Feed</option>
             </select>
@@ -62,7 +62,7 @@ const DebugPage = React.createClass({
           {this.state.component === "Spotlight" &&
             <div className="spotlight">
               {selectSpotlight({
-                Highlights: this.props.raw[this.state.dataSource],
+                Spotlight: this.props.raw[this.state.dataSource],
                 Blocked: {urls: new Set()}
               }).rows.map((item, i) => {
                 return (<SpotlightItem key={i} {...item} />);
@@ -92,7 +92,7 @@ module.exports = connect(state => {
       TopSites: state.TopSites,
       History: state.History,
       Bookmarks: state.Bookmarks,
-      Highlights: state.Highlights,
+      Spotlight: state.Spotlight,
     }
   };
 })(DebugPage);

diff --git a/content-src/selectors/selectors.js b/content-src/selectors/selectors.js
@@ -91,12 +91,11 @@ const selectTopSites = createSelector(
 module.exports.selectNewTabSites = createSelector(
   [
     selectTopSites,
-    state => state.Highlights,
     state => state.History,
     selectSpotlight,
     state => state.Blocked
   ],
-  (TopSites, Highlights, History, Spotlight, Blocked) => {
+  (TopSites, History, Spotlight, Blocked) => {
 
     // Remove duplicates
     let [topSitesRows, spotlightRows] = dedupe.group([TopSites.rows.slice(0, TOP_SITES_LENGTH), Spotlight.rows]);
@@ -111,9 +110,9 @@ module.exports.selectNewTabSites = createSelector(
 
     return {
       TopSites: Object.assign({}, TopSites, {rows: topSitesRows}),
-      Spotlight: Object.assign({}, Highlights, {rows: spotlightRows}),
+      Spotlight: Object.assign({}, Spotlight, {rows: spotlightRows}),
       TopActivity: Object.assign({}, History, {rows: topActivityRows}),
-      isReady: TopSites.init && Highlights.init && History.init
+      isReady: TopSites.init && History.init && Spotlight.init
     };
   }
 );

diff --git a/content-test/selectors/selectors.test.js b/content-test/selectors/selectors.test.js
@@ -66,8 +66,8 @@ describe("selectors", () => {
       assert.equal(result.rows[1].url, invalidSite.url);
     }
 
-    it("should have the same properties as FrecentHistory", () => {
-      assert.deepEqual(Object.keys(state), Object.keys(fakeState.FrecentHistory));
+    it("should have the same properties as Highlights", () => {
+      assert.deepEqual(Object.keys(state), Object.keys(fakeState.Highlights));
     });
     it("should add a bestImage for each item", () => {
       state.rows.forEach(site => {
@@ -94,7 +94,7 @@ describe("selectors", () => {
         favicon_colors: [{color: [11, 11, 11]}]
       };
       const results = selectSpotlight({
-        FrecentHistory: {rows: [site]},
+        Highlights: {rows: [site]},
         Blocked: {urls: new Set()}
       });
       assert.equal(results.rows[0].backgroundColor, "#111111");
@@ -107,9 +107,9 @@ describe("selectors", () => {
       });
       assert.ok(results.rows[0].backgroundColor, "should have a bg color");
     });
-    it("should include first run items if FrecentHistory is empty", () => {
+    it("should include first run items if Highlights is empty", () => {
       const results = selectSpotlight({
-        FrecentHistory: {rows: []},
+        Highlights: {rows: []},
         Blocked: {urls: new Set()}
       });
       firstRunData.Highlights.forEach((item, i) => {

diff --git a/lib/ActivityStreams.js b/lib/ActivityStreams.js
@@ -124,11 +124,6 @@ ActivityStreams.prototype = {
           this._processAndSendLinks(links, "RECENT_LINKS_RESPONSE", append, worker);
         });
         break;
-      case am.type("FRECENT_LINKS_REQUEST"):
-        this._memoized.getFrecentLinks(msg.data).then(links => {
-          this._processAndSendLinks(links, "FRECENT_LINKS_RESPONSE", append, worker);
-        });
-        break;
       case am.type("HIGHLIGHTS_LINKS_REQUEST"):
         this._memoized.getHighlightsLinks(msg.data).then(links => {
           this._processAndSendLinks(links, "HIGHLIGHTS_LINKS_RESPONSE", append, worker, true);
@@ -230,7 +225,6 @@ ActivityStreams.prototype = {
 
     this.on(am.type("TOP_FRECENT_SITES_REQUEST"), this._respondToPlacesRequests);
     this.on(am.type("HIGHLIGHTS_LINKS_REQUEST"), this._respondToPlacesRequests);
-    this.on(am.type("FRECENT_LINKS_REQUEST"), this._respondToPlacesRequests);
     this.on(am.type("RECENT_BOOKMARKS_REQUEST"), this._respondToPlacesRequests);
     this.on(am.type("RECENT_LINKS_REQUEST"), this._respondToPlacesRequests);
     this.on(am.type("NOTIFY_HISTORY_DELETE"), this._respondToPlacesRequests);
@@ -254,7 +248,6 @@ ActivityStreams.prototype = {
     SearchProvider.search.off("browser-search-engine-modified", this._handleCurrentEngineChanges);
 
     this.off(am.type("TOP_FRECENT_SITES_REQUEST"), this._respondToPlacesRequests);
-    this.off(am.type("FRECENT_LINKS_REQUEST"), this._respondToPlacesRequests);
     this.off(am.type("RECENT_BOOKMARKS_REQUEST"), this._respondToPlacesRequests);
     this.off(am.type("RECENT_LINKS_REQUEST"), this._respondToPlacesRequests);
     this.off(am.type("NOTIFY_HISTORY_DELETE"), this._respondToPlacesRequests);
@@ -273,7 +266,6 @@ ActivityStreams.prototype = {
       getTopFrecentSites: cache.memoize("getTopFrecentSites", PlacesProvider.links.getTopFrecentSites.bind(linksObj)),
       getRecentBookmarks: cache.memoize("getRecentBookmarks", PlacesProvider.links.getRecentBookmarks.bind(linksObj)),
       getRecentLinks: cache.memoize("getRecentLinks", PlacesProvider.links.getRecentLinks.bind(linksObj)),
-      getFrecentLinks: cache.memoize("getFrecentLinks", PlacesProvider.links.getFrecentLinks.bind(linksObj)),
       getHighlightsLinks: cache.memoize("getHighlightsLinks", PlacesProvider.links.getHighlightsLinks.bind(linksObj)),
       getHistorySize: cache.memoize("getHistorySize", PlacesProvider.links.getHistorySize.bind(linksObj)),
       getBookmarksSize: cache.memoize("getBookmarksSize", PlacesProvider.links.getBookmarksSize.bind(linksObj)),
@@ -293,7 +285,6 @@ ActivityStreams.prototype = {
             this._memoized.getTopFrecentSites(),
             this._memoized.getRecentBookmarks(),
             this._memoized.getRecentLinks(),
-            this._memoized.getFrecentLinks(),
             this._memoized.getHighlightsLinks(),
             this._memoized.getHistorySize(),
             this._memoized.getBookmarksSize(),
@@ -325,10 +316,6 @@ ActivityStreams.prototype = {
         linksToSend.push(...links);
       }));
 
-      promises.push(PlacesProvider.links.getFrecentLinks().then(links => {
-        linksToSend.push(...links);
-      }));
-
       promises.push(PlacesProvider.links.getHighlightsLinks().then(links => {
         linksToSend.push(...links);
       }));
@@ -362,7 +349,6 @@ ActivityStreams.prototype = {
         "getTopFrecentSites",
         "getRecentBookmarks",
         "getRecentLinks",
-        "getFrecentLinks",
         "getHighlightsLinks",
         "getHistorySize",
         "getBookmarksSize",

diff --git a/lib/PerfMeter.js b/lib/PerfMeter.js
@@ -14,8 +14,6 @@ const VALID_TELEMETRY_TAGS = new Set([
   "RECENT_BOOKMARKS_RESPONSE",
   "RECENT_LINKS_REQUEST",
   "RECENT_LINKS_RESPONSE",
-  "FRECENT_LINKS_REQUEST",
-  "FRECENT_LINKS_RESPONSE",
   "SEARCH_STATE_REQUEST",
   "SEARCH_STATE_RESPONSE",
   "NOTIFY_PERFORMANCE",

diff --git a/test/test-ActivityStreams-places-caching.js b/test/test-ActivityStreams-places-caching.js
@@ -34,7 +34,6 @@ let makeNotifsPromise = (cacheStatus) => {
         "getTopFrecentSites-cache",
         "getRecentBookmarks-cache",
         "getRecentLinks-cache",
-        "getFrecentLinks-cache",
         "getHighlightsLinks-cache",
     ]);
     let notifCount = 0;

diff --git a/test/test-PerfMeter.js b/test/test-PerfMeter.js
@@ -64,13 +64,11 @@ function verifyPerfEvents(eventsArray, assert) {
     "TAB_READY",
     "TOP_FRECENT_SITES_REQUEST",
     "RECENT_LINKS_REQUEST",
-    "FRECENT_LINKS_REQUEST",
     "RECENT_BOOKMARKS_REQUEST",
     "SEARCH_STATE_REQUEST",
     "NOTIFY_PERFORMANCE",
     "TOP_FRECENT_SITES_RESPONSE",
     "RECENT_LINKS_RESPONSE",
-    "FRECENT_LINKS_RESPONSE",
     "RECENT_BOOKMARKS_RESPONSE",
     "SEARCH_STATE_RESPONSE",
     "NOTIFY_PERFORMANCE",