Bug 991210 - [new tab page] Tiles are sometimes arranged all in a single line (wrapping as appropriate, e.g. to two lines with 5 items and then 4 items), instead of 3x3 grid [r=adw]

Always Grid_init before "load", defer size calculation to "load", and only wait for links cache for site rendering. Clean up timers triggered from previous tests to avoid unexpected updates.

--HG--
extra : rebase_source : aba79dc12fd3660c50d8004197a6b2f5a23419cb

browser/base/content/newtab/grid.js

@@ -37,7 +37,7 @@ let gGrid = {
   get sites() [cell.site for each (cell in this.cells)],
 
   // Tells whether the grid has already been initialized.
-  get ready() !!this._node,
+  get ready() !!this._ready,
 
   /**
    * Initializes the grid.
@@ -46,7 +46,11 @@ let gGrid = {
   init: function Grid_init() {
     this._node = document.getElementById("newtab-grid");
     this._createSiteFragment();
-    this._render();
+    this._renderGrid();
+    gLinks.populateCache(() => {
+      this._renderSites();
+      this._ready = true;
+    });
     addEventListener("load", this);
     addEventListener("resize", this);
   },
@@ -69,14 +73,6 @@ let gGrid = {
   handleEvent: function Grid_handleEvent(aEvent) {
     switch (aEvent.type) {
       case "load":
-        // Save the cell's computed height/width including margin and border
-        let refCell = document.querySelector(".newtab-cell");
-        this._cellMargin = parseFloat(getComputedStyle(refCell).marginTop) * 2;
-        this._cellHeight = refCell.offsetHeight + this._cellMargin;
-        this._cellWidth = refCell.offsetWidth + this._cellMargin;
-        this._resizeGrid();
-        break;
-
       case "resize":
         this._resizeGrid();
         break;
@@ -97,8 +93,11 @@ let gGrid = {
     }, this);
 
     // Render the grid again.
-    this._render();
-    this._resizeGrid();
+    if (this._shouldRenderGrid()) {
+      this._renderGrid();
+      this._resizeGrid();
+    }
+    this._renderSites();
   },
 
   /**
@@ -185,21 +184,18 @@ let gGrid = {
     }
   },
 
-  /**
-   * Renders the grid.
-   */
-  _render: function Grid_render() {
-    if (this._shouldRenderGrid()) {
-      this._renderGrid();
-    }
-
-    this._renderSites();
-  },
-
   /**
    * Make sure the correct number of rows and columns are visible
    */
   _resizeGrid: function Grid_resizeGrid() {
+    // Save the cell's computed height/width including margin and border
+    if (this._cellMargin === undefined) {
+      let refCell = document.querySelector(".newtab-cell");
+      this._cellMargin = parseFloat(getComputedStyle(refCell).marginTop) * 2;
+      this._cellHeight = refCell.offsetHeight + this._cellMargin;
+      this._cellWidth = refCell.offsetWidth + this._cellMargin;
+    }
+
     let availSpace = document.documentElement.clientHeight - this._cellMargin -
                      document.querySelector("#newtab-margin-top").offsetHeight;
     let visibleRows = Math.floor(availSpace / this._cellHeight);

browser/base/content/newtab/page.js

@@ -144,19 +144,17 @@ let gPage = {
       attributeFilter: ["allow-background-captures"],
     });
 
-    gLinks.populateCache(function () {
-      // Initialize and render the grid.
-      gGrid.init();
+    // Initialize and render the grid.
+    gGrid.init();
 
-      // Initialize the drop target shim.
-      gDropTargetShim.init();
+    // Initialize the drop target shim.
+    gDropTargetShim.init();
 
 #ifdef XP_MACOSX
-      // Workaround to prevent a delay on MacOSX due to a slow drop animation.
-      document.addEventListener("dragover", this, false);
-      document.addEventListener("drop", this, false);
+    // Workaround to prevent a delay on MacOSX due to a slow drop animation.
+    document.addEventListener("dragover", this, false);
+    document.addEventListener("drop", this, false);
 #endif
-    }.bind(this));
   },
 
   /**

browser/base/content/test/newtab/browser.ini

@@ -16,6 +16,7 @@ skip-if = os == "mac" # Intermittent failures, bug 898317
 [browser_newtab_bug765628.js]
 [browser_newtab_bug876313.js]
 [browser_newtab_bug991111.js]
+[browser_newtab_bug991210.js]
 [browser_newtab_disable.js]
 [browser_newtab_drag_drop.js]
 [browser_newtab_drag_drop_ext.js]

browser/base/content/test/newtab/browser_newtab_bug991210.js

@@ -0,0 +1,41 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const PRELOAD_PREF = "browser.newtab.preload";
+
+function runTests() {
+  // turn off preload to ensure that a newtab page loads
+  Services.prefs.setBoolPref(PRELOAD_PREF, false);
+
+  // add a test provider that waits for load
+  let afterLoadProvider = {
+    getLinks: function(callback) {
+      this.callback = callback;
+    },
+    addObserver: function() {},
+  };
+  NewTabUtils.links.addProvider(afterLoadProvider);
+
+  // wait until about:newtab loads before calling provider callback
+  addNewTabPageTab();
+  let browser = gWindow.gBrowser.selectedTab.linkedBrowser;
+  yield browser.addEventListener("load", function onLoad() {
+    browser.removeEventListener("load", onLoad, true);
+    // afterLoadProvider.callback has to be called asynchronously to make grid
+    // initilize after "load" event was handled
+    executeSoon(() => afterLoadProvider.callback([]));
+  }, true);
+
+  let {_cellMargin, _cellHeight, _cellWidth, node} = getGrid();
+  isnot(_cellMargin, null, "grid has a computed cell margin");
+  isnot(_cellHeight, null, "grid has a computed cell height");
+  isnot(_cellWidth, null, "grid has a computed cell width");
+  let {height, maxHeight, maxWidth} = node.style;
+  isnot(height, "", "grid has a computed grid height");
+  isnot(maxHeight, "", "grid has a computed grid max-height");
+  isnot(maxWidth, "", "grid has a computed grid max-width");
+
+  // restore original state
+  NewTabUtils.links.removeProvider(afterLoadProvider);
+  Services.prefs.clearUserPref(PRELOAD_PREF);
+}

browser/base/content/test/newtab/browser_newtab_update.js

@@ -6,11 +6,6 @@
  */
 
 function runTests() {
-  if (NewTabUtils.allPages.updateScheduledForHiddenPages) {
-    // Wait for dynamic updates triggered by the previous test to finish.
-    yield whenPagesUpdated(null, true);
-  }
-
   // First, start with an empty page.  setLinks will trigger a hidden page
   // update because it calls clearHistory.  We need to wait for that update to
   // happen so that the next time we wait for a page update below, we catch the

browser/base/content/test/newtab/head.js

@@ -14,7 +14,8 @@ Cu.import("resource://gre/modules/NewTabUtils.jsm", tmp);
 Cc["@mozilla.org/moz/jssubscript-loader;1"]
   .getService(Ci.mozIJSSubScriptLoader)
   .loadSubScript("chrome://browser/content/sanitize.js", tmp);
-let {Promise, NewTabUtils, Sanitizer} = tmp;
+Cu.import("resource://gre/modules/Timer.jsm", tmp);
+let {Promise, NewTabUtils, Sanitizer, clearTimeout} = tmp;
 
 let uri = Services.io.newURI("about:newtab", null, null);
 let principal = Services.scriptSecurityManager.getNoAppCodebasePrincipal(uri);
@@ -30,6 +31,13 @@ registerCleanupFunction(function () {
 
   Services.prefs.clearUserPref(PREF_NEWTAB_ENABLED);
   Services.prefs.clearUserPref(PREF_NEWTAB_DIRECTORYSOURCE);
+
+  // Stop any update timers to prevent unexpected updates in later tests
+  let timer = NewTabUtils.allPages._scheduleUpdateTimeout;
+  if (timer) {
+    clearTimeout(timer);
+    delete NewTabUtils.allPages._scheduleUpdateTimeout;
+  }
 });
 
 /**

toolkit/modules/NewTabUtils.jsm

@@ -281,10 +281,6 @@ let AllPages = {
     }
   },
 
-  get updateScheduledForHiddenPages() {
-    return !!this._scheduleUpdateTimeout;
-  },
-
   /**
    * Implements the nsIObserver interface to get notified when the preference
    * value changes or when a new copy of a page thumbnail is available.