Bug 1350377 - Implement `History.fetch` and accordingly change `promisePlaceInfo`, r=mak draft
authormilindl <i.milind.luthra@gmail.com>
Mon, 15 May 2017 19:37:19 +0530
changeset 581158 33a7d0d6f3ddbee5e37c6d66cc513ad662362bd6
parent 577554 e66dedabe582ba7b394aee4f89ed70fe389b3c46
child 581159 78a4381587e040bec8ba5e51d1c5b5a67e70897e
push id59789
push userbmo:i.milind.luthra@gmail.com
push dateFri, 19 May 2017 10:34:14 +0000
reviewersmak
bugs1350377
milestone55.0a1
Bug 1350377 - Implement `History.fetch` and accordingly change `promisePlaceInfo`, r=mak `fetch` was written from scratch to query the database and get information for a single page to replace `getPlacesInfo`. A parameter for visits was also added to the signature (default by false) since it's feasible to get the visits for a single page. `promisePlaceInfo` was marked deprecated and turned into a wrapper for fetch. MozReview-Commit-ID: 9ZYHOmrpCHg
toolkit/components/places/History.jsm
toolkit/components/places/PlacesUtils.jsm
--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -109,33 +109,53 @@ function notify(observers, notification,
     } catch (ex) {}
   }
 }
 
 this.History = Object.freeze({
   /**
    * Fetch the available information for one page.
    *
-   * @param guidOrURI: (URL or nsIURI)
-   *      The full URI of the page.
-   *            or (string)
+   * @param guidOrURI: (string) or (URL, nsIURI or href)
    *      Either the full URI of the page or the GUID of the page.
+   * @param [optional] options (object)
+   *      An optional object whose properties describe options:
+   *        - `includeVisits` (boolean) set this to true if `visits` in the
+   *           PageInfo needs to contain VisitInfo in a reverse chronological order.
+   *           By default, `visits` is undefined inside the returned `PageInfo`.
    *
    * @return (Promise)
    *      A promise resolved once the operation is complete.
    * @resolves (PageInfo | null) If the page could be found, the information
-   *      on that page. Note that this `PageInfo` does NOT contain the visit
-   *      data (i.e. `visits` is `undefined`).
+   *      on that page.
+   * @note the VisitInfo objects returned while fetching visits do not
+   *       contain the property `referrer`.
+   *       TODO: Add `referrer` to VisitInfo. See Bug #1365913.
+   * @note the visits returned will not contain `TRANSITION_EMBED` visits.
    *
    * @throws (Error)
    *      If `guidOrURI` does not have the expected type or if it is a string
    *      that may be parsed neither as a valid URL nor as a valid GUID.
    */
-  fetch(guidOrURI) {
-    throw new Error("Method not implemented");
+  fetch(guidOrURI, options = {}) {
+    // First, normalize to guid or string, and throw if not possible
+    guidOrURI = PlacesUtils.normalizeToURLOrGUID(guidOrURI);
+
+    // See if options exists and make sense
+    if (!options || typeof options !== "object") {
+      throw new TypeError("options should be an object and not null");
+    }
+
+    let hasIncludeVisits = "includeVisits" in options;
+    if (hasIncludeVisits && typeof options.includeVisits !== "boolean") {
+      throw new TypeError("includeVisits should be a boolean if exists");
+    }
+
+    return PlacesUtils.promiseDBConnection()
+                      .then(db => fetch(db, guidOrURI, options));
   },
 
   /**
    * Adds a number of visits for a single page.
    *
    * Any change may be observed through nsINavHistoryObserver
    *
    * @param pageInfo: (PageInfo)
@@ -814,16 +834,70 @@ var notifyOnResult = async function(data
     if (++notifiedCount % ONRESULT_CHUNK_SIZE == 0) {
       // Every few notifications, yield time back to the main
       // thread to avoid jank.
       await Promise.resolve();
     }
   }
 };
 
+// Inner implementation of History.fetch.
+var fetch = async function(db, guidOrURL, options) {
+  let whereClauseFragment = "";
+  let params = {};
+  if (guidOrURL instanceof URL) {
+    whereClauseFragment = "WHERE h.url_hash = hash(:url) AND h.url = :url";
+    params.url = guidOrURL.href;
+  } else {
+    whereClauseFragment = "WHERE h.guid = :guid";
+    params.guid = guidOrURL;
+  }
+
+  let visitSelectionFragment = "";
+  let joinFragment = "";
+  let visitOrderFragment = ""
+  if (options.includeVisits) {
+    visitSelectionFragment = ", v.visit_date, v.visit_type";
+    joinFragment = "JOIN moz_historyvisits v ON h.id = v.place_id";
+    visitOrderFragment = "ORDER BY v.visit_date DESC";
+  }
+
+  let query = `SELECT h.id, guid, url, title, frecency ${visitSelectionFragment}
+               FROM moz_places h ${joinFragment}
+               ${whereClauseFragment}
+               ${visitOrderFragment}`;
+  let pageInfo = null;
+  await db.executeCached(
+    query,
+    params,
+    row => {
+      if (pageInfo === null) {
+        // This means we're on the first row, so we need to get all the page info.
+        pageInfo = {
+          guid: row.getResultByName("guid"),
+          url: new URL(row.getResultByName("url")),
+          frecency: row.getResultByName("frecency"),
+          title: row.getResultByName("title") || ""
+        };
+      }
+      if (options.includeVisits) {
+        // On every row (not just the first), we need to collect visit data.
+        if (!("visits" in pageInfo)) {
+          pageInfo.visits = [];
+        }
+        let date = PlacesUtils.toDate(row.getResultByName("visit_date"));
+        let transition = row.getResultByName("visit_type");
+
+        // TODO: Bug #1365913 add referrer URL to the `VisitInfo` data as well.
+        pageInfo.visits.push({ date, transition });
+      }
+    });
+  return pageInfo;
+};
+
 // Inner implementation of History.removeVisitsByFilter.
 var removeVisitsByFilter = async function(db, filter, onResult = null) {
   // 1. Determine visits that took place during the interval.  Note
   // that the database uses microseconds, while JS uses milliseconds,
   // so we need to *1000 one way and /1000 the other way.
   let conditions = [];
   let args = {};
   if ("beginDate" in filter) {
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -1643,38 +1643,29 @@ this.PlacesUtils = {
 
         resolve(charset);
       });
 
     });
   },
 
   /**
-   * Promised wrapper for mozIAsyncHistory::getPlacesInfo for a single place.
+   * Deprecated wrapper for History.jsm::fetch.
    *
    * @param aPlaceIdentifier
-   *        either an nsIURI or a GUID (@see getPlacesInfo)
-   * @resolves to the place info object handed to handleResult.
+   *        either an URL or a GUID (@see History.jsm::fetch)
+   * @return {Promise}.
+   * @resolve a PageInfo
+   * @reject if there is an error in the place identifier
    */
-  promisePlaceInfo: function PU_promisePlaceInfo(aPlaceIdentifier) {
-    return new Promise((resolve, reject) => {
-      PlacesUtils.asyncHistory.getPlacesInfo(aPlaceIdentifier, {
-        _placeInfo: null,
-        handleResult: function handleResult(aPlaceInfo) {
-          this._placeInfo = aPlaceInfo;
-        },
-        handleError: function handleError(aResultCode, aPlaceInfo) {
-          reject(new Components.Exception("Error", aResultCode));
-        },
-        handleCompletion() {
-          resolve(this._placeInfo);
-        }
-      });
-
-    });
+  promisePlaceInfo(aPlaceIdentifier) {
+    Deprecated.warning(`PlacesUtils.promisePlaceInfo() is deprecated.
+                        Please use PlacesUtils.history.fetch()`,
+                       "https://bugzilla.mozilla.org/show_bug.cgi?id=1350377");
+    return PlacesUtils.history.fetch(aPlaceIdentifier);
   },
 
   /**
    * Gets favicon data for a given page url.
    *
    * @param aPageUrl url of the page to look favicon for.
    * @resolves to an object representing a favicon entry, having the following
    *           properties: { uri, dataLen, data, mimeType }