Bug 1846781 - Use recalc_frecency for updating origins frecency instead of triggers. r=daisuke

Until now we updated origins frecency using direct SQL triggers.
While that guaranteed good performance, it also had some downsides:
 * replacing the algorithms is complicate, the current system only works
   with a straight sum of page frecencies. We are planning to experiment with
   different algorithms in the future.
 * it requires using multiple temp tables and DELETE triggers, that is error
   prone for consumers, that may forget to DELETE from the temp tables, and thus
   break data coherency.
 * there's not much atomicity, since the origins update must be triggered apart
   and a crash would lose some of the changes

This patch is changing the behavior to be closer to the recalc_frecency one that
is already used for pages.
When a page is added, visited, or removed, recalc_frecency of its origin is set
to 1. Later frecency of invalidated origins will be recalculated in chunks.
While this is surely less efficient than the existing system, it solves the
problems presented above.
A threshold is recalculated at each chunk, and stored in the moz_meta table.
This patch continues using the old STATS in the moz_meta table, to allow for
easier downgrades. Once a new threshold will be introduced we'll be able to
stop updating those.

The after delete temp table is maintained because there's no more efficient way
to remove orphan origins promptly. Thus, after a removal from moz_places,
consumers MUST still DELETE from the temp table to cleanup orphan origins.
This also introduces a delayed removal of orphan origins when their frecency
becomes 0.

Differential Revision: https://phabricator.services.mozilla.com/D186070

browser/components/BrowserGlue.sys.mjs

