Bug 1568959 - Add notifications for the urlbar results view opening and closing. r=mak

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

--HG--
extra : moz-landing-system : lando

browser/components/search/content/search-one-offs.js

@@ -191,10 +191,16 @@ class SearchOneOffs {
    * @param {UrlbarView} val
    */
   set view(val) {
+    if (this._view) {
+      this._view.controller.removeQueryListener(this);
+    }
     this._view = val;
-    if (val && val.isOpen) {
+    if (val) {
+      if (val.isOpen) {
         this._rebuild();
       }
+      val.controller.addQueryListener(this);
+    }
     return val;
   }
 
@@ -1371,6 +1377,10 @@ class SearchOneOffs {
   _on_popupshowing() {
     this._rebuild();
   }
+
+  onViewOpen() {
+    this._rebuild();
+  }
 }
 
 window.SearchOneOffs = SearchOneOffs;

browser/components/urlbar/UrlbarController.jsm

@@ -22,12 +22,21 @@ XPCOMUtils.defineLazyModuleGetters(this, {
 
 const TELEMETRY_1ST_RESULT = "PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS";
 const TELEMETRY_6_FIRST_RESULTS = "PLACES_AUTOCOMPLETE_6_FIRST_RESULTS_TIME_MS";
+const NOTIFICATIONS = {
+  QUERY_STARTED: "onQueryStarted",
+  QUERY_RESULTS: "onQueryResults",
+  QUERY_RESULT_REMOVED: "onQueryResultRemoved",
+  QUERY_CANCELLED: "onQueryCancelled",
+  QUERY_FINISHED: "onQueryFinished",
+  VIEW_OPEN: "onViewOpen",
+  VIEW_CLOSE: "onViewClose",
+};
 
 /**
  * The address bar controller handles queries from the address bar, obtains
  * results and returns them to the UI for display.
  *
- * Listeners may be added to listen for the results. They must support the
+ * Listeners may be added to listen for the results. They may support the
  * following methods which may be called when a query is run:
  *
  * - onQueryStarted(queryContext)
@@ -35,6 +44,8 @@ const TELEMETRY_6_FIRST_RESULTS = "PLACES_AUTOCOMPLETE_6_FIRST_RESULTS_TIME_MS";
  * - onQueryCancelled(queryContext)
  * - onQueryFinished(queryContext)
  * - onQueryResultRemoved(index)
+ * - onViewOpen()
+ * - onViewClose()
  */
 class UrlbarController {
   /**
@@ -69,6 +80,10 @@ class UrlbarController {
     this.engagementEvent = new TelemetryEvent(options.eventTelemetryCategory);
   }
 
+  get NOTIFICATIONS() {
+    return NOTIFICATIONS;
+  }
+
   /**
    * Hooks up the controller with an input.
    *
@@ -113,7 +128,7 @@ class UrlbarController {
     // For proper functionality we must ensure this notification is fired
     // synchronously, as soon as startQuery is invoked, but after any
     // notifications related to the previous query.
-    this._notify("onQueryStarted", queryContext);
+    this.notify(NOTIFICATIONS.QUERY_STARTED, queryContext);
     await this.manager.startQuery(queryContext, this);
     // If the query has been cancelled, onQueryFinished was notified already.
     // Note this._lastQueryContextWrapper may have changed in the meanwhile.
@@ -124,7 +139,7 @@ class UrlbarController {
       contextWrapper.done = true;
       // TODO (Bug 1549936) this is necessary to avoid leaks in PB tests.
       this.manager.cancelQuery(queryContext);
-      this._notify("onQueryFinished", queryContext);
+      this.notify(NOTIFICATIONS.QUERY_FINISHED, queryContext);
     }
     return queryContext;
   }
@@ -145,8 +160,8 @@ class UrlbarController {
     TelemetryStopwatch.cancel(TELEMETRY_1ST_RESULT, queryContext);
     TelemetryStopwatch.cancel(TELEMETRY_6_FIRST_RESULTS, queryContext);
     this.manager.cancelQuery(queryContext);
-    this._notify("onQueryCancelled", queryContext);
-    this._notify("onQueryFinished", queryContext);
+    this.notify(NOTIFICATIONS.QUERY_CANCELLED, queryContext);
+    this.notify(NOTIFICATIONS.QUERY_FINISHED, queryContext);
   }
 
   /**
@@ -175,7 +190,7 @@ class UrlbarController {
       );
     }
 
-    this._notify("onQueryResults", queryContext);
+    this.notify(NOTIFICATIONS.QUERY_RESULTS, queryContext);
     // Update lastResultCount after notifying, so the view can use it.
     queryContext.lastResultCount = queryContext.results.length;
   }
@@ -210,7 +225,6 @@ class UrlbarController {
    */
   viewContextChanged() {
     this.cancelQuery();
-    this._notify("onViewContextChanged");
   }
 
   /**
@@ -561,7 +575,7 @@ class UrlbarController {
     }
 
     queryContext.results.splice(index, 1);
-    this._notify("onQueryResultRemoved", index);
+    this.notify(NOTIFICATIONS.QUERY_RESULT_REMOVED, index);
 
     PlacesUtils.history
       .remove(selectedResult.payload.url)
@@ -570,12 +584,12 @@ class UrlbarController {
   }
 
   /**
-   * Internal function to notify listeners of results.
+   * Notifies listeners of results.
    *
    * @param {string} name Name of the notification.
    * @param {object} params Parameters to pass with the notification.
    */
-  _notify(name, ...params) {
+  notify(name, ...params) {
     for (let listener of this._listeners) {
       // Can't use "in" because some tests proxify these.
       if (typeof listener[name] != "undefined") {

browser/components/urlbar/UrlbarView.jsm

@@ -200,6 +200,9 @@ class UrlbarView {
    * Closes the view, cancelling the query if necessary.
    */
   close() {
+    if (!this.isOpen) {
+      return;
+    }
     this.controller.cancelQuery();
     this.panel.setAttribute("hidden", "true");
     this.removeAccessibleFocus();
@@ -210,6 +213,7 @@ class UrlbarView {
     this.window.removeEventListener("mousedown", this);
     this.panel.removeEventListener("mousedown", this);
     this.input.textbox.removeEventListener("mousedown", this);
+    this.controller.notify(this.controller.NOTIFICATIONS.VIEW_CLOSE);
   }
 
   // UrlbarController listener methods.
@@ -398,16 +402,14 @@ class UrlbarView {
     this.input.inputField.setAttribute("aria-expanded", "true");
     this.input.dropmarker.setAttribute("open", "true");
 
-    if (this.oneOffSearchButtons.style.display != "none") {
-      this.oneOffSearchButtons._rebuild();
-    }
-
     this.window.addEventListener("mousedown", this);
     this.panel.addEventListener("mousedown", this);
     this.input.textbox.addEventListener("mousedown", this);
 
     this.window.addEventListener("resize", this);
     this._windowOuterWidth = this.window.outerWidth;
+
+    this.controller.notify(this.controller.NOTIFICATIONS.VIEW_OPEN);
   }
 
   /**

browser/components/urlbar/docs/overview.rst

@@ -331,9 +331,10 @@ Represents the base *View* implementation, communicates with the *Controller*.
     // Invoked when the query is done. This is invoked in any case, even if the
     // query was canceled earlier.
     onQueryFinished(queryContext);
-    // Invoked when the view context changed, so that cached information about
-    // the latest search is no more relevant and can be dropped.
-    onViewContextChanged();
+    // Invoked when the view opens.
+    onViewOpen();
+    // Invoked when the view closes.
+    onViewClose();
   }
 
 

browser/components/urlbar/tests/UrlbarTestUtils.jsm

@@ -21,9 +21,9 @@ var UrlbarTestUtils = {
    * @returns {Promise} Resolved when done.
    */
   async promiseSearchComplete(win) {
-    return BrowserTestUtils.waitForCondition(
-      () => win.gURLBar.view.isOpen
-    ).then(() => win.gURLBar.lastQueryContextPromise);
+    return this.promisePopupOpen(win, () => {}).then(
+      () => win.gURLBar.lastQueryContextPromise
+    );
   },
 
   /**
@@ -256,7 +256,17 @@ var UrlbarTestUtils = {
       throw new Error("openFn should be supplied to promisePopupOpen");
     }
     await openFn();
-    return BrowserTestUtils.waitForCondition(() => win.gURLBar.view.isOpen);
+    if (win.gURLBar.view.isOpen) {
+      return;
+    }
+    return new Promise(resolve => {
+      win.gURLBar.controller.addQueryListener({
+        onViewOpen() {
+          win.gURLBar.controller.removeQueryListener(this);
+          resolve();
+        },
+      });
+    });
   },
 
   /**
@@ -272,7 +282,17 @@ var UrlbarTestUtils = {
     } else {
       win.gURLBar.view.close();
     }
-    return BrowserTestUtils.waitForCondition(() => !win.gURLBar.view.isOpen);
+    if (!win.gURLBar.view.isOpen) {
+      return;
+    }
+    return new Promise(resolve => {
+      win.gURLBar.controller.addQueryListener({
+        onViewClose() {
+          win.gURLBar.controller.removeQueryListener(this);
+          resolve();
+        },
+      });
+    });
   },
 
   /**