Backed out 3 changesets (bug 1468402) for causing perma fails on browser_grids_grid-list-subgrids.js. CLOSED TREE
authorRazvan Maries <rmaries@mozilla.com>
Mon, 13 May 2019 20:21:54 +0300
changeset 535561 0647eae6a6e6abd91ebd206b2db15a6766d2d3ef
parent 535560 6685fd95730257d69b3e9169a1d84c7336a8ce74
child 535563 a0403c2bfae40a8b3cebd532ce730fa824181fee
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1468402
milestone68.0a1
backs out6685fd95730257d69b3e9169a1d84c7336a8ce74
63f40ac370c63ec1aae568a569a25f768998b998
567911d83bda7ad05d88ce584e9a29000fdd4b52
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
Backed out 3 changesets (bug 1468402) for causing perma fails on browser_grids_grid-list-subgrids.js. CLOSED TREE Backed out changeset 6685fd957302 (bug 1468402) Backed out changeset 63f40ac370c6 (bug 1468402) Backed out changeset 567911d83bda (bug 1468402)
devtools/client/inspector/grids/components/GridItem.js
devtools/client/inspector/grids/components/GridList.js
devtools/client/inspector/grids/grid-inspector.js
devtools/client/inspector/grids/test/browser.ini
devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js
devtools/client/inspector/grids/test/doc_subgrid.html
devtools/client/inspector/grids/types.js
devtools/client/themes/layout.css
devtools/server/actors/layout.js
devtools/shared/fronts/layout.js
--- a/devtools/client/inspector/grids/components/GridItem.js
+++ b/devtools/client/inspector/grids/components/GridItem.js
@@ -1,15 +1,15 @@
 /* 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";
 
-const { createElement, createRef, Fragment, PureComponent } = require("devtools/client/shared/vendor/react");
+const { createRef, PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
 loader.lazyGetter(this, "Rep", function() {
   return require("devtools/client/shared/components/reps/reps").REPS.Rep;
 });
 loader.lazyGetter(this, "MODE", function() {
   return require("devtools/client/shared/components/reps/reps").MODE;
@@ -19,17 +19,16 @@ loader.lazyRequireGetter(this, "translat
 
 const Types = require("../types");
 
 class GridItem extends PureComponent {
   static get propTypes() {
     return {
       getSwatchColorPickerTooltip: PropTypes.func.isRequired,
       grid: PropTypes.shape(Types.grid).isRequired,
-      grids: PropTypes.arrayOf(PropTypes.shape(Types.grid)).isRequired,
       onHideBoxModelHighlighter: PropTypes.func.isRequired,
       onSetGridOverlayColor: PropTypes.func.isRequired,
       onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
       onToggleGridHighlighter: PropTypes.func.isRequired,
       setSelectedNode: PropTypes.func.isRequired,
     };
   }
 
@@ -89,52 +88,24 @@ class GridItem extends PureComponent {
   }
 
   onGridInspectIconClick(nodeFront) {
     const { setSelectedNode } = this.props;
     setSelectedNode(nodeFront, { reason: "layout-panel" });
     nodeFront.scrollIntoView().catch(e => console.error(e));
   }
 
-  renderSubgrids() {
-    const { grid, grids } = this.props;
-
-    if (!grid.subgrids.length) {
-      return null;
-    }
-
-    const subgrids = grids.filter(g => grid.subgrids.includes(g.id));
-
-    return (
-      dom.ul({},
-        subgrids.map(g => {
-          return createElement(GridItem, {
-            key: g.id,
-            getSwatchColorPickerTooltip: this.props.getSwatchColorPickerTooltip,
-            grid: g,
-            grids,
-            onHideBoxModelHighlighter: this.props.onHideBoxModelHighlighter,
-            onSetGridOverlayColor: this.props.onSetGridOverlayColor,
-            onShowBoxModelHighlighterForNode: this.props.onShowBoxModelHighlighterForNode,
-            onToggleGridHighlighter: this.props.onToggleGridHighlighter,
-            setSelectedNode: this.props.setSelectedNode,
-          });
-        })
-      )
-    );
-  }
-
   render() {
     const {
       grid,
       onHideBoxModelHighlighter,
       onShowBoxModelHighlighterForNode,
     } = this.props;
 
-    return createElement(Fragment, null,
+    return (
       dom.li({},
         dom.label({},
           dom.input(
             {
               checked: grid.highlighted,
               disabled: grid.disabled,
               type: "checkbox",
               value: grid.id,
@@ -166,15 +137,14 @@ class GridItem extends PureComponent {
         // requirement. See https://bugzilla.mozilla.org/show_bug.cgi?id=1341578
         dom.span(
           {
             className: "layout-color-value",
             ref: this.colorValueEl,
           },
           grid.color
         )
-      ),
-      this.renderSubgrids()
+      )
     );
   }
 }
 
 module.exports = GridItem;
--- a/devtools/client/inspector/grids/components/GridList.js
+++ b/devtools/client/inspector/grids/components/GridList.js
@@ -40,24 +40,20 @@ class GridList extends PureComponent {
     return (
       dom.div({ className: "grid-container" },
         dom.span({}, getStr("layout.overlayGrid")),
         dom.ul(
           {
             id: "grid-list",
             className: "devtools-monospace",
           },
-          grids
-          // Skip subgrids since they are rendered by their parent grids in GridItem.
-          .filter(grid => !grid.isSubgrid)
-          .map(grid => GridItem({
+          grids.map(grid => GridItem({
             key: grid.id,
             getSwatchColorPickerTooltip,
             grid,
-            grids,
             onHideBoxModelHighlighter,
             onSetGridOverlayColor,
             onShowBoxModelHighlighterForNode,
             onToggleGridHighlighter,
             setSelectedNode,
           }))
         )
       )
--- a/devtools/client/inspector/grids/grid-inspector.js
+++ b/devtools/client/inspector/grids/grid-inspector.js
@@ -313,51 +313,33 @@ class GridInspector {
 
       const colorForHost = customColors[hostname] ? customColors[hostname][i] : null;
       const fallbackColor = GRID_COLORS[i % GRID_COLORS.length];
       const color = this.getInitialGridColor(nodeFront, colorForHost, fallbackColor);
       const highlighted = this.highlighters.gridHighlighters.has(nodeFront);
       const disabled = !highlighted &&
                        this.maxHighlighters > 1 &&
                        this.highlighters.gridHighlighters.size === this.maxHighlighters;
-      const isSubgrid = grid.isSubgrid;
-      const gridData = {
+
+      grids.push({
         id: i,
         actorID: grid.actorID,
         color,
         disabled,
         direction: grid.direction,
         gridFragments: grid.gridFragments,
         highlighted,
-        isSubgrid,
         nodeFront,
-        parentNodeActorID: null,
-        subgrids: [],
         writingMode: grid.writingMode,
-      };
-
-      if (isSubgrid) {
-        const parentGridNodeFront = await grid.getParentGridNode(this.walker);
-        if (!parentGridNodeFront) {
-          return;
-        }
-
-        const parentIndex = grids.findIndex(g =>
-          g.nodeFront.actorID === parentGridNodeFront.actorID);
-        gridData.parentNodeActorID = parentGridNodeFront.actorID;
-        grids[parentIndex].subgrids.push(gridData.id);
-      }
-
-      grids.push(gridData);
+      });
     }
 
     this.store.dispatch(updateGrids(grids));
     this.inspector.emit("grid-panel-updated");
   }
-
   /**
    * Handler for "grid-highlighter-shown" events emitted from the
    * HighlightersOverlay. Passes nodefront and event name to handleHighlighterChange.
    * Required since on and off events need the same reference object.
    *
    * @param  {NodeFront} nodeFront
    *         The NodeFront of the grid container element for which the grid
    *         highlighter is shown for.
--- a/devtools/client/inspector/grids/test/browser.ini
+++ b/devtools/client/inspector/grids/test/browser.ini
@@ -1,14 +1,13 @@
 [DEFAULT]
 tags = devtools
 subsuite = devtools
 support-files =
   doc_iframe_reloaded.html
-  doc_subgrid.html
   head.js
   !/devtools/client/inspector/test/head.js
   !/devtools/client/inspector/test/shared-head.js
   !/devtools/client/shared/test/shared-head.js
   !/devtools/client/shared/test/shared-redux-head.js
   !/devtools/client/shared/test/telemetry-test-helpers.js
   !/devtools/client/shared/test/test-actor.js
   !/devtools/client/shared/test/test-actor-registry.js
@@ -21,17 +20,16 @@ support-files =
 [browser_grids_grid-list-color-picker-on-ESC.js]
 [browser_grids_grid-list-color-picker-on-RETURN.js]
 [browser_grids_grid-list-element-rep.js]
 [browser_grids_grid-list-no-grids.js]
 [browser_grids_grid-list-on-iframe-reloaded.js]
 skip-if = (verify && (os == 'win' || os == 'linux'))
 [browser_grids_grid-list-on-mutation-element-added.js]
 [browser_grids_grid-list-on-mutation-element-removed.js]
-[browser_grids_grid-list-subgrids.js]
 [browser_grids_grid-list-toggle-grids_01.js]
 [browser_grids_grid-list-toggle-grids_02.js]
 [browser_grids_grid-list-toggle-multiple-grids.js]
 [browser_grids_grid-outline-cannot-show-outline.js]
 [browser_grids_grid-outline-highlight-area.js]
 skip-if = (verify && (os == 'win')) || (os == "win" && os_version == "10.0" && !debug) #Bug 1501760
 [browser_grids_grid-outline-highlight-cell.js]
 skip-if = (verify && (os == 'win')) || (os == "win" && os_version == "10.0" && asan) #Bug 1501317
deleted file mode 100644
--- a/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js
+++ /dev/null
@@ -1,61 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-// Tests that the list of grids show the subgrids in the correct nested list and toggling
-// the CSS grid highlighter for a subgrid.
-
-const TEST_URI = URL_ROOT + "doc_subgrid.html";
-
-add_task(async function() {
-  await addTab(TEST_URI);
-  const { gridInspector, inspector } = await openLayoutView();
-  const { document: doc } = gridInspector;
-  const { highlighters, store } = inspector;
-
-  await selectNode("#grid", inspector);
-  const gridListEl = doc.getElementById("grid-list");
-  const containerSubgridListEl = gridListEl.children[1];
-  const mainSubgridListEl = containerSubgridListEl.querySelector("ul");
-
-  info("Checking the initial state of the Grid Inspector.");
-  is(getGridItemElements(gridListEl).length, 1, "One grid container is listed.");
-  is(getGridItemElements(containerSubgridListEl).length, 2,
-    "Got the correct number of subgrids in div.container");
-  is(getGridItemElements(mainSubgridListEl).length, 2,
-    "Got the correct number of subgrids in main.subgrid");
-  ok(!highlighters.gridHighlighters.size, "No CSS grid highlighter is shown.");
-
-  info("Toggling ON the CSS grid highlighter from the layout panel.");
-  const onHighlighterShown = highlighters.once("grid-highlighter-shown");
-  let onCheckboxChange = waitUntilState(store, state => state.grids[1].highlighted);
-  const checkbox = containerSubgridListEl.children[0].querySelector("input");
-  checkbox.click();
-  await onHighlighterShown;
-  await onCheckboxChange;
-
-  info("Checking the CSS grid highlighter is created.");
-  is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown.");
-
-  info("Toggling OFF the CSS grid highlighter from the layout panel.");
-  const onHighlighterHidden = highlighters.once("grid-highlighter-hidden");
-  onCheckboxChange = waitUntilState(store, state => !state.grids[1].highlighted);
-  checkbox.click();
-  await onHighlighterHidden;
-  await onCheckboxChange;
-
-  info("Checking the CSS grid highlighter is not shown.");
-  ok(!highlighters.gridHighlighters.size, "No CSS grid highlighter is shown.");
-});
-
-/**
- * Returns the grid item elements <li> from the grid list element <ul>.
- *
- * @param  {Element} gridListEl
- *         The grid list element <ul>.
- * @return {Array<Element>} containing the grid item elements <li>.
- */
-function getGridItemElements(gridListEl) {
-  return [...gridListEl.children].filter(node => node.nodeName === "li");
-}
deleted file mode 100644
--- a/devtools/client/inspector/grids/test/doc_subgrid.html
+++ /dev/null
@@ -1,56 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-  <meta charset="utf-8" />
-  <style>
-    .container {
-      display: grid;
-      grid-gap: 5px;
-      grid-template: auto / 1fr 3fr 1fr;
-      background: lightyellow;
-    }
-
-    .subgrid {
-      display: grid;
-      grid: subgrid;
-    }
-
-    header, aside, section, footer {
-      background: lightblue;
-      font-family: sans-serif;
-      font-size: 3em;
-    }
-
-    header, footer {
-      grid-column: span 3;
-    }
-
-    main {
-      grid-column: span 3;
-    }
-
-    .aside1 {
-      grid-column: 1;
-    }
-
-    .aside2 {
-      grid-column: 3;
-    }
-
-    section {
-      grid-column: 2;
-    }
-  </style>
-</head>
-<body>
-  <div class="container">
-    <header class="subgrid">Header</header>
-    <main class="subgrid">
-      <aside class="aside1 subgrid">aside</aside>
-      <section>section</section>
-      <aside class="aside2 subgrid">aside2</aside>
-    </main>
-    <footer>footer</footer>
-  </div>
-</body>
-</html>
--- a/devtools/client/inspector/grids/types.js
+++ b/devtools/client/inspector/grids/types.js
@@ -24,28 +24,19 @@ exports.grid = {
   disabled: PropTypes.bool,
 
   // The grid fragment object of the grid container
   gridFragments: PropTypes.array,
 
   // Whether or not the grid highlighter is highlighting the grid
   highlighted: PropTypes.bool,
 
-  // Whether or not the grid is a subgrid
-  isSubgrid: PropTypes.bool,
-
   // The node front of the grid container
   nodeFront: PropTypes.object,
 
-  // If the grid is a subgrid, this references the parent node front actor ID
-  parentNodeActorID: PropTypes.string,
-
-  // Array of ids belonging to the subgrid within the grid container
-  subgrids: PropTypes.arrayOf(PropTypes.number),
-
   // The writing mode of the grid container
   writingMode: PropTypes.string,
 };
 
 /**
  * The grid highlighter settings on what to display in its grid overlay in the document.
  */
 exports.highlighterSettings = {
--- a/devtools/client/themes/layout.css
+++ b/devtools/client/themes/layout.css
@@ -33,20 +33,16 @@
   text-overflow: ellipsis;
   overflow: hidden;
 }
 
 /**
  * Common styles for the layout container
  */
 
