Skip to content
This repository has been archived by the owner on Apr 1, 2019. It is now read-only.

Commit

Permalink
Addressing review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
Marina Samuel committed Nov 25, 2015
1 parent 268dae0 commit c942dbb
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 52 deletions.
32 changes: 2 additions & 30 deletions src/js/ProviderManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,43 +13,15 @@ const ProviderManager = {

_initialized: false,

get initialized() {
return this._initialized;
},

set initialized(initialized) {
this._initialized = initialized;
},

init: async(function*(requestURL) {
if (!ProviderManager.initialized) {
if (!ProviderManager._initialized) {
yield DirectoryLinksProvider.init(requestURL);
Links.addProvider(PlacesProvider);
Links.addProvider(DirectoryLinksProvider);
yield gUserDatabase.init({"pinnedLinks": [], "blockedLinks": []});
yield gPinnedLinks.init();
yield gBlockedLinks.init();
ProviderManager.initialized = true;
ProviderManager._initialized = true;
}
}),

/**
* Extract a "site" from a url in a way that multiple urls of a "site" returns
* the same "site."
*
* @param {String} url Url spec string
* @return {String} The "site" string
*/
extractSite(url) {
var host = "";
try {
// Note that URL interface might throw for non-standard urls.
host = new URL(url).host;
} catch (ex) {
return "";
}

// Strip off common subdomains of the same site (e.g., www, load balancer)
return host.replace(/^(m|mobile|www\d*)\./, "");
},
};
16 changes: 8 additions & 8 deletions src/js/directoryLinksProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
/*jshint worker:true*/
/*globals CacheTasks, async, ProviderManager, Links*/
/*globals CacheTasks, async, Links*/
"use strict";

// The const that tells where to obtain directory links
const PREF_DIRECTORY_SOURCE = "https://tiles.services.mozilla.com/v3/links/fetch/en-US/nightly";
(function(exports) {
// The const that tells where to obtain directory links
const PREF_DIRECTORY_SOURCE = "https://tiles.services.mozilla.com/v3/links/fetch/en-US/nightly";

// The frecency of a directory link
const DIRECTORY_FRECENCY = 1000;
// The frecency of a directory link
const DIRECTORY_FRECENCY = 1000;

(function(exports) {
/**
* Emits notifications to PlacesProvider and Links
*/
Expand Down Expand Up @@ -43,16 +43,16 @@ const DIRECTORY_FRECENCY = 1000;
getEnhancedLink(link) {
// Use the provided link if it's already enhanced
return link.enhancedImageURI && link ? link :
this._enhancedLinks.get(ProviderManager.extractSite(link.url));
this._enhancedLinks.get(Links.extractSite(link.url));
},

_cacheSuggestedLinks(link) {
// Don't cache links that don't have the expected 'frecent_sites'
//jscs:disable requireCamelCaseOrUpperCaseIdentifiers
if (!link.hasOwnProperty("frecent_sites")) {
return;
}

//jscs:disable requireCamelCaseOrUpperCaseIdentifiers
for (let suggestedSite of link.frecent_sites) {
let suggestedMap = this._suggestedLinks.get(suggestedSite) || new Map();
suggestedMap.set(link.url, link);
Expand Down
46 changes: 33 additions & 13 deletions src/js/links.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* lastVisitDate: 1394678824766431,
* }
*/
/*globals gPinnedLinks, gBlockedLinks, ProviderManager, DirectoryLinksProvider*/
/*globals gPinnedLinks, gBlockedLinks, DirectoryLinksProvider*/
/*jshint worker:true*/

"use strict";
Expand Down Expand Up @@ -83,20 +83,20 @@
let pinnedLinks = gPinnedLinks.links.slice();
let links = this._getMergedProviderLinks();

let sites = pinnedLinks
// Create sitesMap to contain all the links at each base domain.
let sitesMap = pinnedLinks.concat(links)
.filter(link => link)
.map(link => ProviderManager.extractSite(link.url))
.reduce((set, site) => set.add(site), new Set());
.reduce((map, link) => {
let site = this.extractSite(link.url);
return map.get(site) ? map : map.set(site, link);
}, new Map());

// Filter blocked and pinned links and duplicate base domains.
links = links.filter(function(link) {
let site = ProviderManager.extractSite(link.url);
if (!site || sites.has(site)) {
return false;
}
sites.add(site);

return !gBlockedLinks.isBlocked(link) && !gPinnedLinks.isPinned(link);
let site = Links.extractSite(link.url);
return !gBlockedLinks.isBlocked(link) &&
!gPinnedLinks.isPinned(link) &&
(site && (sitesMap.get(site).url === link.url));
});

// Try to fill the gaps between pinned links.
Expand All @@ -113,7 +113,7 @@

pinnedLinks.forEach(link => {
if (link) {
link.baseDomain = ProviderManager.extractSite(link.url);
link.baseDomain = this.extractSite(link.url);
}
});

Expand All @@ -132,7 +132,7 @@
// Don't count blocked URLs.
return;
}
let site = ProviderManager.extractSite(link.url);
let site = this.extractSite(link.url);
map.set(site, (map.get(site) || 0) + 1);
},

Expand Down Expand Up @@ -190,6 +190,26 @@
this._providers.set(aProvider, {});
aProvider.addObserver(this);
},

/**
* Extract a "site" from a url in a way that multiple urls of a "site" returns
* the same "site."
*
* @param {String} url Url spec string
* @return {String} The "site" string
*/
extractSite(url) {
var host = "";
try {
// Note that URL interface might throw for non-standard urls.
host = new URL(url).host;
} catch (ex) {
return "";
}

// Strip off common subdomains of the same site (e.g., www, load balancer)
return host.replace(/^(m|mobile|www\d*)\./, "");
},
};
exports.Links = Links;
}(self));
2 changes: 1 addition & 1 deletion src/test/lib/cachetasks_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var tRequest = testType(new Request(""));
describe("CacheTasks", function() {
describe("deleteCaches() method", () => {
it("should delete all caches when no argument is given.", async(function*() {
var cacheNames = [CACHENAME, "foo", "bar", "baz", "bing", "skeleton_cache", "ads_cache"];
var cacheNames = [CACHENAME, "foo", "bar", "baz", "bing"].concat(yield caches.keys());
for (var name of cacheNames) {
yield caches.open(name);
}
Expand Down

0 comments on commit c942dbb

Please sign in to comment.