Bug 834545 - Implement History.remove. r=mak

toolkit/components/places/History.jsm

@@ -14,11 +14,11 @@
  * following properties:
  * - guid: (string)
  *     The globally unique id of the page.
- * - uri: (URL)
+ * - url: (URL)
  *     or (nsIURI)
  *     or (string)
  *     The full URI of the page. Note that `PageInfo` values passed as
- *     argument may hold `nsIURI` or `string` values for property `uri`,
+ *     argument may hold `nsIURI` or `string` values for property `url`,
  *     but `PageInfo` objects returned by this module always hold `URL`
  *     values.
  * - title: (string)
@@ -75,7 +75,33 @@ XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Sqlite",
                                   "resource://gre/modules/Sqlite.jsm");
+XPCOMUtils.defineLazyServiceGetter(this, "gNotifier",
+                                   "@mozilla.org/browser/nav-history-service;1",
+                                   Ci.nsPIPlacesHistoryListenersNotifier);
+XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
+                                  "resource://gre/modules/PlacesUtils.jsm");
+Cu.importGlobalProperties(["URL"]);
 
+/**
+ * Shared connection
+ */
+XPCOMUtils.defineLazyGetter(this, "DBConnPromised",
+  () => new Promise((resolve) => {
+    Sqlite.wrapStorageConnection({ connection: PlacesUtils.history.DBConnection } )
+          .then(db => {
+      try {
+        Sqlite.shutdown.addBlocker("Places History.jsm: Closing database wrapper",
+                                   () => db.close());
+      } catch (ex) {
+        // It's too late to block shutdown of Sqlite, so close the connection
+        // immediately.
+        db.close();
+        throw ex;
+      }
+      resolve(db);
+    });
+  })
+);
 
 this.History = Object.freeze({
   /**
@@ -111,7 +137,7 @@ this.History = Object.freeze({
    *
    * @param infos: (PageInfo)
    *      Information on a page. This `PageInfo` MUST contain
-   *        - either a property `guid` or a property `uri`, as specified
+   *        - either a property `guid` or a property `url`, as specified
    *          by the definition of `PageInfo`;
    *        - a property `visits`, as specified by the definition of
    *          `PageInfo`, which MUST contain at least one visit.
@@ -135,14 +161,14 @@ this.History = Object.freeze({
    *       (i.e. if page entries were updated but not created).
    *
    * @throws (Error)
-   *      If the `uri` specified was for a protocol that should not be
+   *      If the `url` specified was for a protocol that should not be
    *      stored (e.g. "chrome:", "mailbox:", "about:", "imap:", "news:",
    *      "moz-anno:", "view-source:", "resource:", "data:", "wyciwyg:",
    *      "javascript:", "blob:").
    * @throws (Error)
    *      If `infos` has an unexpected type.
    * @throws (Error)
-   *      If a `PageInfo` has neither `guid` nor `uri`,
+   *      If a `PageInfo` has neither `guid` nor `url`.
    * @throws (Error)
    *      If a `guid` property provided is not a valid GUID.
    * @throws (Error)
@@ -177,12 +203,42 @@ this.History = Object.freeze({
    *      A promise resoled once the operation is complete.
    * @resolve (bool)
    *      `true` if at least one page was removed, `false` otherwise.
-   * @throws (Error)
+   * @throws (TypeError)
    *       If `pages` has an unexpected type or if a string provided
-   *       is neither a valid GUID nor a valid URI.
+   *       is neither a valid GUID nor a valid URI or if `pages`
+   *       is an empty array.
    */
-  remove: function (pages, onResult) {
-    throw new Error("Method not implemented");
+  remove: function (pages, onResult = null) {
+    // Normalize and type-check arguments
+    if (Array.isArray(pages)) {
+      if (pages.length == 0) {
+        throw new TypeError("Expected at least one page");
+      }
+    } else {
+      pages = [pages];
+    }
+
+    let guids = [];
+    let urls = [];
+    for (let page of pages) {
+      // Normalize to URL or GUID, or throw if `page` cannot
+      // be normalized.
+      let normalized = normalizeToURLOrGUID(page);
+      if (typeof normalized === "string") {
+        guids.push(normalized);
+      } else {
+        urls.push(normalized.href);
+      }
+    }
+    // At this stage, we know that either `guids` is not-empty
+    // or `urls` is not-empty.
+
+    if (onResult && typeof onResult != "function") {
+      throw new TypeError("Invalid function: " + onResult);
+    }
+
+    // Now perform queries
+    return remove({guids: guids, urls: urls}, onResult);
   },
 
   /**
@@ -257,3 +313,154 @@ this.History = Object.freeze({
   TRANSITION_FRAMED_LINK: Ci.nsINavHistoryService.TRANSITION_FRAMED_LINK,
 });
 
+
+/**
+ * Normalize a key to either a string (if it is a valid GUID) or an
+ * instance of `URL` (if it is a `URL`, `nsIURI`, or a string
+ * representing a valid url).
+ *
+ * @throws (TypeError)
+ *         If the key is neither a valid guid nor a valid url.
+ */
+function normalizeToURLOrGUID(key) {
+  if (typeof key === "string") {
+    // A string may be a URL or a guid
+    if (/^[a-zA-Z0-9\-_]{12}$/.test(key)) {
+      return key;
+    }
+    return new URL(key);
+  }
+  if (key instanceof URL) {
+    return key;
+  }
+  if (key instanceof Ci.nsIURI) {
+    return new URL(key.spec);
+  }
+  throw new TypeError("Invalid url or guid: " + key);
+}
+
+/**
+ * Convert a list of strings or numbers to its SQL
+ * representation as a string.
+ */
+function sqlList(list) {
+  return list.map(JSON.stringify).join();
+}
+
+/**
+ * Invalidate and recompute the frecency of a list of pages,
+ * informing frecency observers.
+ *
+ * @param db: (Sqlite connection)
+ * @param idList: (Array)
+ *      The `moz_places` identifiers for the places to invalidate.
+ * @return (Promise)
+ */
+let invalidateFrecencies = Task.async(function*(db, idList) {
+  if (idList.length == 0) {
+    return;
+  }
+  let ids = sqlList(idList);
+  yield db.execute(
+    `UPDATE moz_places
+     SET frecency = NOTIFY_FRECENCY(
+       CALCULATE_FRECENCY(id), url, guid, hidden, last_visit_date
+     ) WHERE id in (${ ids })`
+  );
+  yield db.execute(
+    `UPDATE moz_places
+     SET hidden = 0
+     WHERE id in (${ ids })
+     AND frecency <> 0`
+  );
+});
+
+
+// Inner implementation of History.remove.
+let remove = Task.async(function*({guids, urls}, onResult = null) {
+  let db = yield DBConnPromised;
+
+  // 1. Find out what needs to be removed
+  let query =
+    `SELECT id, url, guid, foreign_count, title, frecency FROM moz_places
+     WHERE guid IN (${ sqlList(guids) })
+        OR url  IN (${ sqlList(urls)  })
+     `;
+
+  let pages = [];
+  let hasPagesToKeep = false;
+  let hasPagesToRemove = false;
+  yield db.execute(query, null, Task.async(function*(row) {
+    let toRemove = row.getResultByName("foreign_count") == 0;
+    if (toRemove) {
+      hasPagesToRemove = true;
+    } else {
+      hasPagesToKeep = true;
+    }
+    let id = row.getResultByName("id");
+    let guid = row.getResultByName("guid");
+    let url = row.getResultByName("url");
+    let page = {
+      id: id,
+      guid: guid,
+      toRemove: toRemove,
+      uri: NetUtil.newURI(url),
+    };
+    pages.push(page);
+    if (onResult) {
+      let pageInfo = {
+        guid: guid,
+        title: row.getResultByName("title"),
+        frecency: row.getResultByName("frecency"),
+        url: new URL(url)
+      };
+      try {
+        yield onResult(pageInfo);
+      } catch (ex) {
+        // Errors should be reported but should not stop `remove`.
+        Promise.reject(ex);
+      }
+    }
+  }));
+
+  if (pages.length == 0) {
+    // Nothing to do
+    return false;
+  }
+
+  yield db.executeTransaction(function*() {
+    // 2. Remove all visits to these pages.
+    yield db.execute(`DELETE FROM moz_historyvisits
+                      WHERE place_id IN (${ sqlList([p.id for (p of pages)]) })
+                     `);
+
+     // 3. For pages that should not be removed, invalidate frecencies.
+    if (hasPagesToKeep) {
+      yield invalidateFrecencies(db, [p.id for (p of pages) if (!p.toRemove)]);
+    }
+
+    // 4. For pages that should be removed, remove page.
+    if (hasPagesToRemove) {
+      let ids = [p.id for (p of pages) if (p.toRemove)];
+      yield db.execute(`DELETE FROM moz_places
+                        WHERE id IN (${ sqlList(ids) })
+                       `);
+    }
+
+    // 5. Notify observers.
+    for (let {guid, uri, toRemove} of pages) {
+      gNotifier.notifyOnPageExpired(
+        uri, // uri
+        0, // visitTime - There are no more visits
+        toRemove, // wholeEntry
+        guid, // guid
+        Ci.nsINavHistoryObserver.REASON_DELETED, // reason
+        -1 // transition
+      );
+    }
+  });
+
+  PlacesUtils.history.clearEmbedVisits();
+
+  return hasPagesToRemove;
+});

toolkit/components/places/nsINavHistoryService.idl

@@ -1176,7 +1176,7 @@ interface nsINavHistoryQueryOptions : nsISupports
   nsINavHistoryQueryOptions clone();
 };
 
-[scriptable, uuid(4b6963bf-763a-4f39-9fec-25670d354dd9)]
+[scriptable, uuid(47f7b08b-71e0-492e-a2be-9a9fbfc75250)]
 interface nsINavHistoryService : nsISupports
 {
   /**
@@ -1391,6 +1391,11 @@ interface nsINavHistoryService : nsISupports
    * history is disabled if the places.history.enabled pref is false.
    */
   readonly attribute boolean historyDisabled;
+
+  /**
+   * Clear all TRANSITION_EMBED visits.
+   */
+  void clearEmbedVisits();
 };
 
 /**

toolkit/components/places/nsNavHistory.cpp

@@ -3757,6 +3757,12 @@ nsNavHistory::clearEmbedVisits() {
   mEmbedVisits.Clear();
 }
 
+NS_IMETHODIMP
+nsNavHistory::ClearEmbedVisits() {
+  clearEmbedVisits();
+  return NS_OK;
+}
+
 // nsNavHistory::CheckIsRecentEvent
 //
 //    Sees if this URL happened "recently."

toolkit/components/places/tests/moz.build

@@ -16,6 +16,7 @@ XPCSHELL_TESTS_MANIFESTS += [
     'network/xpcshell.ini',
     'queries/xpcshell.ini',
     'unifiedcomplete/xpcshell.ini',
+    'unit/history/xpcshell.ini',
     'unit/xpcshell.ini',
     'xpcshell.ini',
 ]

toolkit/components/places/tests/unit/history/head_history.js

@@ -0,0 +1,19 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+const Ci = Components.interfaces;
+const Cc = Components.classes;
+const Cr = Components.results;
+const Cu = Components.utils;
+
+Cu.import("resource://gre/modules/Services.jsm");
+
+// Import common head.
+let (commonFile = do_get_file("../../head_common.js", false)) {
+  let uri = Services.io.newFileURI(commonFile);
+  Services.scriptloader.loadSubScript(uri.spec, this);
+};
+

toolkit/components/places/tests/unit/history/test_remove.js

@@ -0,0 +1,342 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+// Tests for `History.remove`, as implemented in History.jsm
+
+"use strict";
+
+Cu.importGlobalProperties(["URL"]);
+
+
+// Test removing a single page
+add_task(function* test_remove_single() {
+  let WITNESS_URI = NetUtil.newURI("http://mozilla.com/test_browserhistory/test_remove/" + Math.random());
+  yield promiseAddVisits(WITNESS_URI);
+  Assert.ok(page_in_database(WITNESS_URI));
+
+  let remover = Task.async(function*(name, filter, options) {
+    do_print(name);
+    do_print(JSON.stringify(options));
+    do_print("Setting up visit");
+
+    let uri = NetUtil.newURI("http://mozilla.com/test_browserhistory/test_remove/" + Math.random());
+    let title = "Visit " + Math.random();
+    yield promiseAddVisits({uri: uri, title: title});
+    Assert.ok(visits_in_database(uri), "History entry created");
+
+    let removeArg = yield filter(uri);
+
+    if (options.addBookmark) {
+      PlacesUtils.bookmarks.insertBookmark(
+        PlacesUtils.unfiledBookmarksFolderId,
+        uri,
+        PlacesUtils.bookmarks.DEFAULT_INDEX,
+        "test bookmark");
+    }
+
+    let shouldRemove = !options.addBookmark;
+    let observer;
+    let promiseObserved = new Promise((resolve, reject) => {
+      observer = {
+        onBeginUpdateBatch: function() {},
+        onEndUpdateBatch: function() {},
+        onVisit: function(uri) {
+          reject(new Error("Unexpected call to onVisit " + uri.spec));
+        },
+        onTitleChanged: function(uri) {
+          reject(new Error("Unexpected call to onTitleChanged " + uri.spec));
+        },
+        onClearHistory: function() {
+          reject("Unexpected call to onClearHistory");
+        },
+        onPageChanged: function(uri) {
+          reject(new Error("Unexpected call to onPageChanged " + uri.spec));
+        },
+        onFrecencyChanged: function(aURI) {
+          try {
+            Assert.ok(!shouldRemove, "Observing onFrecencyChanged");
+            Assert.equal(aURI.spec, uri.spec, "Observing effect on the right uri");
+          } finally {
+            resolve();
+          }
+        },
+        onManyFrecenciesChanged: function() {
+          try {
+            Assert.ok(!shouldRemove, "Observing onManyFrecenciesChanged");
+          } finally {
+            resolve();
+          }
+        },
+        onDeleteURI: function(aURI) {
+          try {
+            Assert.ok(shouldRemove, "Observing onDeleteURI");
+            Assert.equal(aURI.spec, uri.spec, "Observing effect on the right uri");
+          } finally {
+            resolve();
+          }
+        },
+        onDeleteVisits: function(aURI) {
+          Assert.equal(aURI.spec, uri.spec, "Observing onDeleteVisits on the right uri");
+        }
+      };
+    });
+    PlacesUtils.history.addObserver(observer, false);
+
+    do_print("Performing removal");
+    let removed = false;
+    if (options.useCallback) {
+      let onRowCalled = false;
+      removed = yield PlacesUtils.history.remove(removeArg, page => {
+        Assert.equal(onRowCalled, false, "Callback has not been called yet");
+        onRowCalled = true;
+        Assert.equal(page.url.href, uri.spec, "Callback provides the correct url");
+        Assert.equal(page.guid, do_get_guid_for_uri(uri), "Callback provides the correct guid");
+        Assert.equal(page.title, title, "Callback provides the correct title");
+        Assert.equal(page.frecency, frecencyForUrl(uri), "Callback provides the correct frecency");
+      });
+      Assert.ok(onRowCalled, "Callback has been called");
+    } else {
+      removed = yield PlacesUtils.history.remove(removeArg);
+    }
+
+    yield promiseObserved;
+    PlacesUtils.history.removeObserver(observer);
+
+    Assert.equal(visits_in_database(uri), 0, "History entry has disappeared");
+    Assert.notEqual(visits_in_database(WITNESS_URI), 0, "Witness URI still has visits");
+    Assert.notEqual(page_in_database(WITNESS_URI), 0, "Witness URI is still here");
+    if (shouldRemove) {
+      Assert.ok(removed, "Something was removed");
+      Assert.equal(page_in_database(uri), 0, "Page has disappeared");
+    } else {
+      Assert.ok(!removed, "The page was not removed, as there was a bookmark");
+      Assert.notEqual(page_in_database(uri), 0, "The page is still present");
+    }
+  });
+
+  try {
+    for (let useCallback of [false, true]) {
+      for (let addBookmark of [false, true]) {
+        let options = { useCallback: useCallback, addBookmark: addBookmark };
+        yield remover("Testing History.remove() with a single URI", x => x, options);
+        yield remover("Testing History.remove() with a single string url", x => x.spec, options);
+        yield remover("Testing History.remove() with a single string guid", x => do_get_guid_for_uri(x), options);
+        yield remover("Testing History.remove() with a single URI in an array", x => [x], options);
+        yield remover("Testing History.remove() with a single string url in an array", x => [x.spec], options);
+        yield remover("Testing History.remove() with a single string guid in an array", x => [do_get_guid_for_uri(x)], options);
+      }
+    }
+  } finally {
+    yield promiseClearHistory();
+  }
+  return;
+});
+
+// Test removing a list of pages
+add_task(function* test_remove_many() {
+  const SIZE = 10;
+
+  do_print("Adding a witness page");
+  let WITNESS_URI = NetUtil.newURI("http://mozilla.com/test_browserhistory/test_remove/" + Math.random());;
+  yield promiseAddVisits(WITNESS_URI);
+  Assert.ok(page_in_database(WITNESS_URI), "Witness page added");
+
+  do_print("Generating samples");
+  let pages = [];
+  for (let i = 0; i < SIZE; ++i) {
+    let uri = NetUtil.newURI("http://mozilla.com/test_browserhistory/test_remove?sample=" + i + "&salt=" + Math.random());
+    let title = "Visit " + i + ", " + Math.random();
+    let hasBookmark = i % 3 == 0;
+    let resolve;
+    let page = {
+      uri: uri,
+      title: title,
+      hasBookmark: hasBookmark,
+      // `true` once `onResult` has been called for this page
+      onResultCalled: false,
+      // `true` once `onDeleteVisits` has been called for this page
+      onDeleteVisitsCalled: false,
+      // `true` once `onFrecencyChangedCalled` has been called for this page
+      onFrecencyChangedCalled: false,
+      // `true` once `onDeleteURI` has been called for this page
+      onDeleteURICalled: false,
+    };
+    do_print("Pushing: " + uri.spec);
+    pages.push(page);
+
+    yield promiseAddVisits(page);
+    page.guid = do_get_guid_for_uri(uri);
+    if (hasBookmark) {
+      PlacesUtils.bookmarks.insertBookmark(
+        PlacesUtils.unfiledBookmarksFolderId,
+        uri,
+        PlacesUtils.bookmarks.DEFAULT_INDEX,
+        "test bookmark " + i);
+    }
+    Assert.ok(page_in_database(uri), "Page added");
+  }
+
+  do_print("Mixing key types and introducing dangling keys");
+  let keys = [];
+  for (let i = 0; i < SIZE; ++i) {
+    if (i % 4 == 0) {
+      keys.push(pages[i].uri);
+      keys.push(NetUtil.newURI("http://example.org/dangling/nsIURI/" + i));
+    } else if (i % 4 == 1) {
+      keys.push(new URL(pages[i].uri.spec));
+      keys.push(new URL("http://example.org/dangling/URL/" + i));
+    } else if (i % 4 == 2) {
+      keys.push(pages[i].uri.spec);
+      keys.push("http://example.org/dangling/stringuri/" + i);
+    } else {
+      keys.push(pages[i].guid);
+      keys.push(("guid_" + i + "_01234567890").substr(0, 12));
+    }
+  }
+
+  let observer = {
+    onBeginUpdateBatch: function() {},
+    onEndUpdateBatch: function() {},
+    onVisit: function(aURI) {
+      Assert.ok(false, "Unexpected call to onVisit " + aURI.spec);
+    },
+    onTitleChanged: function(aURI) {
+      Assert.ok(false, "Unexpected call to onTitleChanged " + aURI.spec);
+    },
+    onClearHistory: function() {
+      Assert.ok(false, "Unexpected call to onClearHistory");
+    },
+    onPageChanged: function(aURI) {
+      Assert.ok(false, "Unexpected call to onPageChanged " + aURI.spec);
+    },
+    onFrecencyChanged: function(aURI) {
+      let origin = pages.find(x =>  x.uri.spec == aURI.spec);
+      Assert.ok(origin);
+      Assert.ok(origin.hasBookmark, "Observing onFrecencyChanged on a page with a bookmark");
+      origin.onFrecencyChangedCalled = true;
+      // We do not make sure that `origin.onFrecencyChangedCalled` is `false`, as 
+    },
+    onManyFrecenciesChanged: function() {
+      Assert.ok(false, "Observing onManyFrecenciesChanges, this is most likely correct but not covered by this test");
+    },
+    onDeleteURI: function(aURI) {
+      let origin = pages.find(x => x.uri.spec == aURI.spec);
+      Assert.ok(origin);
+      Assert.ok(!origin.hasBookmark, "Observing onDeleteURI on a page without a bookmark");
+      Assert.ok(!origin.onDeleteURICalled, "Observing onDeleteURI for the first time");
+      origin.onDeleteURICalled = true;
+    },
+    onDeleteVisits: function(aURI) {
+      let origin = pages.find(x => x.uri.spec == aURI.spec);
+      Assert.ok(origin);
+      Assert.ok(!origin.onDeleteVisitsCalled, "Observing onDeleteVisits for the first time");
+      origin.onDeleteVisitsCalled = true;
+    }
+  };
+  PlacesUtils.history.addObserver(observer, false);
+
+  do_print("Removing the pages and checking the callbacks");
+  let removed = yield PlacesUtils.history.remove(keys, page => {
+    let origin = pages.find(candidate => candidate.uri.spec == page.url.href);
+
+    Assert.ok(origin, "onResult has a valid page");
+    Assert.ok(!origin.onResultCalled, "onResult has not seen this page yet");
+    origin.onResultCalled = true;
+    Assert.equal(page.guid, origin.guid, "onResult has the right guid");
+    Assert.equal(page.title, origin.title, "onResult has the right title");
+  });
+  Assert.ok(removed, "Something was removed");
+
+  PlacesUtils.history.removeObserver(observer);
+
+  do_print("Checking out results");
+  // By now the observers should have been called.
+  for (let i = 0; i < pages.length; ++i) {
+    let page = pages[i];
+    do_print("Page: " + i);
+    Assert.ok(page.onResultCalled, "We have reached the page from the callback");
+    Assert.ok(visits_in_database(page.uri) == 0, "History entry has disappeared");
+    Assert.equal(page_in_database(page.uri) != 0, page.hasBookmark, "Page is present only if it also has bookmarks");
+    Assert.equal(page.onFrecencyChangedCalled, page.onDeleteVisitsCalled, "onDeleteVisits was called iff onFrecencyChanged was called");
+    Assert.ok(page.onFrecencyChangedCalled ^ page.onDeleteURICalled, "Either onFrecencyChanged or onDeleteURI was called");
+  }
+
+  Assert.notEqual(visits_in_database(WITNESS_URI), 0, "Witness URI still has visits");
+  Assert.notEqual(page_in_database(WITNESS_URI), 0, "Witness URI is still here");
+
+  do_print("Cleaning up");
+  yield promiseClearHistory();
+
+});
+
+// Test the various error cases
+add_task(function* test_error_cases() {
+  Assert.throws(
+    () =>  PlacesUtils.history.remove(),
+    /TypeError: Invalid url/,
+    "History.remove with no argument should throw a TypeError"
+  );
+  Assert.throws(
+    () =>  PlacesUtils.history.remove(null),
+    /TypeError: Invalid url/,
+    "History.remove with `null` should throw a TypeError"
+  );
+  Assert.throws(
+    () =>  PlacesUtils.history.remove(undefined),
+    /TypeError: Invalid url/,
+    "History.remove with `undefined` should throw a TypeError"
+  );
+  Assert.throws(
+    () => PlacesUtils.history.remove("not a guid, obviously"),
+    /TypeError: .* is not a valid URL/,
+    "History.remove with an ill-formed guid/url argument should throw a TypeError"
+  );
+  Assert.throws(
+    () => PlacesUtils.history.remove({"not the kind of object we know how to handle": true}),
+    /TypeError: Invalid url/,
+    "History.remove with an unexpected object should throw a TypeError"
+  );
+  Assert.throws(
+    () => PlacesUtils.history.remove([]),
+    /TypeError: Expected at least one page/,
+    "History.remove with an empty array should throw a TypeError"
+  );
+  Assert.throws(
+    () => PlacesUtils.history.remove([null]),
+    /TypeError: Invalid url or guid/,
+    "History.remove with an array containing null should throw a TypeError"
+  );
+  Assert.throws(
+    () => PlacesUtils.history.remove(["http://example.org", "not a guid, obviously"]),
+    /TypeError: .* is not a valid URL/,
+    "History.remove with an array containing an ill-formed guid/url argument should throw a TypeError"
+  );
+  Assert.throws(
+    () => PlacesUtils.history.remove(["0123456789ab"/*valid guid*/, null]),
+    /TypeError: Invalid url or guid: null/,
+    "History.remove with an array containing a guid and a second argument that is null should throw a TypeError"
+  );
+  Assert.throws(
+    () => PlacesUtils.history.remove(["http://example.org", {"not the kind of object we know how to handle": true}]),
+    /TypeError: Invalid url/,
+    "History.remove with an array containing an unexpected objecgt should throw a TypeError"
+  );
+  Assert.throws(
+    () => PlacesUtils.history.remove("http://example.org", "not a function, obviously"),
+    /TypeError: Invalid function/,
+    "History.remove with a second argument that is not a function argument should throw a TypeError"
+  );
+  try {
+    PlacesUtils.history.remove("http://example.org/I/have/clearly/not/been/added", null);
+    Assert.ok(true, "History.remove should ignore `null` as a second argument");
+  } catch (ex) {
+    Assert.ok(false, "History.remove should ignore `null` as a second argument");
+  }
+});
+
+function run_test() {
+  run_next_test();
+}

toolkit/components/places/tests/unit/history/xpcshell.ini

@@ -0,0 +1,5 @@
+[DEFAULT]
+head = head_history.js
+tail =
+
+[test_remove.js]