Bug 1403871 - remove misleading traverseFrames argument in getAllGrids;r=gl draft
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 21 Nov 2017 17:15:20 +0100
changeset 701561 db3c8ab4afd726379294198bdadd3383a6a73f1f
parent 701250 72ee4800d4156931c89b58bd807af4a3083702bb
child 701562 df0bb583afa34df7d0a6b25e2e733f253863d030
push id90196
push userjdescottes@mozilla.com
push dateTue, 21 Nov 2017 20:45:38 +0000
reviewersgl
bugs1403871
milestone59.0a1
Bug 1403871 - remove misleading traverseFrames argument in getAllGrids;r=gl The current implementation of getAllGrids supports a traverseFrames argument supposed to enable or not traversing nested frames under a provided root node. First of all this argument is only used in server tests. Also, even if this argument is false, the tree walk performed will traverse frames anyway, meaning there is no way here to not traverse frames when calling getAllGrids. The only real difference when calling the method with traverseFrames = true is that we will retrieve grid containers no longer contained in the root node (making the root node pointless). If we really want to implement this behavior in the future, we should split the API in two with getGrids(node) and getAllGrids(). For now I just remove the argument since it is not useful. MozReview-Commit-ID: JDVLL4qQYDv
devtools/server/actors/layout.js
devtools/server/tests/browser/browser_layout_getAllGrids.js
devtools/server/tests/browser/browser_layout_simple.js
devtools/shared/specs/layout.js
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -223,37 +223,27 @@ const LayoutActor = ActorClassWithSpec(l
       }
     }
 
     return grids;
   },
 
   /**
    * Returns an array of GridActor objects for all existing grid containers found by
-   * iterating below the given rootNode and optionally including nested frames.
+   * iterating below the given rootNode and including nested frames.
    *
    * @param  {NodeActor} rootNode
-   * @param  {Boolean} traverseFrames
-   *         Whether or not we should iterate through nested frames.
    * @return {Array} An array of GridActor objects.
    */
-  getAllGrids(rootNode, traverseFrames) {
+  getAllGrids(rootNode) {
     let grids = [];
 
     if (!rootNode) {
       return grids;
     }
 
-    if (!traverseFrames) {
-      return this.getGrids(rootNode.rawNode);
-    }
-
-    for (let {document} of this.tabActor.windows) {
-      grids = [...grids, ...this.getGrids(document.documentElement)];
-    }
-
-    return grids;
+    return this.getGrids(rootNode.rawNode);
   },
 });
 
 exports.FlexboxActor = FlexboxActor;
 exports.GridActor = GridActor;
 exports.LayoutActor = LayoutActor;
--- a/devtools/server/tests/browser/browser_layout_getAllGrids.js
+++ b/devtools/server/tests/browser/browser_layout_getAllGrids.js
@@ -104,17 +104,17 @@ const GRID_FRAGMENT_DATA = {
         type: "explicit"
       }
     ]
   }
 };
 
 add_task(function* () {
   let { client, walker, layout } = yield initLayoutFrontForUrl(MAIN_DOMAIN + "grid.html");
-  let grids = yield layout.getAllGrids(walker.rootNode, true);
+  let grids = yield layout.getAllGrids(walker.rootNode);
   let grid = grids[0];
   let { gridFragments } = grid;
 
   is(grids.length, 1, "One grid was returned.");
   is(gridFragments.length, 1, "One grid fragment was returned.");
   ok(Array.isArray(gridFragments), "An array of grid fragments was returned.");
   Assert.deepEqual(gridFragments[0], GRID_FRAGMENT_DATA,
     "Got the correct grid fragment data.");
--- a/devtools/server/tests/browser/browser_layout_simple.js
+++ b/devtools/server/tests/browser/browser_layout_simple.js
@@ -25,15 +25,15 @@ add_task(function* () {
   try {
     yield layout.getAllGrids(null);
   } catch (e) {
     didThrow = true;
   }
   ok(didThrow, "An exception was thrown for a missing NodeActor in getAllGrids");
 
   let invalidNode = yield walker.querySelector(walker.rootNode, "title");
-  let grids = yield layout.getAllGrids(invalidNode, true);
+  let grids = yield layout.getAllGrids(invalidNode);
   ok(Array.isArray(grids), "An array of grids was returned");
   is(grids.length, 0, "0 grids have been returned for the invalid node");
 
   yield client.close();
   gBrowser.removeCurrentTab();
 });
--- a/devtools/shared/specs/layout.js
+++ b/devtools/shared/specs/layout.js
@@ -29,18 +29,17 @@ const layoutSpec = generateActorSpec({
       },
       response: {
         flexboxes: RetVal("array:flexbox")
       }
     },
 
     getAllGrids: {
       request: {
-        rootNode: Arg(0, "domnode"),
-        traverseFrames: Arg(1, "nullable:boolean")
+        rootNode: Arg(0, "domnode")
       },
       response: {
         grids: RetVal("array:grid")
       }
     },
   },
 });