@@ -3479,7 +3479,19 @@ BrowserGlue.prototype = {
         if (bookmarksUrl) {
           // Import from bookmarks.html file.
           try {
-            if (Services.policies.isAllowed("defaultBookmarks")) {
+            if (
+              Services.policies.isAllowed("defaultBookmarks") &&
+              // Default bookmarks are imported after startup, and they may
+              // influence the outcome of tests, thus it's possible to use
+              // this test-only pref to skip the import.
+              !(
+                Cu.isInAutomation &&
+                Services.prefs.getBoolPref(
+                  "browser.bookmarks.testing.skipDefaultBookmarksImport",
+                  false
+                )
+              )
+            ) {
               await lazy.BookmarkHTMLUtils.importFromURL(bookmarksUrl, {
                 replace: true,
                 source: lazy.PlacesUtils.bookmarks.SOURCES.RESTORE_ON_STARTUP,

browser/components/urlbar/tests/browser-tips/browser.ini

@@ -5,6 +5,8 @@
 [DEFAULT]
 support-files =
   head.js
+prefs=
+  browser.bookmarks.testing.skipDefaultBookmarksImport=true
 
 [browser_interventions.js]
 [browser_picks.js]

browser/components/urlbar/tests/browser-updateResults/browser.ini

@@ -5,6 +5,8 @@
 [DEFAULT]
 support-files =
   head.js
+prefs=
+  browser.bookmarks.testing.skipDefaultBookmarksImport=true
 
 [browser_appendSpanCount.js]
 [browser_noUpdateResultsFromOtherProviders.js]

browser/components/urlbar/tests/browser/browser.ini

@@ -11,6 +11,7 @@ skip-if =
   os == "win" && os_version == "6.1" # Skip on Azure - frequent failure
 
 prefs =
+  browser.bookmarks.testing.skipDefaultBookmarksImport=true
   browser.urlbar.trending.featureGate=false
   extensions.screenshots.disabled=false
   screenshots.browser.component.enabled=true

browser/components/urlbar/tests/browser/browser_autoFill_firstResult.js

@@ -10,6 +10,7 @@ add_setup(async function () {
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesUtils.history.clear();
   await PlacesTestUtils.addVisits(["http://example.com/"]);
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   // Disable placeholder completion.  The point of this test is to make sure the
   // first result is autofilled (or not) correctly.  Autofilling the placeholder

browser/components/urlbar/tests/browser/browser_autoFill_paste.js

@@ -9,6 +9,7 @@ add_task(async function test() {
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesUtils.history.clear();
   await PlacesTestUtils.addVisits(["http://example.com/"]);
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   registerCleanupFunction(async () => {
     await PlacesUtils.history.clear();
   });

browser/components/urlbar/tests/browser/browser_autoFill_placeholder.js

@@ -884,6 +884,7 @@ async function addVisits(...urls) {
       await PlacesTestUtils.addVisits(url);
     }
   }
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 }
 
 async function cleanUp() {

browser/components/urlbar/tests/browser/browser_autoFill_typed.js

@@ -12,6 +12,7 @@ add_setup(async function () {
 
 add_task(async function origin() {
   await PlacesTestUtils.addVisits(["http://example.com/"]);
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   // all lowercase
   await typeAndCheck([
     ["e", "example.com/"],
@@ -48,6 +49,7 @@ add_task(async function origin() {
 
 add_task(async function url() {
   await PlacesTestUtils.addVisits(["http://example.com/foo/bar"]);
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   // all lowercase
   await typeAndCheck([
     ["e", "example.com/"],

browser/components/urlbar/tests/browser/browser_autoFill_undo.js

@@ -10,6 +10,7 @@ add_task(async function test() {
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesUtils.history.clear();
   await PlacesTestUtils.addVisits(["http://example.com/"]);
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   // Search for "ex".  It should autofill to example.com/.
   await UrlbarTestUtils.promiseAutocompleteResultPopup({

browser/components/urlbar/tests/browser/browser_canonizeURL.js

@@ -5,6 +5,8 @@
  * Tests turning non-url-looking values typed in the input field into proper URLs.
  */
 
+requestLongerTimeout(2);
+
 const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
 
 add_task(async function checkCtrlWorks() {
@@ -61,6 +63,7 @@ add_task(async function checkCtrlWorks() {
       undefined,
       true
     );
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await UrlbarTestUtils.inputIntoURLBar(win, inputValue);
     EventUtils.synthesizeKey("KEY_Enter", options, win);
     await Promise.all([promiseLoad, promiseStopped]);
@@ -167,17 +170,17 @@ add_task(async function autofill() {
     ["@goo", "https://www.goo.com/", { ctrlKey: true }],
   ];
 
-  function promiseAutofill() {
-    return BrowserTestUtils.waitForEvent(win.gURLBar.inputField, "select");
-  }
-
   for (let [inputValue, expectedURL, options] of testcases) {
     let promiseLoad = BrowserTestUtils.waitForDocLoadAndStopIt(
       expectedURL,
       win.gBrowser.selectedBrowser
     );
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     win.gURLBar.select();
-    let autofillPromise = promiseAutofill();
+    let autofillPromise = BrowserTestUtils.waitForEvent(
+      win.gURLBar.inputField,
+      "select"
+    );
     EventUtils.sendString(inputValue, win);
     await autofillPromise;
     EventUtils.synthesizeKey("KEY_Enter", options, win);

browser/components/urlbar/tests/browser/browser_copying.js

@@ -420,6 +420,7 @@ add_task(async function includingProtocol() {
   await PlacesTestUtils.clearInputHistory();
 
   await PlacesTestUtils.addVisits(["https://example.com/"]);
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   // If the url is autofilled, the protocol should be included in the copied
   // value.

browser/components/urlbar/tests/browser/browser_downArrowKeySearch.js

@@ -23,6 +23,7 @@ add_setup(async function () {
 
 add_task(async function url() {
   await BrowserTestUtils.withNewTab("http://example.com/", async () => {
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     gURLBar.focus();
     gURLBar.selectionEnd = gURLBar.untrimmedValue.length;
     gURLBar.selectionStart = gURLBar.untrimmedValue.length;

browser/components/urlbar/tests/browser/browser_ime_composition.js

@@ -44,6 +44,7 @@ add_task(async function () {
     url: "http://example.com/",
     parentGuid: PlacesUtils.bookmarks.menuGuid,
   });
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   await SearchTestUtils.installSearchExtension(
     {

browser/components/urlbar/tests/browser/browser_inputHistory_autofill.js

@@ -52,6 +52,7 @@ add_task(async function bumped() {
     info("Running subtest: " + JSON.stringify({ url, searchString }));
 
     await PlacesTestUtils.addVisits(url);
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await UrlbarUtils.addToInputHistory(url, input);
     addToInputHistorySpy.resetHistory();
 
@@ -100,6 +101,7 @@ add_task(async function notBumped_origin() {
   for (let i = 0; i < 5; i++) {
     await PlacesTestUtils.addVisits(url);
   }
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   await triggerAutofillAndPickResult("exam", "example.com/");
 
@@ -120,6 +122,7 @@ add_task(async function notBumped_origin() {
 add_task(async function notBumped_url() {
   let url = "http://example.com/test";
   await PlacesTestUtils.addVisits(url);
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   await triggerAutofillAndPickResult("example.com/t", "example.com/test");
 

browser/components/urlbar/tests/browser/browser_move_tab_to_new_window.js

@@ -80,6 +80,7 @@ add_task(async function move_tab_into_new_window_and_open_new_tab() {
   );
   let newWindow = gBrowser.replaceTabWithWindow(tab);
   await swapDocShellPromise;
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   info("Type in the urlbar to open it and see an autofill suggestion.");
   await UrlbarTestUtils.promisePopupOpen(newWindow, async () => {

browser/components/urlbar/tests/browser/browser_searchMode_autofill.js

@@ -12,7 +12,7 @@ add_setup(async function () {
   for (let i = 0; i < 5; i++) {
     await PlacesTestUtils.addVisits([{ uri: "http://example.com/" }]);
   }
-
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   await SearchTestUtils.installSearchExtension({}, { setAsDefault: true });
   let defaultEngine = Services.search.getEngineByName("Example");
   await Services.search.moveEngine(defaultEngine, 0);

browser/components/urlbar/tests/browser/browser_speculative_connect.js

@@ -132,7 +132,7 @@ add_task(async function test_autofill() {
     let connectionNumber = server.connectionNumber;
     let searchString = serverInfo.host;
     info(`Searching for '${searchString}'`);
-
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await UrlbarTestUtils.promiseAutocompleteResultPopup({
       window,
       value: searchString,
@@ -162,7 +162,7 @@ add_task(async function test_autofill_privateContext() {
     let connectionNumber = server.connectionNumber;
     let searchString = serverInfo.host;
     info(`Searching for '${searchString}'`);
-
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await UrlbarTestUtils.promiseAutocompleteResultPopup({
       window: privateWin,
       value: searchString,

browser/components/urlbar/tests/browser/browser_tabToSearch.js

@@ -36,7 +36,7 @@ add_setup(async function () {
   for (let i = 0; i < 3; i++) {
     await PlacesTestUtils.addVisits([`https://${TEST_ENGINE_DOMAIN}/`]);
   }
-
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   registerCleanupFunction(async () => {
     await PlacesUtils.history.clear();
   });

browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js

@@ -366,6 +366,7 @@ const tests = [
     info("Type an autofilled string, Enter.");
     win.gURLBar.select();
     let promise = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await UrlbarTestUtils.promiseAutocompleteResultPopup({
       window: win,
       value: "exa",
@@ -837,6 +838,7 @@ const tests = [
     win.gURLBar.value = "example.org";
     win.gURLBar.setPageProxyState("invalid");
     let promise = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await UrlbarTestUtils.promisePopupOpen(win, () => {
       win.document.getElementById("Browser:OpenLocation").doCommand();
     });
@@ -866,6 +868,7 @@ const tests = [
     win.gURLBar.value = "example.com";
     win.gURLBar.setPageProxyState("invalid");
     let promise = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await UrlbarTestUtils.promisePopupOpen(win, () => {
       win.document.getElementById("Browser:OpenLocation").doCommand();
     });

browser/components/urlbar/tests/browser/browser_urlbar_telemetry_autofill.js

@@ -158,18 +158,6 @@ if (AppConstants.platform == "macosx") {
 }
 
 add_setup(async function () {
-  // The following initialization code is necessary to avoid a frequent
-  // intermittent failure in verify-fission where, due to timings, we may or
-  // may not import default bookmarks.
-  info("Ensure Places init is complete");
-  let placesInitCompleteObserved = TestUtils.topicObserved(
-    "places-browser-init-complete"
-  );
-  Cc["@mozilla.org/browser/browserglue;1"]
-    .getService(Ci.nsIObserver)
-    .observe(null, "browser-glue-test", "places-browser-init-complete");
-  await placesInitCompleteObserved;
-
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesUtils.history.clear();
   await PlacesTestUtils.clearInputHistory();
@@ -294,6 +282,7 @@ add_task(async function history() {
     for (const { uri, input } of inputHistory) {
       await UrlbarUtils.addToInputHistory(uri, input);
     }
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
     UrlbarPrefs.set("autoFill.adaptiveHistory.enabled", useAdaptiveHistory);
 
@@ -490,7 +479,7 @@ add_task(async function impression() {
         await UrlbarUtils.addToInputHistory(uri, input);
       }
     }
-
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await triggerAutofillAndPickResult(
       userInput,
       autofilled,
@@ -538,6 +527,7 @@ add_task(async function impression() {
 // Checks autofill deletion telemetry.
 add_task(async function deletion() {
   await PlacesTestUtils.addVisits(["http://example.com/"]);
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   info("Delete autofilled value by DELETE key");
   await doDeletionTest({

browser/components/urlbar/tests/browser/browser_urlbar_telemetry_tabtosearch.js

@@ -109,6 +109,7 @@ add_task(async function test() {
     for (let i = 0; i < 3; i++) {
       await PlacesTestUtils.addVisits([`https://${ENGINE_DOMAIN}/`]);
     }
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
     await UrlbarTestUtils.promiseAutocompleteResultPopup({
       window,
@@ -196,6 +197,7 @@ async function impressions_test(isOnboarding) {
       await PlacesTestUtils.addVisits([`https://${firstEngineHost}-2.com`]);
       await PlacesTestUtils.addVisits([`https://${ENGINE_DOMAIN}/`]);
     }
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
     // First do multiple searches for substrings of firstEngineHost. The view
     // should show the same tab-to-search onboarding result the entire time, so

browser/components/urlbar/tests/engagementTelemetry/browser/browser.ini

@@ -18,6 +18,8 @@ support-files =
   ../../ext/schema.json
 skip-if =
   os == "win" && os_version == "6.1" # Skip on Azure - frequent failure
+prefs =
+  browser.bookmarks.testing.skipDefaultBookmarksImport=true
 
 [browser_glean_telemetry_abandonment_groups.js]
 [browser_glean_telemetry_abandonment_interaction.js]
@@ -40,6 +42,7 @@ skip-if =
 [browser_glean_telemetry_engagement_selected_result.js]
 support-files =
   ../../../../search/test/browser/trendingSuggestionEngine.sjs
+skip-if = verify # Bug 1852375 - MerinoTestUtils.initWeather() doesn't play well with pushPrefEnv()
 [browser_glean_telemetry_engagement_tips.js]
 [browser_glean_telemetry_engagement_type.js]
 [browser_glean_telemetry_exposure.js]

browser/components/urlbar/tests/engagementTelemetry/browser/browser_glean_telemetry_engagement_selected_result.js

@@ -67,6 +67,7 @@ add_task(async function selected_result_autofill_adaptive() {
 add_task(async function selected_result_autofill_origin() {
   await doTest(async browser => {
     await PlacesTestUtils.addVisits("https://example.com/test");
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await openPopup("exa");
     await doEnter();
 
@@ -85,6 +86,7 @@ add_task(async function selected_result_autofill_origin() {
 add_task(async function selected_result_autofill_url() {
   await doTest(async browser => {
     await PlacesTestUtils.addVisits("https://example.com/test");
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     await openPopup("https://example.com/test");
     await doEnter();
 

browser/components/urlbar/tests/engagementTelemetry/browser/head.js

@@ -35,6 +35,12 @@ ChromeUtils.defineESModuleGetters(lazy, {
   sinon: "resource://testing-common/Sinon.sys.mjs",
 });
 
+ChromeUtils.defineLazyGetter(this, "PlacesFrecencyRecalculator", () => {
+  return Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
+    Ci.nsIObserver
+  ).wrappedJSObject;
+});
+
 async function addTopSites(url) {
   for (let i = 0; i < 5; i++) {
     await PlacesTestUtils.addVisits(url);

browser/components/urlbar/tests/ext/browser/browser.ini

@@ -8,6 +8,8 @@ support-files =
   ../api.js
   ../schema.json
   head.js
+prefs =
+  browser.bookmarks.testing.skipDefaultBookmarksImport=true
 
 [browser_ext_urlbar_attributionURL.js]
 [browser_ext_urlbar_clearInput.js]

browser/components/urlbar/tests/quicksuggest/browser/browser.ini

@@ -8,6 +8,8 @@ support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
   subdialog.xhtml
+prefs =
+  browser.bookmarks.testing.skipDefaultBookmarksImport=true
 
 [browser_quicksuggest.js]
 [browser_quicksuggest_addons.js]

browser/components/urlbar/tests/quicksuggest/browser/browser_telemetry_navigationalSuggestions.js

@@ -294,6 +294,7 @@ async function doTest({
     recordNavigationalSuggestionTelemetry: true,
   },
 }) {
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   MerinoTestUtils.server.response.body.suggestions = suggestion
     ? [suggestion]
     : [];

browser/components/urlbar/tests/quicksuggest/browser/head.js

@@ -35,6 +35,12 @@ ChromeUtils.defineLazyGetter(this, "MerinoTestUtils", () => {
   return module;
 });
 
+ChromeUtils.defineLazyGetter(this, "PlacesFrecencyRecalculator", () => {
+  return Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
+    Ci.nsIObserver
+  ).wrappedJSObject;
+});
+
 registerCleanupFunction(async () => {
   // Ensure the popup is always closed at the end of each test to avoid
   // interfering with the next test.

browser/components/urlbar/tests/unit/head.js

@@ -872,7 +872,7 @@ async function check_results({
   // updates.
   // This is not a problem in real life, but autocomplete tests should
   // return reliable resultsets, thus we have to wait.
-  await PlacesTestUtils.promiseAsyncUpdates();
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
 
   const controller = UrlbarTestUtils.newMockController({
     input: {

browser/components/urlbar/tests/unit/test_autofill_bookmarked.js

@@ -30,6 +30,7 @@ add_task(async function () {
   }
 
   async function check_autofill() {
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     let threshold = await getOriginAutofillThreshold();
     let httpOriginFrecency = await getOriginFrecency("http://", host);
     Assert.less(

browser/components/urlbar/tests/unit/test_autofill_origins.js

@@ -1025,7 +1025,7 @@ async function doTitleTest({ visits, input, expected }) {
           url: uri,
         }
       );
-      await db.executeCached("DELETE FROM moz_updateoriginsupdate_temp");
+      await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     });
   }
 

browser/components/urlbar/tests/unit/test_autofill_originsAndQueries.js

@@ -946,7 +946,7 @@ add_autofill_task(async function zeroThreshold() {
     await db.execute("UPDATE moz_places SET frecency = -1 WHERE url = :url", {
       url: pageUrl,
     });
-    await db.executeCached("DELETE FROM moz_updateoriginsupdate_temp");
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   });
 
   // Make sure the place's frecency is -1.
@@ -1120,14 +1120,14 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_0() {
   // Make the bookmark fall below the autofill frecency threshold so we ensure
   // the bookmark is always autofilled in this case, even if it doesn't meet
   // the threshold.
-  let meetsThreshold = true;
+  await TestUtils.waitForCondition(async () => {
-  while (meetsThreshold) {
     // Add a visit to another origin to boost the threshold.
     await PlacesTestUtils.addVisits("http://foo-" + url);
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     let originFrecency = await getOriginFrecency("http://", host);
     let threshold = await getOriginAutofillThreshold();
-    meetsThreshold = threshold <= originFrecency;
+    return threshold > originFrecency;
-  }
+  }, "Make the bookmark fall below the frecency threshold");
 
   // At this point, the bookmark doesn't meet the threshold, but it should
   // still be autofilled.
@@ -1227,14 +1227,14 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_prefix_0() {
   // Make the bookmark fall below the autofill frecency threshold so we ensure
   // the bookmark is always autofilled in this case, even if it doesn't meet
   // the threshold.
-  let meetsThreshold = true;
+  await TestUtils.waitForCondition(async () => {
-  while (meetsThreshold) {
     // Add a visit to another origin to boost the threshold.
     await PlacesTestUtils.addVisits("http://foo-" + url);
+    await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
     let originFrecency = await getOriginFrecency("http://", host);
     let threshold = await getOriginAutofillThreshold();
-    meetsThreshold = threshold <= originFrecency;
+    return threshold > originFrecency;
-  }
+  }, "Make the bookmark fall below the frecency threshold");
 
   // At this point, the bookmark doesn't meet the threshold, but it should
   // still be autofilled.

browser/components/urlbar/tests/unit/test_history_bookmark_results_on_search_service_failure.js

@@ -74,6 +74,7 @@ add_task(
       false,
       "Search Service should have failed to initialize."
     );
+    await cleanupPlaces();
   }
 );
 

services/sync/tests/unit/test_history_store.js

@@ -179,9 +179,6 @@ add_task(async function test_invalid_records() {
           TIMESTAMP3 +
           ")"
       );
-      // Trigger the update to the moz_origin tables by deleting the added rows
-      // from moz_updateoriginsinsert_temp
-      await db.executeCached("DELETE FROM moz_updateoriginsinsert_temp");
       // Add the corresponding visit to retain database coherence.
       await db.execute(
         "INSERT INTO moz_historyvisits " +

toolkit/components/places/Database.cpp

@@ -1612,12 +1612,6 @@ nsresult Database::InitTempEntities() {
   rv = mMainConn->ExecuteSimpleSQL(CREATE_HISTORYVISITS_AFTERDELETE_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Add the triggers that update the moz_origins table as necessary.
-  rv = mMainConn->ExecuteSimpleSQL(CREATE_UPDATEORIGINSINSERT_TEMP);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mMainConn->ExecuteSimpleSQL(
-      CREATE_UPDATEORIGINSINSERT_AFTERDELETE_TRIGGER);
-  NS_ENSURE_SUCCESS(rv, rv);
   rv = mMainConn->ExecuteSimpleSQL(CREATE_PLACES_AFTERINSERT_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = mMainConn->ExecuteSimpleSQL(CREATE_UPDATEORIGINSDELETE_TEMP);
@@ -1635,16 +1629,16 @@ nsresult Database::InitTempEntities() {
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  rv = mMainConn->ExecuteSimpleSQL(CREATE_UPDATEORIGINSUPDATE_TEMP);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mMainConn->ExecuteSimpleSQL(
-      CREATE_UPDATEORIGINSUPDATE_AFTERDELETE_TRIGGER);
-  NS_ENSURE_SUCCESS(rv, rv);
   rv = mMainConn->ExecuteSimpleSQL(CREATE_PLACES_AFTERUPDATE_FRECENCY_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = mMainConn->ExecuteSimpleSQL(
       CREATE_PLACES_AFTERUPDATE_RECALC_FRECENCY_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
+  rv = mMainConn->ExecuteSimpleSQL(
+      CREATE_ORIGINS_AFTERUPDATE_RECALC_FRECENCY_TRIGGER);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_ORIGINS_AFTERUPDATE_FRECENCY_TRIGGER);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   rv = mMainConn->ExecuteSimpleSQL(
       CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERDELETE_TRIGGER);
@@ -1887,7 +1881,17 @@ nsresult Database::MigrateV70Up() {
       "WHERE frecency < -1) AS places "
       "WHERE moz_origins.id = places.origin_id"_ns);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = RecalculateOriginFrecencyStatsInternal();
+  rv = mMainConn->ExecuteSimpleSQL(
+      "INSERT OR REPLACE INTO moz_meta(key, value) VALUES "
+      "('origin_frecency_count', "
+      "(SELECT COUNT(*) FROM moz_origins WHERE frecency > 0) "
+      "), "
+      "('origin_frecency_sum', "
+      "(SELECT TOTAL(frecency) FROM moz_origins WHERE frecency > 0) "
+      "), "
+      "('origin_frecency_sum_of_squares', "
+      "(SELECT TOTAL(frecency * frecency) FROM moz_origins WHERE frecency > 0) "
+      ") "_ns);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Now set recalc_frecency = 1 and positive frecency to any page having a
@@ -2010,26 +2014,6 @@ nsresult Database::MigrateV75Up() {
   return NS_OK;
 }
 
-nsresult Database::RecalculateOriginFrecencyStatsInternal() {
-  return mMainConn->ExecuteSimpleSQL(nsLiteralCString(
-      "INSERT OR REPLACE INTO moz_meta(key, value) VALUES "
-      "( "
-      "'" MOZ_META_KEY_ORIGIN_FRECENCY_COUNT
-      "' , "
-      "(SELECT COUNT(*) FROM moz_origins WHERE frecency > 0) "
-      "), "
-      "( "
-      "'" MOZ_META_KEY_ORIGIN_FRECENCY_SUM
-      "', "
-      "(SELECT TOTAL(frecency) FROM moz_origins WHERE frecency > 0) "
-      "), "
-      "( "
-      "'" MOZ_META_KEY_ORIGIN_FRECENCY_SUM_OF_SQUARES
-      "' , "
-      "(SELECT TOTAL(frecency * frecency) FROM moz_origins WHERE frecency > 0) "
-      ") "));
-}
-
 int64_t Database::CreateMobileRoot() {
   MOZ_ASSERT(NS_IsMainThread());
 

toolkit/components/places/Database.h

@@ -202,7 +202,6 @@ class Database final : public nsIObserver, public nsSupportsWeakReference {
     mozilla::Unused << EnsureConnection();
     return mTagsRootId;
   }
-  nsresult RecalculateOriginFrecencyStatsInternal();
 
  protected:
   /**

toolkit/components/places/History.cpp

@@ -990,26 +990,6 @@ class InsertVisitedURIs final : public Runnable {
       }
     }
 
-    {
-      // Trigger insertions for all the new origins of the places we inserted.
-      nsAutoCString query("DELETE FROM moz_updateoriginsinsert_temp");
-      nsCOMPtr<mozIStorageStatement> stmt = mHistory->GetStatement(query);
-      NS_ENSURE_STATE(stmt);
-      mozStorageStatementScoper scoper(stmt);
-      nsresult rv = stmt->Execute();
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-
-    {
-      // Trigger frecency updates for all those origins.
-      nsAutoCString query("DELETE FROM moz_updateoriginsupdate_temp");
-      nsCOMPtr<mozIStorageStatement> stmt = mHistory->GetStatement(query);
-      NS_ENSURE_STATE(stmt);
-      mozStorageStatementScoper scoper(stmt);
-      nsresult rv = stmt->Execute();
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-
     nsresult rv = transaction.Commit();
     NS_ENSURE_SUCCESS(rv, rv);
 

toolkit/components/places/History.sys.mjs

@@ -867,9 +867,6 @@ var clear = async function (db) {
   });
 
   PlacesObservers.notifyListeners([new PlacesHistoryCleared()]);
-
-  // Trigger frecency updates for all affected origins.
-  await db.execute(`DELETE FROM moz_updateoriginsupdate_temp`);
 };
 
 /**

toolkit/components/places/PlacesDBUtils.sys.mjs

@@ -41,7 +41,6 @@ export var PlacesDBUtils = {
       this.checkIntegrity,
       this.checkCoherence,
       this._refreshUI,
-      this.originFrecencyStats,
       this.incrementalVacuum,
       this.removeOldCorruptDBs,
       this.deleteOrphanPreviews,
@@ -77,7 +76,6 @@ export var PlacesDBUtils = {
       this.checkIntegrity,
       this.checkCoherence,
       this.expire,
-      this.originFrecencyStats,
       this.vacuum,
       this.stats,
       this._refreshUI,
@@ -981,19 +979,6 @@ export var PlacesDBUtils = {
     return logs;
   },
 
-  /**
-   * Recalculates statistical data on the origin frecencies in the database.
-   *
-   * @return {Promise} resolves when statistics are collected.
-   */
-  originFrecencyStats() {
-    return new Promise(resolve => {
-      lazy.PlacesUtils.history.recalculateOriginFrecencyStats(() =>
-        resolve(["Recalculated origin frecency stats"])
-      );
-    });
-  },
-
   /**
    * Collects telemetry data and reports it to Telemetry.
    *

toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs

@@ -159,13 +159,14 @@ export class PlacesFrecencyRecalculator {
    * values to update at the end of the process, it may rearm the task.
    * @param {Number} chunkSize maximum number of entries to update at a time,
    *   set to -1 to update any entry.
-   * @resolves {Number} Number of affected pages.
+   * @resolves {boolean} Whether any entry was recalculated.
    */
   async recalculateSomeFrecencies({ chunkSize = DEFAULT_CHUNK_SIZE } = {}) {
     lazy.logger.trace(
       `Recalculate ${chunkSize >= 0 ? chunkSize : "infinite"} frecency values`
     );
     let affectedCount = 0;
+    let hasRecalculatedAnything = false;
     let db = await lazy.PlacesUtils.promiseUnsafeWritableDBConnection();
     await db.executeTransaction(async function () {
       let affected = await db.executeCached(
@@ -180,22 +181,29 @@ export class PlacesFrecencyRecalculator {
         RETURNING id`
       );
       affectedCount += affected.length;
-      // We've finished recalculating frecencies. Trigger frecency updates for
-      // any affected origin.
-      await db.executeCached(`DELETE FROM moz_updateoriginsupdate_temp`);
     });
-
+    let shouldRestartRecalculation = affectedCount >= chunkSize;
-    if (affectedCount) {
+    hasRecalculatedAnything = affectedCount > 0;
+    if (hasRecalculatedAnything) {
       PlacesObservers.notifyListeners([new PlacesRanking()]);
     }
 
+    // Also recalculate some origins frecency.
+    affectedCount = await this.#recalculateSomeOriginsFrecencies({
+      chunkSize,
+    });
+    shouldRestartRecalculation ||= affectedCount >= chunkSize;
+    hasRecalculatedAnything ||= affectedCount > 0;
+
     // If alternative frecency is enabled, also recalculate a chunk of it.
-    affectedCount +=
+    affectedCount =
       await this.#alternativeFrecencyHelper.recalculateSomeAlternativeFrecencies(
         { chunkSize }
       );
+    shouldRestartRecalculation ||= affectedCount >= chunkSize;
+    hasRecalculatedAnything ||= affectedCount > 0;
 
-    if (chunkSize > 0 && affectedCount >= chunkSize) {
+    if (chunkSize > 0 && shouldRestartRecalculation) {
       // There's more entries to recalculate, rearm the task.
       this.#task.arm();
     } else {
@@ -203,6 +211,56 @@ export class PlacesFrecencyRecalculator {
       lazy.PlacesUtils.history.shouldStartFrecencyRecalculation = false;
       this.#task.disarm();
     }
+    return hasRecalculatedAnything;
+  }
+
+  async #recalculateSomeOriginsFrecencies({ chunkSize }) {
+    lazy.logger.trace(`Recalculate ${chunkSize} origins frecency values`);
+    let affectedCount = 0;
+    let db = await lazy.PlacesUtils.promiseUnsafeWritableDBConnection();
+    await db.executeTransaction(async () => {
+      let affected = await db.executeCached(
+        `
+        UPDATE moz_origins
+        SET frecency = CAST(
+              (SELECT total(frecency)
+               FROM moz_places h
+               WHERE origin_id = moz_origins.id AND frecency > 0)
+              AS INT
+            ),
+            recalc_frecency = 0
+        WHERE id IN (
+          SELECT id FROM moz_origins
+          WHERE recalc_frecency = 1
+          ORDER BY frecency DESC
+          LIMIT ${chunkSize}
+        )
+        RETURNING id`
+      );
+      affectedCount += affected.length;
+
+      // Calculate and store the frecency statistics used to calculate a
+      // thredhold. Origins above that threshold will be considered meaningful
+      // and autofilled.
+      // While it may be tempting to do this only when some frecency was
+      // updated, that won't catch the edge case of the moz_origins table being
+      // emptied.
+      let row = (
+        await db.executeCached(`
+        SELECT count(*), total(frecency), total(pow(frecency,2))
+        FROM moz_origins
+        WHERE frecency > 0
+      `)
+      )[0];
+      await lazy.PlacesUtils.metadata.setMany(
+        new Map([
+          ["origin_frecency_count", row.getResultByIndex(0)],
+          ["origin_frecency_sum", row.getResultByIndex(1)],
+          ["origin_frecency_sum_of_squares", row.getResultByIndex(2)],
+        ])
+      );
+    });
+
     return affectedCount;
   }
 
@@ -273,9 +331,6 @@ export class PlacesFrecencyRecalculator {
             ),
           }
         );
-        // We've finished decaying frecencies. Trigger frecency updates for
-        // any affected origin.
-        await db.executeCached(`DELETE FROM moz_updateoriginsupdate_temp`);
 
         TelemetryStopwatch.finish("PLACES_IDLE_FRECENCY_DECAY_TIME_MS", refObj);
         PlacesObservers.notifyListeners([new PlacesRanking()]);

toolkit/components/places/PlacesSyncUtils.sys.mjs

@@ -690,8 +690,12 @@ const BookmarkSyncUtils = (PlacesSyncUtils.bookmarks = Object.freeze({
     // wipe; we want to merge the restored tree with the one on the server.
     await lazy.PlacesUtils.metadata.setWithConnection(
       db,
-      BookmarkSyncUtils.WIPE_REMOTE_META_KEY,
+      new Map([
-      source == lazy.PlacesUtils.bookmarks.SOURCES.RESTORE
+        [
+          BookmarkSyncUtils.WIPE_REMOTE_META_KEY,
+          source == lazy.PlacesUtils.bookmarks.SOURCES.RESTORE,
+        ],
+      ])
     );
 
     // Reset change counters and sync statuses for roots and remaining
@@ -2027,8 +2031,7 @@ var removeUndeletedTombstones = function (db, guids) {
 async function setHistorySyncId(db, newSyncId) {
   await lazy.PlacesUtils.metadata.setWithConnection(
     db,
-    HistorySyncUtils.SYNC_ID_META_KEY,
+    new Map([[HistorySyncUtils.SYNC_ID_META_KEY, newSyncId]])
-    newSyncId
   );
 
   await lazy.PlacesUtils.metadata.deleteWithConnection(
@@ -2041,8 +2044,7 @@ async function setHistorySyncId(db, newSyncId) {
 async function setBookmarksSyncId(db, newSyncId) {
   await lazy.PlacesUtils.metadata.setWithConnection(
     db,
-    BookmarkSyncUtils.SYNC_ID_META_KEY,
+    new Map([[BookmarkSyncUtils.SYNC_ID_META_KEY, newSyncId]])
-    newSyncId
   );
 
   await lazy.PlacesUtils.metadata.deleteWithConnection(

toolkit/components/places/PlacesUtils.sys.mjs

@@ -1876,7 +1876,6 @@ export var PlacesUtils = {
         frecency: url.protocol == "place:" ? 0 : -1,
       }
     );
-    await db.executeCached("DELETE FROM moz_updateoriginsinsert_temp");
   },
 
   /**
@@ -1906,7 +1905,6 @@ export var PlacesUtils = {
         maybeguid: this.history.makeGuid(),
       }))
     );
-    await db.executeCached("DELETE FROM moz_updateoriginsinsert_temp");
   },
 
   /**
@@ -2183,7 +2181,19 @@ PlacesUtils.metadata = {
    */
   set(key, value) {
     return PlacesUtils.withConnectionWrapper("PlacesUtils.metadata.set", db =>
-      this.setWithConnection(db, key, value)
+      this.setWithConnection(db, new Map([[key, value]]))
+    );
+  },
+
+  /**
+   * Sets the value for multiple keys.
+   *
+   * @param {Map} pairs
+   *        The metadata keys to update, with their value.
+   */
+  setMany(pairs) {
+    return PlacesUtils.withConnectionWrapper("PlacesUtils.metadata.set", db =>
+      this.setWithConnection(db, pairs)
     );
   },
 
@@ -2248,28 +2258,46 @@ PlacesUtils.metadata = {
     return value;
   },
 
-  async setWithConnection(db, key, value) {
+  async setWithConnection(db, pairs) {
-    if (value === null) {
+    let entriesToSet = [];
-      await this.deleteWithConnection(db, key);
+    let keysToDelete = Array.from(pairs.entries())
-      return;
+      .filter(([key, value]) => {
+        if (value !== null) {
+          console.log("key " + key);
+          entriesToSet.push({ key: this.canonicalizeKey(key), value });
+          return false;
+        }
+        return true;
+      })
+      .map(([key, value]) => key);
+    if (keysToDelete.length) {
+      await this.deleteWithConnection(db, ...keysToDelete);
+      if (keysToDelete.length == pairs.size) {
+        return;
+      }
     }
 
-    let cacheValue = value;
+    // Generate key{i}, value{i} pairs for the SQL bindings.
-    if (
+    let params = entriesToSet.reduce((accumulator, { key, value }, i) => {
-      typeof value == "object" &&
+      accumulator[`key${i}`] = key;
-      ChromeUtils.getClassName(value) != "Uint8Array"
+      // Convert Objects to base64 JSON urls.
-    ) {
+      accumulator[`value${i}`] =
-      value = this.jsonPrefix + this._base64Encode(JSON.stringify(value));
+        typeof value == "object" &&
-    }
+        ChromeUtils.getClassName(value) != "Uint8Array"
-
+          ? this.jsonPrefix + this._base64Encode(JSON.stringify(value))
-    key = this.canonicalizeKey(key);
+          : value;
+      return accumulator;
+    }, {});
     await db.executeCached(
-      `
+      "REPLACE INTO moz_meta (key, value) VALUES " +
-      REPLACE INTO moz_meta (key, value)
+        entriesToSet.map((e, i) => `(:key${i}, :value${i})`).join(),
-      VALUES (:key, :value)`,
+      params
-      { key, value }
     );
-    this.cache.set(key, cacheValue);
+
+    // Update the cache.
+    entriesToSet.forEach(({ key, value }) => {
+      this.cache.set(key, value);
+    });
   },
 
   async deleteWithConnection(db, ...keys) {

toolkit/components/places/bookmark_sync/src/store.rs

@@ -551,11 +551,6 @@ fn update_local_items_in_places<'t>(
         statement.execute()?;
     }
 
-    // Trigger frecency updates for all new origins.
-    debug!(driver, "Updating origins for new URLs");
-    controller.err_if_aborted()?;
-    db.exec("DELETE FROM moz_updateoriginsinsert_temp")?;
-
     // Build a table of new and updated items.
     debug!(driver, "Staging apply remote item ops");
     for chunk in ops.apply_remote_items.chunks(db.variable_limit()? / 3) {
@@ -747,11 +742,6 @@ fn update_local_items_in_places<'t>(
     debug!(driver, "Applying remote items");
     apply_remote_items(db, driver, controller)?;
 
-    // Trigger frecency updates for all affected origins.
-    debug!(driver, "Updating origins for changed URLs");
-    controller.err_if_aborted()?;
-    db.exec("DELETE FROM moz_updateoriginsupdate_temp")?;
-
     // Fires the `applyNewLocalStructure` trigger.
     debug!(driver, "Applying new local structure");
     controller.err_if_aborted()?;

toolkit/components/places/nsINavHistoryService.idl

@@ -1109,16 +1109,6 @@ interface nsINavHistoryService : nsISupports
    */
   unsigned long long hashURL(in ACString aSpec, [optional] in ACString aMode);
 
-  /**
-   * Resets and recalculates the origin frecency statistics that are kept in the
-   * moz_meta table.
-   *
-   * @param aCallback
-   *        Called when the recalculation is complete.  The arguments passed to
-   *        the observer are not defined.
-   */
-  void recalculateOriginFrecencyStats([optional] in nsIObserver aCallback);
-
   /**
    * Whether frecency is in the process of being decayed. The value can also
    * be read through the is_frecency_decaying() SQL function exposed by Places

toolkit/components/places/nsNavHistory.cpp

@@ -395,14 +395,6 @@ nsresult nsNavHistory::GetOrCreateIdForPage(nsIURI* aURI, int64_t* _pageId,
     *_pageId = sLastInsertedPlaceId;
   }
 
-  {
-    // Trigger the updates to the moz_origins tables
-    nsCOMPtr<mozIStorageStatement> stmt =
-        mDB->GetStatement("DELETE FROM moz_updateoriginsinsert_temp");
-    NS_ENSURE_STATE(stmt);
-    mozStorageStatementScoper scoper(stmt);
-  }
-
   return NS_OK;
 }
 
@@ -452,33 +444,6 @@ void nsNavHistory::UpdateDaysOfHistory(PRTime visitTime) {
   }
 }
 
-NS_IMETHODIMP
-nsNavHistory::RecalculateOriginFrecencyStats(nsIObserver* aCallback) {
-  RefPtr<nsNavHistory> self(this);
-  nsMainThreadPtrHandle<nsIObserver> callback(
-      !aCallback ? nullptr
-                 : new nsMainThreadPtrHolder<nsIObserver>(
-                       "nsNavHistory::RecalculateOriginFrecencyStats callback",
-                       aCallback));
-
-  nsCOMPtr<nsIEventTarget> target(do_GetInterface(mDB->MainConn()));
-  NS_ENSURE_STATE(target);
-  nsresult rv = target->Dispatch(NS_NewRunnableFunction(
-      "nsNavHistory::RecalculateOriginFrecencyStats", [self, callback] {
-        Unused << self->mDB->RecalculateOriginFrecencyStatsInternal();
-        Unused << NS_DispatchToMainThread(NS_NewRunnableFunction(
-            "nsNavHistory::RecalculateOriginFrecencyStats callback",
-            [callback] {
-              if (callback) {
-                Unused << callback->Observe(nullptr, "", nullptr);
-              }
-            }));
-      }));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return NS_OK;
-}
-
 Atomic<int64_t> nsNavHistory::sLastInsertedPlaceId(0);
 Atomic<int64_t> nsNavHistory::sLastInsertedVisitId(0);
 Atomic<bool> nsNavHistory::sIsFrecencyDecaying(false);

toolkit/components/places/nsNavHistory.h

@@ -426,8 +426,6 @@ class nsNavHistory final : public nsSupportsWeakReference,
   int32_t mUnvisitedTypedBonus;
   int32_t mReloadVisitBonus;
 
-  nsresult RecalculateOriginFrecencyStatsInternal();
-
   // in nsNavHistoryQuery.cpp
   nsresult TokensToQuery(
       const nsTArray<mozilla::places::QueryKeyValuePair>& aTokens,

toolkit/components/places/nsPlacesTables.h

@@ -199,48 +199,20 @@
       ", PRIMARY KEY (url, userContextId)"     \
       ")")
 
-// This table is used, along with moz_places_afterinsert_trigger, to update
+// This table is used to remove orphan origins after pages are removed from
-// origins after places removals. During an INSERT into moz_places, origins are
+// moz_places. Insertions are made by moz_places_afterdelete_trigger.
-// accumulated in this table, then a DELETE FROM moz_updateoriginsinsert_temp
+// This allows for more performant handling of batch removals, since we'll look
-// will take care of updating the moz_origins table for every new origin. See
+// for orphan origins only once, instead of doing it for each page removal.
-// CREATE_PLACES_AFTERINSERT_TRIGGER in nsPlacestriggers.h for details.
+// The downside of this approach is that after the removal is complete the
-#define CREATE_UPDATEORIGINSINSERT_TEMP                   \
+// consumer must remember to also delete from this table, and a trigger will
-  nsLiteralCString(                                       \
+// take care of orphans.
-      "CREATE TEMP TABLE moz_updateoriginsinsert_temp ( " \
-      "place_id INTEGER PRIMARY KEY, "                    \
-      "prefix TEXT NOT NULL, "                            \
-      "host TEXT NOT NULL, "                              \
-      "frecency INTEGER NOT NULL "                        \
-      ") ")
-
-// This table is used in a similar way to moz_updateoriginsinsert_temp, but for
-// deletes, and triggered via moz_places_afterdelete_trigger.
-//
-// When rows are added to this table, moz_places.origin_id may be null.  That's
-// why this table uses prefix + host as its primary key, not origin_id.
 #define CREATE_UPDATEORIGINSDELETE_TEMP                   \
   nsLiteralCString(                                       \
       "CREATE TEMP TABLE moz_updateoriginsdelete_temp ( " \
-      "prefix TEXT NOT NULL, "                            \
+      "  prefix TEXT NOT NULL, "                          \
-      "host TEXT NOT NULL, "                              \
+      "  host TEXT NOT NULL, "                            \
-      "frecency_delta INTEGER NOT NULL, "                 \
+      "  PRIMARY KEY (prefix, host) "                     \
-      "PRIMARY KEY (prefix, host) "                       \
+      ") WITHOUT ROWID")
-      ") WITHOUT ROWID ")
-
-// This table is used in a similar way to moz_updateoriginsinsert_temp, but for
-// updates to places' frecencies, and triggered via
-// moz_places_afterupdate_frecency_trigger.
-//
-// When rows are added to this table, moz_places.origin_id may be null.  That's
-// why this table uses prefix + host as its primary key, not origin_id.
-#define CREATE_UPDATEORIGINSUPDATE_TEMP                   \
-  nsLiteralCString(                                       \
-      "CREATE TEMP TABLE moz_updateoriginsupdate_temp ( " \
-      "prefix TEXT NOT NULL, "                            \
-      "host TEXT NOT NULL, "                              \
-      "frecency_delta INTEGER NOT NULL, "                 \
-      "PRIMARY KEY (prefix, host) "                       \
-      ") WITHOUT ROWID ")
 
 // This table would not be strictly needed for functionality since it's just
 // mimicking moz_places, though it's great for database portability.
@@ -298,12 +270,6 @@
       "value NOT NULL"          \
       ") WITHOUT ROWID ")
 
-// Keys in the moz_meta table.
-#define MOZ_META_KEY_ORIGIN_FRECENCY_COUNT "origin_frecency_count"
-#define MOZ_META_KEY_ORIGIN_FRECENCY_SUM "origin_frecency_sum"
-#define MOZ_META_KEY_ORIGIN_FRECENCY_SUM_OF_SQUARES \
-  "origin_frecency_sum_of_squares"
-
 // This table holds history interactions that will be used to achieve improved
 // history recalls.
 #define CREATE_MOZ_PLACES_METADATA                                           \

toolkit/components/places/nsPlacesTriggers.h

@@ -55,102 +55,46 @@
         "WHERE id = OLD.place_id;"                                            \
         "END")
 
-// This macro is a helper for the next several triggers.  It updates the origin
+// This trigger stores the last_inserted_id, and inserts entries in moz_origins
-// frecency stats.  Use it as follows.  Before changing an origin's frecency,
+// when pages are added.
-// call the macro and pass "-" (subtraction) as the argument.  That will update
-// the stats by deducting the origin's current contribution to them.  And then
-// after you change the origin's frecency, call the macro again, this time
-// passing "+" (addition) as the argument.  That will update the stats by adding
-// the origin's new contribution to them.
-#  define UPDATE_ORIGIN_FRECENCY_STATS(op)                            \
-    "INSERT OR REPLACE INTO moz_meta(key, value) "                    \
-    "SELECT '" MOZ_META_KEY_ORIGIN_FRECENCY_COUNT                     \
-    "', "                                                             \
-    "IFNULL((SELECT value FROM moz_meta WHERE key = "                 \
-    "'" MOZ_META_KEY_ORIGIN_FRECENCY_COUNT "'), 0) " op               \
-    " CAST(frecency > 0 AS INT) "                                     \
-    "FROM moz_origins WHERE prefix = OLD.prefix AND host = OLD.host " \
-    "UNION "                                                          \
-    "SELECT '" MOZ_META_KEY_ORIGIN_FRECENCY_SUM                       \
-    "', "                                                             \
-    "IFNULL((SELECT value FROM moz_meta WHERE key = "                 \
-    "'" MOZ_META_KEY_ORIGIN_FRECENCY_SUM "'), 0) " op                 \
-    " MAX(frecency, 0) "                                              \
-    "FROM moz_origins WHERE prefix = OLD.prefix AND host = OLD.host " \
-    "UNION "                                                          \
-    "SELECT '" MOZ_META_KEY_ORIGIN_FRECENCY_SUM_OF_SQUARES            \
-    "', "                                                             \
-    "IFNULL((SELECT value FROM moz_meta WHERE key = "                 \
-    "'" MOZ_META_KEY_ORIGIN_FRECENCY_SUM_OF_SQUARES "'), 0) " op      \
-    " (MAX(frecency, 0) * MAX(frecency, 0)) "                         \
-    "FROM moz_origins WHERE prefix = OLD.prefix AND host = OLD.host "
-
-// The next several triggers are a workaround for the lack of FOR EACH STATEMENT
-// in Sqlite, until bug 871908 can be fixed properly.
-//
-// While doing inserts or deletes into moz_places, we accumulate the affected
-// origins into a temp table. Afterwards, we delete everything from the temp
-// table, causing the AFTER DELETE trigger to fire for it, which will then
-// update moz_origins and the origin frecency stats. As a consequence, we also
-// do this for updates to moz_places.frecency in order to make sure that changes
-// to origins are serialized.
-//
-// Note this way we lose atomicity, crashing between the 2 queries may break the
-// tables' coherency. So it's better to run those DELETE queries in a single
-// transaction. Regardless, this is still better than hanging the browser for
-// several minutes on a fast machine.
-
-// This trigger runs on inserts into moz_places.
 #  define CREATE_PLACES_AFTERINSERT_TRIGGER                                 \
     nsLiteralCString(                                                       \
         "CREATE TEMP TRIGGER moz_places_afterinsert_trigger "               \
         "AFTER INSERT ON moz_places FOR EACH ROW "                          \
         "BEGIN "                                                            \
         "SELECT store_last_inserted_id('moz_places', NEW.id); "             \
-        "INSERT OR IGNORE INTO moz_updateoriginsinsert_temp (place_id, "    \
+        "INSERT INTO moz_origins "                                          \
-        "prefix, "                                                          \
+        "  (prefix, host, frecency, recalc_frecency, recalc_alt_frecency) " \
-        "host, frecency) "                                                  \
+        "VALUES (get_prefix(NEW.url), get_host_and_port(NEW.url), "         \
-        "VALUES (NEW.id, get_prefix(NEW.url), get_host_and_port(NEW.url), " \
+        "        NEW.frecency, 1, 1)  "                                     \
-        "NEW.frecency); "                                                   \
+        "ON CONFLICT(prefix, host) DO UPDATE "                              \
+        "  SET recalc_frecency = 1, recalc_alt_frecency = 1 "               \
+        "  WHERE EXCLUDED.recalc_frecency = 0 OR "                          \
+        "        EXCLUDED.recalc_alt_frecency = 0; "                        \
+        "UPDATE moz_places SET origin_id = ( "                              \
+        "  SELECT id "                                                      \
+        "  FROM moz_origins "                                               \
+        "  WHERE prefix = get_prefix(NEW.url) "                             \
+        "    AND host = get_host_and_port(NEW.url) "                        \
+        ") "                                                                \
+        "WHERE id = NEW.id; "                                               \
         "END")
-// This trigger corresponds to the previous trigger.  It runs on deletes on
-// moz_updateoriginsinsert_temp -- logically, after inserts on moz_places.
-#  define CREATE_UPDATEORIGINSINSERT_AFTERDELETE_TRIGGER \
-    nsLiteralCString( \
-  "CREATE TEMP TRIGGER moz_updateoriginsinsert_afterdelete_trigger " \
-  "AFTER DELETE ON moz_updateoriginsinsert_temp FOR EACH ROW " \
-  "BEGIN " \
-    /* Deduct the origin's current contribution to frecency stats */ \
-    UPDATE_ORIGIN_FRECENCY_STATS("-") "; " \
-    "INSERT INTO moz_origins (prefix, host, frecency, recalc_alt_frecency) " \
-    "VALUES (OLD.prefix, OLD.host, MAX(OLD.frecency, 0), 1) " \
-    "ON CONFLICT(prefix, host) DO UPDATE " \
-    "SET frecency = frecency + OLD.frecency " \
-    "WHERE OLD.frecency > 0; " \
-    /* Add the origin's new contribution to frecency stats */ \
-    UPDATE_ORIGIN_FRECENCY_STATS("+") "; " \
-    "UPDATE moz_places SET origin_id = ( " \
-      "SELECT id " \
-      "FROM moz_origins " \
-      "WHERE prefix = OLD.prefix AND host = OLD.host " \
-    ") " \
-    "WHERE id = OLD.place_id; " \
-  "END" \
-)
 
-// This trigger runs on deletes on moz_places.
+// This trigger is a workaround for the lack of FOR EACH STATEMENT in Sqlite.
+// While doing deletes into moz_places, we accumulate the affected origins into
+// a temp table. Afterwards, we delete everything from the temp table, causing
+// the AFTER DELETE trigger to fire for it, which will then update moz_origins.
+//
+// Note this way we lose atomicity, crashing between the 2 queries may break the
+// tables' coherency. So it's better to run those DELETE queries in the same
+// transaction as the original change.
 #  define CREATE_PLACES_AFTERDELETE_TRIGGER                                    \
     nsLiteralCString(                                                          \
         "CREATE TEMP TRIGGER moz_places_afterdelete_trigger "                  \
         "AFTER DELETE ON moz_places FOR EACH ROW "                             \
         "BEGIN "                                                               \
-        "INSERT INTO moz_updateoriginsdelete_temp (prefix, host, "             \
+        "INSERT OR IGNORE INTO moz_updateoriginsdelete_temp (prefix, host) "   \
-        "frecency_delta) "                                                     \
+        "VALUES (get_prefix(OLD.url), get_host_and_port(OLD.url)); "           \
-        "VALUES (get_prefix(OLD.url), get_host_and_port(OLD.url), "            \
-        "-MAX(OLD.frecency, 0)) "                                              \
-        "ON CONFLICT(prefix, host) DO UPDATE "                                 \
-        "SET frecency_delta = frecency_delta - OLD.frecency "                  \
-        "WHERE OLD.frecency > 0; "                                             \
         "UPDATE moz_origins SET recalc_frecency = 1, recalc_alt_frecency = 1 " \
         "WHERE id = OLD.origin_id; "                                           \
         "END ")
@@ -163,51 +107,27 @@
         "CREATE TEMP TRIGGER moz_places_afterdelete_wpreviews_trigger "        \
         "AFTER DELETE ON moz_places FOR EACH ROW "                             \
         "BEGIN "                                                               \
-        "INSERT INTO moz_updateoriginsdelete_temp (prefix, host, "             \
+        "INSERT OR IGNORE INTO moz_updateoriginsdelete_temp (prefix, host) "   \
-        "frecency_delta) "                                                     \
+        "VALUES (get_prefix(OLD.url), get_host_and_port(OLD.url)); "           \
-        "VALUES (get_prefix(OLD.url), get_host_and_port(OLD.url), "            \
-        "-MAX(OLD.frecency, 0)) "                                              \
-        "ON CONFLICT(prefix, host) DO UPDATE "                                 \
-        "SET frecency_delta = frecency_delta - OLD.frecency "                  \
-        "WHERE OLD.frecency > 0; "                                             \
         "UPDATE moz_origins SET recalc_frecency = 1, recalc_alt_frecency = 1 " \
         "WHERE id = OLD.origin_id; "                                           \
         "INSERT OR IGNORE INTO moz_previews_tombstones VALUES "                \
         "(md5hex(OLD.url));"                                                   \
         "END ")
 
-// This trigger corresponds to the previous trigger.  It runs on deletes
+// This is the supporting table for the "AFTER DELETE ON moz_places" triggers.
-// on moz_updateoriginsdelete_temp -- logically, after deletes on
+#  define CREATE_UPDATEORIGINSDELETE_AFTERDELETE_TRIGGER                   \
-// moz_places.
+    nsLiteralCString(                                                      \
-#  define CREATE_UPDATEORIGINSDELETE_AFTERDELETE_TRIGGER \
+        "CREATE TEMP TRIGGER moz_updateoriginsdelete_afterdelete_trigger " \
-    nsLiteralCString( \
+        "AFTER DELETE ON moz_updateoriginsdelete_temp FOR EACH ROW "       \
-  "CREATE TEMP TRIGGER moz_updateoriginsdelete_afterdelete_trigger " \
+        "BEGIN "                                                           \
-  "AFTER DELETE ON moz_updateoriginsdelete_temp FOR EACH ROW " \
+        "DELETE FROM moz_origins "                                         \
-  "BEGIN " \
+        "WHERE prefix = OLD.prefix AND host = OLD.host "                   \
-    /* Deduct the origin's current contribution to frecency stats */ \
+        "AND NOT EXISTS ( "                                                \
-    UPDATE_ORIGIN_FRECENCY_STATS("-") "; " \
+        "    SELECT id FROM moz_places "                                   \
-    "UPDATE moz_origins SET frecency = frecency + OLD.frecency_delta, " \
+        "    WHERE origin_id = moz_origins.id "                            \
-                           "recalc_frecency = 0 " \
+        "); "                                                              \
-    "WHERE prefix = OLD.prefix AND host = OLD.host; " \
+        "END")
-    "DELETE FROM moz_origins " \
-    "WHERE prefix = OLD.prefix AND host = OLD.host AND NOT EXISTS ( " \
-      "SELECT id FROM moz_places " \
-      "WHERE origin_id = moz_origins.id " \
-      "LIMIT 1 " \
-    "); " \
-    /* Add the origin's new contribution to frecency stats */ \
-    UPDATE_ORIGIN_FRECENCY_STATS("+") "; " \
-    "DELETE FROM moz_icons WHERE id IN ( " \
-      "SELECT id FROM moz_icons " \
-      "WHERE fixed_icon_url_hash = hash(fixup_url(OLD.host || '/favicon.ico')) " \
-        "AND fixup_url(icon_url) = fixup_url(OLD.host || '/favicon.ico') " \
-        "AND NOT EXISTS (SELECT 1 FROM moz_origins WHERE host = OLD.host " \
-                                                     "OR host = fixup_url(OLD.host)) " \
-      "EXCEPT " \
-      "SELECT icon_id FROM moz_icons_to_pages " \
-    "); " \
-  "END" \
-)
 
 // This trigger runs on updates to moz_places.frecency.
 //
@@ -222,34 +142,10 @@
         "AFTER UPDATE OF frecency ON moz_places FOR EACH ROW "                 \
         "WHEN NOT is_frecency_decaying() "                                     \
         "BEGIN "                                                               \
-        "INSERT INTO moz_updateoriginsupdate_temp (prefix, host, "             \
-        "frecency_delta) "                                                     \
-        "VALUES (get_prefix(NEW.url), get_host_and_port(NEW.url), "            \
-        "MAX(NEW.frecency, 0) - MAX(OLD.frecency, 0)) "                        \
-        "ON CONFLICT(prefix, host) DO UPDATE "                                 \
-        "SET frecency_delta = frecency_delta + EXCLUDED.frecency_delta; "      \
         "UPDATE moz_places SET recalc_frecency = 0 WHERE id = NEW.id; "        \
         "UPDATE moz_origins SET recalc_frecency = 1, recalc_alt_frecency = 1 " \
         "WHERE id = NEW.origin_id; "                                           \
         "END ")
-// This trigger corresponds to the previous trigger.  It runs on deletes
-// on moz_updateoriginsupdate_temp -- logically, after updates to
-// moz_places.frecency.
-#  define CREATE_UPDATEORIGINSUPDATE_AFTERDELETE_TRIGGER \
-    nsLiteralCString( \
-  "CREATE TEMP TRIGGER moz_updateoriginsupdate_afterdelete_trigger " \
-  "AFTER DELETE ON moz_updateoriginsupdate_temp FOR EACH ROW " \
-  "BEGIN " \
-    /* Deduct the origin's current contribution to frecency stats */ \
-    UPDATE_ORIGIN_FRECENCY_STATS("-") "; " \
-    "UPDATE moz_origins " \
-    "SET frecency = frecency + OLD.frecency_delta, " \
-        "recalc_frecency = 0 " \
-    "WHERE prefix = OLD.prefix AND host = OLD.host; " \
-    /* Add the origin's new contribution to frecency stats */ \
-    UPDATE_ORIGIN_FRECENCY_STATS("+") "; " \
-  "END" \
-)
 
 // Runs when recalc_frecency is set to 1
 #  define CREATE_PLACES_AFTERUPDATE_RECALC_FRECENCY_TRIGGER                   \
@@ -260,6 +156,31 @@
         "BEGIN "                                                              \
         "  SELECT set_should_start_frecency_recalculation();"                 \
         "END")
+#  define CREATE_ORIGINS_AFTERUPDATE_RECALC_FRECENCY_TRIGGER                   \
+    nsLiteralCString(                                                          \
+        "CREATE TEMP TRIGGER moz_origins_afterupdate_recalc_frecency_trigger " \
+        "AFTER UPDATE OF recalc_frecency ON moz_origins FOR EACH ROW "         \
+        "WHEN NEW.recalc_frecency = 1 "                                        \
+        "BEGIN "                                                               \
+        "  SELECT set_should_start_frecency_recalculation();"                  \
+        "END")
+
+// Runs when origin frecency is set to 0.
+// This is in addition to moz_updateoriginsdelete_afterdelete_trigger, as a
+// sanity check to ensure orphan origins don't stay around. We cannot just rely
+// on this because it runs delayed and in the meanwhile the existence of the
+// origin may impact user's privacy.
+#  define CREATE_ORIGINS_AFTERUPDATE_FRECENCY_TRIGGER                   \
+    nsLiteralCString(                                                   \
+        "CREATE TEMP TRIGGER moz_origins_afterupdate_frecency_trigger " \
+        "AFTER UPDATE OF recalc_frecency ON moz_origins FOR EACH ROW "  \
+        "WHEN NEW.frecency = 0 AND OLD.frecency > 0 "                   \
+        "BEGIN "                                                        \
+        "DELETE FROM moz_origins "                                      \
+        "WHERE id = NEW.id AND NOT EXISTS ( "                           \
+        "  SELECT id FROM moz_places WHERE origin_id = NEW.id "         \
+        "); "                                                           \
+        "END")
 
 /**
  * This trigger removes a row from moz_openpages_temp when open_count

toolkit/components/places/tests/maintenance/test_preventive_maintenance.js

@@ -62,7 +62,6 @@ async function addPlace(
           }
         )
       )[0].getResultByIndex(0);
-      await db.executeCached("DELETE FROM moz_updateoriginsinsert_temp");
 
       if (aFavicon) {
         await db.executeCached(
@@ -2570,84 +2569,6 @@ tests.push({
 
 // ------------------------------------------------------------------------------
 
-tests.push({
-  name: "T.1",
-  desc: "history.recalculateOriginFrecencyStats() is called",
-
-  async setup() {
-    let urls = [
-      "http://example1.com/",
-      "http://example2.com/",
-      "http://example3.com/",
-    ];
-    await PlacesTestUtils.addVisits(urls.map(u => ({ uri: u })));
-
-    this._frecencies = [];
-    for (let url of urls) {
-      this._frecencies.push(
-        await PlacesTestUtils.getDatabaseValue("moz_places", "frecency", {
-          url,
-        })
-      );
-    }
-
-    let stats = await this._promiseStats();
-    Assert.equal(stats.count, this._frecencies.length, "Sanity check");
-    Assert.equal(stats.sum, this._sum(this._frecencies), "Sanity check");
-    Assert.equal(
-      stats.squares,
-      this._squares(this._frecencies),
-      "Sanity check"
-    );
-
-    await PlacesUtils.withConnectionWrapper("T.1", db =>
-      db.execute(`
-        INSERT OR REPLACE INTO moz_meta VALUES
-        ('origin_frecency_count', 99),
-        ('origin_frecency_sum', 99999),
-        ('origin_frecency_sum_of_squares', 99999 * 99999);
-      `)
-    );
-
-    stats = await this._promiseStats();
-    Assert.equal(stats.count, 99);
-    Assert.equal(stats.sum, 99999);
-    Assert.equal(stats.squares, 99999 * 99999);
-  },
-
-  async check() {
-    let stats = await this._promiseStats();
-    Assert.equal(stats.count, this._frecencies.length);
-    Assert.equal(stats.sum, this._sum(this._frecencies));
-    Assert.equal(stats.squares, this._squares(this._frecencies));
-  },
-
-  _sum(frecs) {
-    return frecs.reduce((memo, f) => memo + f, 0);
-  },
-
-  _squares(frecs) {
-    return frecs.reduce((memo, f) => memo + f * f, 0);
-  },
-
-  async _promiseStats() {
-    let db = await PlacesUtils.promiseDBConnection();
-    let rows = await db.execute(`
-      SELECT
-        IFNULL((SELECT value FROM moz_meta WHERE key = 'origin_frecency_count'), 0),
-        IFNULL((SELECT value FROM moz_meta WHERE key = 'origin_frecency_sum'), 0),
-        IFNULL((SELECT value FROM moz_meta WHERE key = 'origin_frecency_sum_of_squares'), 0)
-    `);
-    return {
-      count: rows[0].getResultByIndex(0),
-      sum: rows[0].getResultByIndex(1),
-      squares: rows[0].getResultByIndex(2),
-    };
-  },
-});
-
-// ------------------------------------------------------------------------------
-
 tests.push({
   name: "Z",
   desc: "Sanity: Preventive maintenance does not touch valid items",

toolkit/components/places/tests/maintenance/test_preventive_maintenance_checkAndFixDatabase.js

@@ -26,10 +26,10 @@ add_task(async function () {
       failedTasks.push(val);
     }
   });
-  Assert.equal(numberOfTasksRun, 7, "Check that we have run all tasks.");
+  Assert.equal(numberOfTasksRun, 6, "Check that we have run all tasks.");
   Assert.equal(
     successfulTasks.length,
-    7,
+    6,
     "Check that we have run all tasks successfully"
   );
   Assert.equal(failedTasks.length, 0, "Check that no task is failing");

toolkit/components/places/tests/unit/test_frecency_origins_recalc.js

@@ -3,10 +3,6 @@
 
 // Tests that recalc_frecency in the moz_origins table works is consistent.
 
-// This test does not completely cover origins frecency recalculation because
-// the current system uses temp tables and triggers to make the recalculation,
-// but it's likely that will change in the future and then we can add to this.
-
 add_task(async function test() {
   // test recalc_frecency is set to 1 when frecency of a page changes.
   // Add a couple visits, then remove one of them.
@@ -27,8 +23,8 @@ add_task(async function test() {
     await PlacesTestUtils.getDatabaseValue("moz_origins", "recalc_frecency", {
       host,
     }),
-    0,
+    1,
-    "Should have been calculated already"
+    "Frecency should be calculated"
   );
   Assert.equal(
     await PlacesTestUtils.getDatabaseValue(
@@ -39,7 +35,7 @@ add_task(async function test() {
       }
     ),
     1,
-    "Should have been calculated already"
+    "Alt frecency should be calculated"
   );
   await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   let alt_frecency = await PlacesTestUtils.getDatabaseValue(
@@ -94,14 +90,12 @@ add_task(async function test() {
   await PlacesTestUtils.addVisits(url2);
   // Remove the first page.
   await PlacesUtils.history.remove(url);
-  // Note common frecency is currently calculated by a trigger, this will change
-  // in the future to use a delayed recalc.
   Assert.equal(
     await PlacesTestUtils.getDatabaseValue("moz_origins", "recalc_frecency", {
       host,
     }),
-    0,
+    1,
-    "Should have been recalculated"
+    "Frecency should be calculated"
   );
   Assert.equal(
     await PlacesTestUtils.getDatabaseValue(
@@ -110,6 +104,6 @@ add_task(async function test() {
       { host }
     ),
     1,
-    "Should request recalculation"
+    "Alt frecency should be calculated"
   );
 });

toolkit/components/places/tests/unit/test_frecency_recalculator.js

@@ -49,7 +49,6 @@ async function addVisitsAndSetRecalc(urls) {
          )`,
         urls
       );
-      await db.executeCached(`DELETE FROM moz_updateoriginsupdate_temp`);
       await db.executeCached(
         `UPDATE moz_places
          SET recalc_frecency = (CASE WHEN url in (
@@ -92,7 +91,10 @@ add_task(async function test() {
     PlacesFrecencyRecalculator.isRecalculationPending,
     "Recalculation should be pending"
   );
-  await PlacesFrecencyRecalculator.recalculateSomeFrecencies({ chunkSize: 2 });
+  // Recalculating uri1 will set its origin to recalc, that means there's 2
+  // origins to recalc now. Passing chunkSize: 2 here would then retrigger the
+  // recalc, thinking we saturated the chunk, thus we use 3.
+  await PlacesFrecencyRecalculator.recalculateSomeFrecencies({ chunkSize: 3 });
   Assert.ok(
     !PlacesFrecencyRecalculator.isRecalculationPending,
     "Recalculation should not be pending"

toolkit/components/places/tests/unit/test_metadata.js

@@ -251,3 +251,35 @@ add_task(async function test_metadata_unparsable() {
 
   await PlacesTestUtils.clearMetadata();
 });
+
+add_task(async function test_metadata_setMany() {
+  await PlacesUtils.metadata.setMany(
+    new Map([
+      ["test/string", "hi"],
+      ["test/boolean", true],
+    ])
+  );
+  await PlacesUtils.metadata.set("test/string", "hi");
+  Assert.deepEqual(
+    await PlacesUtils.metadata.get("test/string"),
+    "hi",
+    "Should store new string value"
+  );
+  Assert.deepEqual(
+    await PlacesUtils.metadata.get("test/boolean"),
+    true,
+    "Should store new boolean value"
+  );
+  await PlacesUtils.metadata.cache.clear();
+  Assert.equal(
+    await PlacesUtils.metadata.get("test/string"),
+    "hi",
+    "Should return string value after clearing cache"
+  );
+  Assert.deepEqual(
+    await PlacesUtils.metadata.get("test/boolean"),
+    true,
+    "Should store new boolean value"
+  );
+  await PlacesTestUtils.clearMetadata();
+});

toolkit/components/places/tests/unit/test_origins.js

@@ -880,18 +880,15 @@ add_task(async function addRemoveBookmarks() {
       })
     );
   }
-  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   await checkDB([
     ["http://", "example.com", ["http://example.com/"]],
     ["http://", "www.example.com", ["http://www.example.com/"]],
   ]);
   await PlacesUtils.bookmarks.remove(bookmarks[0]);
   await PlacesUtils.history.clear();
-  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   await checkDB([["http://", "www.example.com", ["http://www.example.com/"]]]);
   await PlacesUtils.bookmarks.remove(bookmarks[1]);
   await PlacesUtils.history.clear();
-  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   await checkDB([]);
   await cleanUp();
 });
@@ -908,7 +905,6 @@ add_task(async function changeBookmarks() {
       })
     );
   }
-  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   await checkDB([
     ["http://", "example.com", ["http://example.com/"]],
     ["http://", "www.example.com", ["http://www.example.com/"]],
@@ -918,7 +914,6 @@ add_task(async function changeBookmarks() {
     guid: bookmarks[0].guid,
   });
   await PlacesUtils.history.clear();
-  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   await checkDB([["http://", "www.example.com", ["http://www.example.com/"]]]);
   await cleanUp();
 });
@@ -978,7 +973,6 @@ add_task(async function moreOriginFrecencyStats() {
     title: "A bookmark",
     url: NetUtil.newURI("http://example.com/1"),
   });
-  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   await checkDB([
     [
       "http://",
@@ -1002,7 +996,6 @@ add_task(async function moreOriginFrecencyStats() {
   // contributing to the frecency stats.
   await PlacesUtils.bookmarks.remove(bookmark);
   await PlacesUtils.history.remove("http://example.com/1");
-  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
   await checkDB([["http://", "example.com", ["http://example.com/0"]]]);
 
   // Remove URL 0.
@@ -1044,9 +1037,7 @@ async function expectedOriginFrecency(urls) {
  *        this element can be `undefined`.
  */
 async function checkDB(expectedOrigins) {
-  // Frencencies for bookmarks are generated asynchronously but not within the
+  await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
-  // await cycle for bookmarks.insert() etc, so wait for them to happen.
-  await PlacesTestUtils.promiseAsyncUpdates();
 
   let db = await PlacesUtils.promiseDBConnection();
   let rows = await db.execute(`