Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2025-02-27 21:00:50 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1013051 - Fix "Comparable link missing required property: frecency" error. r=ttaubert

Browse Source
This commit is contained in:
Drew Willcoxon 2014-06-10 10:34:20 -07:00
parent eeea9fdc80
commit bb9fc828cf
2 changed files with 83 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
toolkit/modules/NewTabUtils.jsm
View File

@ -917,58 +917,55 @@ let Links = {
return;
let { sortedLinks, linkMap } = links;
// Nothing to do if the list is full and the link isn't in it and shouldn't
// be in it.
if (!linkMap.has(aLink.url) &&
sortedLinks.length &&
sortedLinks.length == aProvider.maxNumLinks) {
let lastLink = sortedLinks[sortedLinks.length - 1];
if (this.compareLinks(lastLink, aLink) < 0)
return;
}
let existingLink = linkMap.get(aLink.url);
let insertionLink = null;
let updatePages = false;
// Update the title in O(1).
if ("title" in aLink) {
let link = linkMap.get(aLink.url);
if (link && link.title != aLink.title) {
link.title = aLink.title;
if (existingLink) {
// Update our copy's position in O(lg n) by first removing it from its
// list. It's important to do this before modifying its properties.
if (this._sortProperties.some(prop => prop in aLink)) {
let idx = this._indexOf(sortedLinks, existingLink);
if (idx < 0) {
throw new Error("Link should be in _sortedLinks if in _linkMap");
}
sortedLinks.splice(idx, 1);
// Update our copy's properties.
for (let prop of this._sortProperties) {
if (prop in aLink) {
existingLink[prop] = aLink[prop];
}
}
// Finally, reinsert our copy below.
insertionLink = existingLink;
}
// Update our copy's title in O(1).
if ("title" in aLink && aLink.title != existingLink.title) {
existingLink.title = aLink.title;
updatePages = true;
}
}
else if (this._sortProperties.every(prop => prop in aLink)) {
// Before doing the O(lg n) insertion below, do an O(1) check for the
// common case where the new link is too low-ranked to be in the list.
if (sortedLinks.length && sortedLinks.length == aProvider.maxNumLinks) {
let lastLink = sortedLinks[sortedLinks.length - 1];
if (this.compareLinks(lastLink, aLink) < 0) {
return;
}
}
// Copy the link object so that changes later made to it by the caller
// don't affect our copy.
insertionLink = {};
for (let prop in aLink) {
insertionLink[prop] = aLink[prop];
}
linkMap.set(aLink.url, insertionLink);
}
// Update the link's position in O(lg n).
if (this._sortProperties.some((prop) => prop in aLink)) {
let link = linkMap.get(aLink.url);
if (link) {
// The link is already in the list.
let idx = this._indexOf(sortedLinks, link);
if (idx < 0)
throw new Error("Link should be in _sortedLinks if in _linkMap");
sortedLinks.splice(idx, 1);
for (let prop of this._sortProperties) {
if (prop in aLink)
link[prop] = aLink[prop];
}
}
else {
// The link is new.
for (let prop of this._sortProperties) {
if (!(prop in aLink))
throw new Error("New link missing required sort property: " + prop);
}
// Copy the link object so that if the caller changes it, it doesn't
// screw up our bookkeeping.
link = {};
for (let [prop, val] of Iterator(aLink)) {
link[prop] = val;
}
linkMap.set(link.url, link);
}
let idx = this._insertionIndexOf(sortedLinks, link);
sortedLinks.splice(idx, 0, link);
if (insertionLink) {
let idx = this._insertionIndexOf(sortedLinks, insertionLink);
sortedLinks.splice(idx, 0, insertionLink);
if (sortedLinks.length > aProvider.maxNumLinks) {
let lastLink = sortedLinks.pop();
linkMap.delete(lastLink.url);

57
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
View File

@ -51,12 +51,7 @@ add_test(function changeLinks() {
do_check_links(NewTabUtils.links.getLinks(), expectedLinks);
// Notify of a new link.
let newLink = {
url: "http://example.com/19",
title: "My frecency is 19",
frecency: 19,
lastVisitDate: 0,
};
let newLink = makeLink(19);
expectedLinks.splice(1, 0, newLink);
provider.notifyLinkChanged(newLink);
do_check_links(NewTabUtils.links.getLinks(), expectedLinks);
@ -81,11 +76,7 @@ add_test(function changeLinks() {
// Notify of a new link again, but this time make it overflow maxNumLinks.
provider.maxNumLinks = expectedLinks.length;
newLink = {
url: "http://example.com/21",
frecency: 21,
lastVisitDate: 0,
};
newLink = makeLink(21);
expectedLinks.unshift(newLink);
expectedLinks.pop();
do_check_eq(expectedLinks.length, provider.maxNumLinks); // Sanity check.
@ -125,6 +116,34 @@ add_task(function oneProviderAlreadyCached() {
NewTabUtils.links.removeProvider(provider2);
});
add_task(function newLowRankedLink() {
// Init a provider with 10 links and make its maximum number also 10.
let links = makeLinks(0, 10, 1);
let provider = new TestProvider(done => done(links));
provider.maxNumLinks = links.length;
NewTabUtils.initWithoutProviders();
NewTabUtils.links.addProvider(provider);
// This is sync since the provider's getLinks is sync.
NewTabUtils.links.populateCache(function () {}, false);
do_check_links(NewTabUtils.links.getLinks(), links);
// Notify of a new link that's low-ranked enough not to make the list.
let newLink = makeLink(0);
provider.notifyLinkChanged(newLink);
do_check_links(NewTabUtils.links.getLinks(), links);
// Notify about the new link's title change.
provider.notifyLinkChanged({
url: newLink.url,
title: "a new title",
});
do_check_links(NewTabUtils.links.getLinks(), links);
NewTabUtils.links.removeProvider(provider);
});
function TestProvider(getLinksFn) {
this.getLinks = getLinksFn;
this._observers = new Set();
@ -165,12 +184,16 @@ function makeLinks(frecRangeStart, frecRangeEnd, step) {
let links = [];
// Remember, links are ordered by frecency descending.
for (let i = frecRangeEnd; i > frecRangeStart; i -= step) {
links.push({
url: "http://example.com/" + i,
title: "My frecency is " + i,
frecency: i,
lastVisitDate: 0,
});
links.push(makeLink(i));
}
return links;
}
function makeLink(frecency) {
return {
url: "http://example.com/" + frecency,
title: "My frecency is " + frecency,
frecency: frecency,
lastVisitDate: 0,
};
}