Bug 1252224 - Remove synchronous layout flushes and style invalidations during the opening of multiview panels. r=mconley

MozReview-Commit-ID: 9T4gAPwFrXp --HG-- extra : rebase_source : fb476e4ced4ddb576d10703392fc9dfd1bb03f81
2016-08-04 17:30:54 -04:00 · 2016-08-04 17:30:54 -04:00 · 1e5fb3c3cb
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@ -7231,6 +7231,8 @@ var gIdentityHandler = {
    if (event.target == this._identityPopup) {
      window.addEventListener("focus", this, true);
    }
+    this._identityPopupMultiView._mainView.style.height =
+      this._identityPopup.getBoundingClientRect().height + "px";
  },

  onPopupHidden(event) {
--- a/browser/components/customizableui/content/panelUI.xml
+++ b/browser/components/customizableui/content/panelUI.xml
@ -58,8 +58,7 @@
      <field name="_anchorElement">null</field>
      <field name="_mainViewHeight">0</field>
      <field name="_subViewObserver">null</field>
-      <field name="__transitioning">false</field>
-      <field name="_ignoreMutations">false</field>
+      <field name="__transitioning">true</field>

      <property name="showingSubView" readonly="true"
                onget="return this._viewStack.getAttribute('viewtype') == 'subview'"/>
@ -69,22 +68,6 @@
      <property name="showingSubViewAsMainView" readonly="true"
                onget="return this.getAttribute('mainViewIsSubView') == 'true'"/>

-      <property name="ignoreMutations">
-        <getter>
-          return this._ignoreMutations;
-        </getter>
-        <setter><![CDATA[
-          this._ignoreMutations = val;
-          if (!val && this._panel.state == "open") {
-            if (this.showingSubView) {
-              this._syncContainerWithSubView();
-            } else {
-              this._syncContainerWithMainView();
-            }
-          }
-        ]]></setter>
-      </property>
-
      <property name="_transitioning">
        <getter>
          return this.__transitioning;
@ -223,7 +206,10 @@
          let container = this._viewContainer;
          this._transitioning = true;

-          let onTransitionEnd = () => {
+          let onTransitionEnd = (event) => {
+            if (event.propertyName != "transform") {
+              return;
+            }
            container.removeEventListener("transitionend", onTransitionEnd);
            this._transitioning = false;
          };
@ -296,7 +282,6 @@
              }
              break;
            case "popupshowing":
-              this.setAttribute("panelopen", "true");
              // Bug 941196 - The panel can get taller when opening a subview. Disabling
              // autoPositioning means that the panel won't jump around if an opened
              // subview causes the panel to exceed the dimensions of the screen in the
@ -312,9 +297,18 @@
                subtree: true
              });

-              break;
-            case "popupshown":
-              this._setMaxHeight();
+              let onTransitionEnd = (event) => {
+                if (event.propertyName != "transform") {
+                  return;
+                }
+                let panel = event.target;
+                panel.removeEventListener("tranitionend", onTransitionEnd);
+                // Needed in case the panel is closed before the transition ends.
+                if (panel.state == "open") {
+                  this.setAttribute("panelopen", "true");
+                }
+              };
+              this._panel.addEventListener("transitionend", onTransitionEnd);
              break;
            case "popuphidden":
              this.removeAttribute("panelopen");
@ -338,22 +332,9 @@
        ]]></body>
      </method>

