Bug 1312999 - Cache one-off buttons instead of regenerating them at every popupopen, r=adw.
authorFlorian Queze <florian@queze.net>
Wed, 19 Apr 2017 00:04:07 +0200
changeset 401877 6187bdf62907ca7e6372592752da45f00178d33e
parent 401876 13fd2a40e7d689b3b4582759972ccdc32e203b68
child 401878 1735bad11778eee56faadccb916c31b9ad9e0be5
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1312999
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1312999 - Cache one-off buttons instead of regenerating them at every popupopen, r=adw.
browser/components/search/content/search.xml
--- a/browser/components/search/content/search.xml
+++ b/browser/components/search/content/search.xml
@@ -1220,17 +1220,17 @@
                       label="&searchInNewTab.label;"
                       accesskey="&searchInNewTab.accesskey;"/>
         <xul:menuitem anonid="search-one-offs-context-set-default"
                       label="&searchSetAsDefault.label;"
                       accesskey="&searchSetAsDefault.accesskey;"/>
       </xul:menupopup>
     </content>
 
-    <implementation implements="nsIDOMEventListener">
+    <implementation implements="nsIDOMEventListener,nsIObserver,nsIWeakReference">
 
       <!-- Width in pixels of the one-off buttons.  49px is the min-width of
            each search engine button, adapt this const when changing the css.
            It's actually 48px + 1px of right border. -->
       <property name="buttonWidth" readonly="true" onget="return 49;"/>
 
       <field name="_popup">null</field>
 
@@ -1268,16 +1268,17 @@
           if (val && val.state != "closed") {
             this._rebuild();
           }
           return val;
         ]]></setter>
       </property>
 
       <field name="_textbox">null</field>
+      <field name="_textboxWidth">0</field>
 
       <!-- The textbox associated with the one-offs.  Set this to a textbox to
            automatically keep the related one-offs UI up to date.  Otherwise you
            can leave it null/undefined, and in that case you should update the
            query property manually. -->
       <property name="textbox">
         <getter><![CDATA[
           return this._textbox;
@@ -1410,16 +1411,20 @@
         menu.addEventListener("popupshown", aEvent => {
           this._ignoreMouseEvents = true;
           aEvent.stopPropagation();
         });
         menu.addEventListener("popuphidden", aEvent => {
           this._ignoreMouseEvents = false;
           aEvent.stopPropagation();
         });
+
+        // Add weak referenced observers to invalidate our cached list of engines.
+        Services.prefs.addObserver("browser.search.hiddenOneOffs", this, true);
+        Services.obs.addObserver(this, "browser-search-engine-modified", true);
       ]]></constructor>
 
       <!-- This handles events outside the one-off buttons, like on the popup
            and textbox. -->
       <method name="handleEvent">
         <parameter name="event"/>
         <body><![CDATA[
           switch (event.type) {
@@ -1438,16 +1443,26 @@
                 this.selectedButton = null;
                 this._contextEngine = null;
               });
               break;
           }
         ]]></body>
       </method>
 
+      <method name="observe">
+        <parameter name="aEngine"/>
+        <parameter name="aTopic"/>
+        <parameter name="aData"/>
+        <body><![CDATA[
+          // Make sure the engine list is refetched next time it's needed.
+          this._engines = null;
+        ]]></body>
+      </method>
+
       <method name="showSettings">
         <body><![CDATA[
           BrowserUITelemetry.countSearchSettingsEvent(this.telemetryOrigin);
           openPreferences("general-search");
           // If the preference tab was already selected, the panel doesn't
           // close itself automatically.
           this.popup.hidePopup();
         ]]></body>
@@ -1486,73 +1501,103 @@
             groupText = noSearchHeader.value;
             if (!isOneOffSelected)
               headerPanel.selectedIndex = 0;
           }
           list.setAttribute("aria-label", groupText);
         ]]></body>
       </method>
 