-.layout-container ul {
-  list-style: none;
-}
-
 .layout-container li {
   padding: 3px 0;
   -moz-user-select: none;
 }
 
 .layout-container input {
   margin-inline-end: 7px;
   vertical-align: middle;
@@ -642,24 +638,21 @@ html[dir="rtl"] .flex-item-list .devtool
 
 .grid-container > span {
   font-weight: 600;
   margin-bottom: 5px;
   pointer-events: none;
 }
 
 .grid-container > ul {
+  list-style: none;
   margin: 0;
   padding: 0;
 }
 
-#grid-list ul {
-  padding-inline-start: 20px;
-}
-
 /**
  * Grid Content
  */
 
 .grid-content {
   display: flex;
   flex-wrap: wrap;
   flex: 1;
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -1,16 +1,15 @@
 /* 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";
 
 const { Cu } = require("chrome");
-const Services = require("Services");
 const { Actor, ActorClassWithSpec } = require("devtools/shared/protocol");
 const {
   flexboxSpec,
   flexItemSpec,
   gridSpec,
   layoutSpec,
 } = require("devtools/shared/specs/layout");
 const { SHOW_ELEMENT } = require("devtools/shared/dom-node-filter-constants");
@@ -18,19 +17,16 @@ const { getStringifiableFragments } =
   require("devtools/server/actors/utils/css-grid-utils");
 
 loader.lazyRequireGetter(this, "CssLogic", "devtools/server/actors/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "getCSSStyleRules", "devtools/shared/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "isCssPropertyKnown", "devtools/server/actors/css-properties", true);
 loader.lazyRequireGetter(this, "parseDeclarations", "devtools/shared/css/parsing-utils", true);
 loader.lazyRequireGetter(this, "nodeConstants", "devtools/shared/dom-node-constants");
 
-const SUBGRID_ENABLED =
-  Services.prefs.getBoolPref("layout.css.grid-template-subgrid-value.enabled");
-
 /**
  * Set of actors the expose the CSS layout information to the devtools protocol clients.
  *
  * The |Layout| actor is the main entry point. It is used to get various CSS
  * layout-related information from the document.
  *
  * The |Flexbox| actor provides the container node information to inspect the flexbox
  * container. It is also used to return an array of |FlexItem| actors which provide the
@@ -257,68 +253,42 @@ const GridActor = ActorClassWithSpec(gri
    * @param  {DOMNode} containerEl
    *         The grid container element.
    */
   initialize(layoutActor, containerEl) {
     Actor.prototype.initialize.call(this, layoutActor.conn);
 
     this.containerEl = containerEl;
     this.walker = layoutActor.walker;
-    this.computedStyle = CssLogic.getComputedStyle(this.containerEl);
-
-    if (SUBGRID_ENABLED) {
-      const { gridTemplateColumns, gridTemplateRows } = this.computedStyle;
-      this.isSubgrid = gridTemplateRows === "subgrid" ||
-                       gridTemplateColumns === "subgrid";
-
-      if (this.isSubgrid) {
-        // The parent grid container for the subgrid.
-        this.parentEl = findGridParentContainerForNode(this.containerEl, this.walker);
-      }
-    }
   },
 
   destroy() {
     Actor.prototype.destroy.call(this);
 
-    this.computedStyle = null;
     this.containerEl = null;
     this.gridFragments = null;
-    this.isSubgrid = null;
-    this.parentEl = null;
     this.walker = null;
   },
 
   form() {
     // Seralize the grid fragment data into JSON so protocol.js knows how to write
     // and read the data.
     const gridFragments = this.containerEl.getGridFragments();
     this.gridFragments = getStringifiableFragments(gridFragments);
 
     // Record writing mode and text direction for use by the grid outline.
-    const { direction, writingMode } = this.computedStyle;
+    const { direction, writingMode } = CssLogic.getComputedStyle(this.containerEl);
 
     const form = {
       actor: this.actorID,
       direction,
       gridFragments: this.gridFragments,
       writingMode,
     };
 
-    if (SUBGRID_ENABLED && this.isSubgrid) {
-      form.isSubgrid = this.isSubgrid;
-
-      // If the WalkerActor already knows the parent grid element, then also return its
-      // ActorID so we avoid the client from doing another round trip to get it in many
-      // cases.
-      if (this.walker.hasNode(this.parentEl)) {
-        form.parentGridNodeActorID = this.walker.getNode(this.parentEl).actorID;
-      }
-    }
-
     // If the WalkerActor already knows the container element, then also return its
     // ActorID so we avoid the client from doing another round trip to get it in many
     // cases.
     if (this.walker.hasNode(this.containerEl)) {
       form.containerNodeActorID = this.walker.getNode(this.containerEl).actorID;
     }
 
     return form;
--- a/devtools/shared/fronts/layout.js
+++ b/devtools/shared/fronts/layout.js
@@ -108,57 +108,26 @@ class GridFront extends FrontClassWithSp
   /**
    * Getter for the grid fragments data.
    */
   get gridFragments() {
     return this._form.gridFragments;
   }
 
   /**
-   * Get whether or not the grid is a subgrid.
-   */
-  get isSubgrid() {
-    return !!this._form.isSubgrid;
-  }
-
-  /**
    * Get the writing mode of the grid container.
    * Added in Firefox 60.
    */
   get writingMode() {
     if (!this._form.writingMode) {
       return "horizontal-tb";
     }
 
     return this._form.writingMode;
   }
-
-  /**
-   * For a subgrid, returns the NodeFront of the parent grid container.
-   *
-   * @param  {WalkerFront} walker
-   *         Client side of the DOM walker.
-   * @return {NodeFront} of the parent grid container.
-   */
-  async getParentGridNode(walker) {
-    if (this._form.parentGridNodeActorID) {
-      return this.conn.getActor(this._form.parentGridNodeActorID);
-    }
-
-    // If the parentGridNodeActorID wasn't set, that means the nodeFront for this
-    // subgrid's parent grid container hasn't been seen by the walker yet.
-    // So, get the nodeFront from the server.
-    try {
-      return await walker.getNodeFromActor(this.actorID, ["parentEl"]);
-    } catch (e) {
-      // This call might fail if called asynchrously after the toolbox is finished
-      // closing.
-      return null;
-    }
-  }
 }
 
 class LayoutFront extends FrontClassWithSpec(layoutSpec) {
 }
 
 exports.FlexboxFront = FlexboxFront;
 registerFront(FlexboxFront);
 exports.FlexItemFront = FlexItemFront;