-      <method name="_setMaxHeight">
-        <body><![CDATA[
-          if (!this._shouldSetHeight())
-            return;
-
-          // Ignore the mutation that'll fire when we set the height of
-          // the main view.
-          this.ignoreMutations = true;
-          this._mainView.style.height =
-            this.getBoundingClientRect().height + "px";
-          this.ignoreMutations = false;
-        ]]></body>
-      </method>
      <method name="_adjustContainerHeight">
        <body><![CDATA[
-          if (!this.ignoreMutations && !this.showingSubView && !this._transitioning) {
+          if (!this.showingSubView && !this._transitioning) {
            let height;
            if (this.showingSubViewAsMainView) {
              height = this._heightOfSubview(this._mainView);
@ -371,7 +352,7 @@
            return;
          }

-          if (!this.ignoreMutations && this.showingSubView) {
+          if (this.showingSubView) {
            let newHeight = this._heightOfSubview(this._currentSubView, this._subViews);
            this._viewContainer.style.height = newHeight + "px";
          }
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@ -149,7 +149,8 @@ skip-if = os == "mac"
 [browser_1096763_seen_widgets_post_reset.js]
 [browser_1161838_inserted_new_default_buttons.js]
 [browser_bootstrapped_custom_toolbar.js]
+[browser_check_tooltips_in_navbar.js]
 [browser_customizemode_contextmenu_menubuttonstate.js]
+[browser_no_mutationrecords_during_panel_opening.js]
 [browser_panel_toggle.js]
 [browser_switch_to_customize_mode.js]
-[browser_check_tooltips_in_navbar.js]
--- a/browser/components/customizableui/test/browser_no_mutationrecords_during_panel_opening.js
+++ b/browser/components/customizableui/test/browser_no_mutationrecords_during_panel_opening.js
@ -0,0 +1,79 @@
+/* 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/. */
+
+"use strict";
+
+/**
+ * Test that we don't get unexpected mutations during the opening of the
+ * browser menu.
+ */
+
+add_task(function* test_setup() {
+  yield resetCustomization();
+  yield PanelUI.show();
+  let hiddenPromise = promisePanelHidden(window);
+  PanelUI.hide();
+  yield hiddenPromise;
+});
+
+add_task(function* no_mutation_events_during_opening() {
+  let panel = PanelUI.panel;
+  yield PanelUI.ensureReady();
+
+  let failures = 0;
+  let observer = new MutationObserver(function(mutations) {
+    for (let mutation of mutations) {
+      if (mutation.target.localName == "panel" &&
+          mutation.type == "attributes" &&
+          mutation.attributeName == "animate") {
+        // This mutation is allowed because it triggers the CSS transition.
+        continue;
+      }
+
+      if (mutation.type == "attributes" &&
+          mutation.attributeName == "panelopen") {
+        // This mutation is allowed because it is set after the panel has
+        // finished the transition.
+        continue;
+      }
+
+      let newValue = null;
+      if (mutation.type == "attributes") {
+        newValue = mutation.target.getAttribute(mutation.attributeName);
+      } else if (mutation.type == "characterData") {
+        newValue = mutation.target.textContent;
+      }
+
+      if (newValue == mutation.oldValue) {
+        // Mutations records are observed even when the new and old value are
+        // identical. This is unlikely to invalidate the panel, so ignore these.
+        continue;
+      }
+
+      let nodeIdentifier = `${mutation.target.localName}#${mutation.target.id}.${mutation.target.className};`;
+      ok(false, `Observed: ${mutation.type}; ${nodeIdentifier} ${mutation.attributeName}; oldValue: ${mutation.oldValue}; newValue: ${newValue}`);
+      failures++;
+    }
+  });
+  observer.observe(panel, {
+    childList: true,
+    attributes: true,
+    characterData: true,
+    subtree: true,
+    attributeOldValue: true,
+    characterDataOldValue: true,
+  });
+  let shownPromise = promisePanelShown(window);
+  PanelUI.show();
+  yield shownPromise;
+  observer.disconnect();
+
+  is(failures, 0, "There should be no unexpected mutation events during opening of the panel");
+});
+
+add_task(function* cleanup() {
+  let hiddenPromise = promisePanelHidden(window);
+  PanelUI.hide();
+  yield hiddenPromise;
+});
--- a/toolkit/content/widgets/popup.xml
+++ b/toolkit/content/widgets/popup.xml
@ -425,7 +425,9 @@
          } else {
            arrowbox.pack = "start";
          }
-          arrowbox.style.transform = "translate(0, " + -offset + "px)";
+          if (offset != "0") {
+            arrowbox.style.transform = "translate(0, " + -offset + "px)";
+          }

          // The assigned side stays the same regardless of direction.
          var isRTL = (window.getComputedStyle(this).direction == "rtl");
@ -447,7 +449,9 @@
          } else {
            arrowbox.pack = "start";
          }
-          arrowbox.style.transform = "translate(" + -offset + "px, 0)";
+          if (offset != "0") {
+            arrowbox.style.transform = "translate(" + -offset + "px, 0)";
+          }

          if (position.indexOf("before_") == 0) {
            container.dir = "reverse";