Currently we use an "in-memory datastore" to load all of the places results whenever the search app or new tab page loads. I believe the consensus is that we should be using indexedDB store instead. Since we are rapidly approaching the 2.1 FL date, we should do whatever the minimum amount of work necessary to meet all of our requirements are.

Dale - do you know if we have an existing bug anywhere that would track this work? I'm a bit concerned about this one as it seems as it might be tricky to get right and take some iteration. If we want to do this for 2.1, can we make this a priority?

I swear I filed one for candice, but I cant find it now, I will use this for now, but I am not so worried about it, we originally planned for the end of this sprint so going to aim to get a first implementation done for tomorrow

Working on the tests now, partially in combination with https://bugzilla.mozilla.org/show_bug.cgi?id=1049143 so I wanted to have this looked at while I worked on them Francisco this is gonna conflict with your icons patch but trivially at the point we pick up the icon in search.js which has been simplified Definitely not landing prior to the tests, but I think this is in a good enough place to be looked at I didnt make a replacement for places_preload since that was a hack and will be working on its proper replacement next

Comment on attachment 8476220 [details] https://github.com/mozilla-b2g/gaia/pull/23108 Dale this is awesome, thanks for knocking this out. Few things I noticed during this pass: 1 - Failing unit places_test.js unit test. I suppose we should try to keep this working while we get integration tests up, though seems that might be tricky with IDB. 2 - I'm not seeing places results in the search app. I guess we are trying to get these tests running again. Do you know if this should work?

1. Yeh I have the unit test fixed as well as new unit tests for places, thats all sorted and will be up soon 2. Should definitely be working, one thing I have noticed is that the ordering may differ now that we arent pulling them from memory, but ill take a look before I up the 2nd version with tests

Comment on attachment 8476220 [details] https://github.com/mozilla-b2g/gaia/pull/23108 Fixed places.js unit tests and added new tests for the screenshot functionality, plus new tests for places_idb.js

Comment on attachment 8476220 [details] https://github.com/mozilla-b2g/gaia/pull/23108 I think something like this would be fine for now, thanks for knocking it out. Is this the proper algorithm for frecency?

No, firefox proper uses a decaying algorithm for places, this doesnt change the behaviour of the frecency which is the same as we had in the previous browser, since the fxos implementation is vastly different from desktop (one result per host etc) not sure how much of an impact the decay will have, but can look in a follow up

Comment on attachment 8476220 [details] https://github.com/mozilla-b2g/gaia/pull/23108 I saw that you took into account the ugly patch for passing the icon url, don't have anything else to add here. Excellent job!

https://github.com/mozilla-b2g/gaia/commit/fac2363c9dba7d3fc7d1965dbf555abcd36c3980 I took out test/providers/places.js, it was a single test that is much better covered in places_idb_test.js, the way the places test was setup meant it touched too many things and was extremely brittle