+      <field name="_engines">null</field>
+      <property name="engines" readonly="true">
+        <getter><![CDATA[
+          if (this._engines)
+            return this._engines;
+          let currentEngineNameToIgnore;
+          if (!this.getAttribute("includecurrentengine"))
+            currentEngineNameToIgnore = Services.search.currentEngine.name;
+
+          let pref = Services.prefs.getStringPref("browser.search.hiddenOneOffs");
+          let hiddenList = pref ? pref.split(",") : [];
+
+          this._engines = Services.search.getVisibleEngines().filter(e => {
+            let name = e.name;
+            return (!currentEngineNameToIgnore ||
+                    name != currentEngineNameToIgnore) &&
+                   !hiddenList.includes(name);
+          });
+
+          return this._engines;
+        ]]></getter>
+      </property>
+
       <!-- Builds all the UI. -->
       <method name="_rebuild">
         <body><![CDATA[
           // Update the 'Search for <keywords> with:" header.
           this._updateAfterQueryChanged();
 
-          let list = document.getAnonymousElementByAttribute(this, "anonid",
-                                                             "search-panel-one-offs");
-
           // Handle opensearch items. This needs to be done before building the
           // list of one off providers, as that code will return early if all the
           // alternative engines are hidden.
-          this._rebuildAddEngineList();
+          // Skip this in compact mode, ie. for the urlbar.
+          if (!this.compact)
+            this._rebuildAddEngineList();
 
+          // Check if the one-off buttons really need to be rebuilt.
+          if (this._textbox) {
+            // We can't get a reliable value for the popup width without flushing,
+            // but the popup width won't change if the textbox width doesn't.
+            let DOMUtils = window.QueryInterface(Ci.nsIInterfaceRequestor)
+                                 .getInterface(Ci.nsIDOMWindowUtils);
+            let textboxWidth =
+              DOMUtils.getBoundsWithoutFlushing(this._textbox).width;
+            // We can return early if neither the list of engines nor the panel
+            // width has changed.
+            if (this._engines && this._textboxWidth == textboxWidth) {
+              return;
+            }
+            this._textboxWidth = textboxWidth;
+          }
+
+          let list = document.getAnonymousElementByAttribute(this, "anonid",
+                                                             "search-panel-one-offs");
           let settingsButton =
             document.getAnonymousElementByAttribute(this, "anonid",
                                                     "search-settings-compact");
           // Finally, build the list of one-off buttons.
           while (list.firstChild != settingsButton)
             list.firstChild.remove();
           // Remove the trailing empty text node introduced by the binding's
           // content markup above.
           if (settingsButton.nextSibling)
             settingsButton.nextSibling.remove();
 
-          let Preferences =
-            Cu.import("resource://gre/modules/Preferences.jsm", {}).Preferences;
-          let pref = Preferences.get("browser.search.hiddenOneOffs");
-          let hiddenList = pref ? pref.split(",") : [];
-
-          let currentEngineName = Services.search.currentEngine.name;
-          let includeCurrentEngine = this.getAttribute("includecurrentengine");
-          let engines = Services.search.getVisibleEngines().filter(e => {
-            return (includeCurrentEngine || e.name != currentEngineName) &&
-                   !hiddenList.includes(e.name);
-          });
+          let engines = this.engines;
+          let oneOffCount = engines.length;
 
           let header = document.getAnonymousElementByAttribute(this, "anonid",
                                                                "search-panel-one-offs-header")
           // header is a xul:deck so collapsed doesn't work on it, see bug 589569.
-          header.hidden = list.collapsed = !engines.length;
+          header.hidden = list.collapsed = !oneOffCount;
 
-          if (!engines.length)
+          if (!oneOffCount)
             return;
 
           let panelWidth = parseInt(this.popup.clientWidth);
           // The + 1 is because the last button doesn't have a right border.
           let enginesPerRow = Math.floor((panelWidth + 1) / this.buttonWidth);
           let buttonWidth = Math.floor(panelWidth / enginesPerRow);
           // There will be an emtpy area of:
           //   panelWidth - enginesPerRow * buttonWidth  px
           // at the end of each row.
 
           // If the <description> tag with the list of search engines doesn't have
           // a fixed height, the panel will be sized incorrectly, causing the bottom
           // of the suggestion <tree> to be hidden.
-          let oneOffCount = engines.length;
           if (this.compact)
             ++oneOffCount;
           let rowCount = Math.ceil(oneOffCount / enginesPerRow);
           let height = rowCount * 33; // 32px per row, 1px border.
           list.setAttribute("height", height + "px");
 
           // Ensure we can refer to the settings buttons by ID:
           let settingsEl = document.getAnonymousElementByAttribute(this, "anonid", "search-settings");
@@ -1642,30 +1687,22 @@
         <body><![CDATA[
         let list = document.getAnonymousElementByAttribute(this, "anonid",
                                                            "add-engines");
         while (list.firstChild) {
           list.firstChild.remove();
         }
 
         // Add a button for each engine that the page in the selected browser
-        // offers, but with the following exceptions:
-        //
-        // (1) Not when the one-offs are compact.  Compact one-offs are shown in
-        // the urlbar, and the add-engine buttons span the width of the popup,
-        // so if we added all the engines that a page offers, it could break the
-        // urlbar popup by offering a ton of engines.  We should probably make a
-        // smaller version of the buttons for compact one-offs.
-        //
-        // (2) Not when there are too many offered engines.  The popup isn't
-        // designed to handle too many (by scrolling for example), so a page
-        // could break the popup by offering too many.  Instead, add a single
-        // menu button with a submenu of all the engines.
+        // offers, except when there are too many offered engines.
+        // The popup isn't designed to handle too many (by scrolling for
+        // example), so a page could break the popup by offering too many.
+        // Instead, add a single menu button with a submenu of all the engines.
 
-        if (this.compact || !gBrowser.selectedBrowser.engines) {
+        if (!gBrowser.selectedBrowser.engines) {
           return;
         }
 
         const kXULNS =
           "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
         let engines = gBrowser.selectedBrowser.engines;
         let tooManyEngines = engines.length > this._addEngineMenuThreshold;