Bug 477880. Since we kick off some loads during reflow, make sure to flush layout before our last pre-onload emptiness check. r=jst, sr=roc
authorBoris Zbarsky <bzbarsky@mit.edu>
Sun, 15 Feb 2009 13:14:19 -0500
changeset 25008 1190db3643e4926f9a18abb8577e5d6df63710b5
parent 25007 405c4bcbd3b368223664dd786f57427217f533cf
child 25009 776c2e2a210b1905fad2bb444df6593406243209
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjst, roc
bugs477880
milestone1.9.2a1pre
Bug 477880. Since we kick off some loads during reflow, make sure to flush layout before our last pre-onload emptiness check. r=jst, sr=roc
layout/reftests/font-face/download-3-notref.html
layout/reftests/font-face/download-3-ref.html
layout/reftests/font-face/download-3.html
layout/reftests/font-face/reftest.list
uriloader/base/nsDocLoader.cpp
uriloader/base/nsDocLoader.h
new file mode 100644
--- /dev/null
+++ b/layout/reftests/font-face/download-3-notref.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN"
+	"http://www.w3.org/TR/html4/strict.dtd">
+<html lang="en-US">
+<head>
+	<title></title>
+	<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+	<style type="text/css">
+	body { font-family: "MarkA"; }
+	</style>
+</head>
+<body>
+
+<div id="t" style="visibility:hidden; width: -moz-fit-content">ABC</div>
+<script>
+  document.body.offsetWidth;
+  var n = document.getElementById("t");
+  var w = document.defaultView.getComputedStyle(n, "").width;
+  var h = document.defaultView.getComputedStyle(n, "").height;
+  var d = document.createElement("div");
+  d.style.width = w;
+  d.style.height = h;
+  d.style.backgroundColor = "green";
+  n.parentNode.removeChild(n);
+  document.body.appendChild(d);
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/font-face/download-3-ref.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN"
+	"http://www.w3.org/TR/html4/strict.dtd">
+<html lang="en-US">
+<head>
+	<title></title>
+	<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+	<style type="text/css">
+
+	@font-face {
+		font-family: "MarkA";
+		src: url(../fonts/markA.ttf);
+	}
+
+	body { font-family: "MarkA"; }
+
+	</style>
+</head>
+<body>
+
+<div id="t" style="visibility:hidden; width: -moz-fit-content">ABC</div>
+<script>
+  // Force a reflow to make sure we start our font download now
+  document.body.offsetWidth;
+  window.addEventListener("load",
+    function() {
+      var n = document.getElementById("t");
+      var w = document.defaultView.getComputedStyle(n, "").width;
+      var h = document.defaultView.getComputedStyle(n, "").height;
+      var d = document.createElement("div");
+      d.style.width = w;
+      d.style.height = h;
+      d.style.backgroundColor = "green";
+      n.parentNode.removeChild(n);
+      document.body.appendChild(d);
+    },
+    false);
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/font-face/download-3.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN"
+	"http://www.w3.org/TR/html4/strict.dtd">
+<html lang="en-US" style="display: none">
+<head>
+	<title></title>
+	<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+	<style type="text/css">
+
+	@font-face {
+		font-family: "MarkA";
+		src: url(../fonts/markA.ttf);
+	}
+
+	body { font-family: "MarkA"; }
+
+	</style>
+</head>
+<body>
+
+<div id="t" style="visibility:hidden; width: -moz-fit-content">ABC</div>
+<script>
+  // Make sure to show our stuff as late as we can, so we only get reflows
+  // from onload.  
+  window.addEventListener("DOMContentLoaded",
+    function() { document.documentElement.style.display = ""; },
+    false)
+  window.addEventListener("load",
+    function() {
+      var n = document.getElementById("t");
+      var w = document.defaultView.getComputedStyle(n, "").width;
+      var h = document.defaultView.getComputedStyle(n, "").height;
+      var d = document.createElement("div");
+      d.style.width = w;
+      d.style.height = h;
+      d.style.backgroundColor = "green";
+      n.parentNode.removeChild(n);
+      document.body.appendChild(d);
+    },
+    false);
+</script>
+</body>
+</html>
--- a/layout/reftests/font-face/reftest.list
+++ b/layout/reftests/font-face/reftest.list
@@ -2,16 +2,18 @@
 # ../fonts/.  We can't use file:/// URLs because of cross-directory access
 # restrictions on file: URLs.
 
 HTTP(..) != download-1.html download-1-notref.html
 HTTP(..) == download-2.html download-2-ref.html
 HTTP(..) != download-2.html about:blank
 fails-if(MOZ_WIDGET_TOOLKIT=="windows") HTTP(..) == download-2-big.html download-2-big-otf.html # bug 470713
 HTTP(..) != download-2-big-otf.html about:blank
+HTTP(..) != download-3-notref.html download-3.html
+HTTP(..) == download-3-ref.html download-3.html
 HTTP(..) == fallback-to-system-1.html fallback-to-system-1-ref.html
 HTTP(..) == name-override-simple-1.html name-override-simple-1-ref.html
 HTTP(..) != name-override-simple-1.html download-1-notref.html
 fails HTTP(..) == name-override-1.html name-override-1-ref.html
 HTTP(..) == multiple-descriptor-1.html multiple-descriptor-1-ref.html
 HTTP(..) != multiple-descriptor-1.html multiple-descriptor-1-notref.html
 HTTP(..) == src-list-1.html src-list-1-ref.html
 HTTP(..) == src-list-2.html src-list-2-ref.html
--- a/uriloader/base/nsDocLoader.cpp
+++ b/uriloader/base/nsDocLoader.cpp
@@ -58,16 +58,19 @@
 #include "nsIPresShell.h"
 #include "nsPresContext.h"
 #include "nsIStringBundle.h"
 #include "nsIScriptSecurityManager.h"
 
 #include "nsITransport.h"
 #include "nsISocketTransport.h"
 
+#include "nsIDOMDocument.h"
+#include "nsIDocument.h"
+
 static NS_DEFINE_CID(kThisImplCID, NS_THIS_DOCLOADER_IMPL_CID);
 
 #if defined(PR_LOGGING)
 //
 // Log module for nsIDocumentLoader logging...
 //
 // To enable logging (see prlog.h for full details):
 //
@@ -126,29 +129,28 @@ struct nsListenerInfo {
   nsWeakPtr mWeakListener;
 
   // Mask indicating which notifications the listener wants to receive.
   unsigned long mNotifyMask;
 };
 
 
 nsDocLoader::nsDocLoader()
-  : mListenerInfoList(8)
+  : mParent(nsnull),
+    mListenerInfoList(8),
+    mIsLoadingDocument(PR_FALSE),
+    mIsRestoringDocument(PR_FALSE),
+    mIsFlushingLayout(PR_FALSE)
 {
 #if defined(PR_LOGGING)
   if (nsnull == gDocLoaderLog) {
       gDocLoaderLog = PR_NewLogModule("DocLoader");
   }
 #endif /* PR_LOGGING */
 
-  mParent    = nsnull;
-
-  mIsLoadingDocument = PR_FALSE;
-  mIsRestoringDocument = PR_FALSE;
-
   static PLDHashTableOps hash_table_ops =
   {
     PL_DHashAllocTable,
     PL_DHashFreeTable,
     PL_DHashVoidPtrKeyStub,
     PL_DHashMatchEntryStub,
     PL_DHashMoveEntryStub,
     PL_DHashClearEntryStub,
@@ -322,17 +324,17 @@ nsDocLoader::Stop(void)
   // happened yet, Canceling the loadgroup did nothing (because it was already
   // empty), and we're about to start a new load (which is what triggered this
   // Stop() call).
 
   // XXXbz If the child frame loadgroups were requests in mLoadgroup, I suspect
   // we wouldn't need the call here....
 
   NS_ASSERTION(!IsBusy(), "Shouldn't be busy here");
-  DocLoaderIsEmpty();
+  DocLoaderIsEmpty(PR_FALSE);
   
   return rv;
 }       
 
 
 PRBool
 nsDocLoader::IsBusy()
 {
@@ -340,19 +342,20 @@ nsDocLoader::IsBusy()
 
   //
   // A document loader is busy if either:
   //
   //   1. One of its children is in the middle of an onload handler.  Note that
   //      the handler may have already removed this child from mChildList!
   //   2. It is currently loading a document and either has parts of it still
   //      loading, or has a busy child docloader.
+  //   3. It's currently flushing layout in DocLoaderIsEmpty().
   //
 
-  if (mChildrenInOnload.Count()) {
+  if (mChildrenInOnload.Count() || mIsFlushingLayout) {
     return PR_TRUE;
   }
 
   /* Is this document loader busy? */
   if (!mIsLoadingDocument) {
     return PR_FALSE;
   }
   
@@ -567,17 +570,16 @@ nsDocLoader::OnStopRequest(nsIRequest *a
   }
 #endif
 
   //
   // Only fire the OnEndDocumentLoad(...) if the document loader 
   // has initiated a load...
   //
   if (mIsLoadingDocument) {
-    PRUint32 count;
     PRBool bFireTransferring = PR_FALSE;
 
     //
     // Set the Maximum progress to the same value as the current progress.
     // Since the URI has finished loading, all the data is there.  Also,
     // this will allow a more accurate estimation of the max progress (in case
     // the old value was unknown ie. -1)
     //
@@ -662,27 +664,23 @@ nsDocLoader::OnStopRequest(nsIRequest *a
       }
 
       FireOnStateChange(this, aRequest, flags, NS_OK);
     }
 
     //
     // Fire the OnStateChange(...) notification for stop request
     //
+    // XXXbz can we just combine these notifications with the else case?  Or
+    // can it happen that mIsLoadingDocument is false when we enter this method
+    // but becomes true after the doStartURLLoad call, in which case we may not
+    // want to call DocLoaderIsEmpty if we're really empty?
     doStopURLLoad(aRequest, aStatus);
     
-    rv = mLoadGroup->GetActiveCount(&count);
-    if (NS_FAILED(rv)) return rv;
-
-    //
-    // The load group for this DocumentLoader is idle...
-    //
-    if (0 == count) {
-      DocLoaderIsEmpty();
-    }
+    DocLoaderIsEmpty(PR_TRUE);
   }
   else {
     doStopURLLoad(aRequest, aStatus); 
   }
   
   return NS_OK;
 }
 
@@ -711,25 +709,46 @@ NS_IMETHODIMP nsDocLoader::GetDocumentCh
     *aChannel = nsnull;
     return NS_OK;
   }
   
   return CallQueryInterface(mDocumentRequest, aChannel);
 }
 
 
-void nsDocLoader::DocLoaderIsEmpty()
+void nsDocLoader::DocLoaderIsEmpty(PRBool aFlushLayout)
 {
   if (mIsLoadingDocument) {
     /* In the unimagineably rude circumstance that onload event handlers
        triggered by this function actually kill the window ... ok, it's
        not unimagineable; it's happened ... this deathgrip keeps this object
        alive long enough to survive this function call. */
     nsCOMPtr<nsIDocumentLoader> kungFuDeathGrip(this);
 
+    // Don't flush layout if we're still busy.
+    if (IsBusy()) {
+      return;
+    }
+
+    NS_ASSERTION(!mIsFlushingLayout, "Someone screwed up");
+
+    // The load group for this DocumentLoader is idle.  Flush layout if we need
+    // to.
+    if (aFlushLayout) {
+      nsCOMPtr<nsIDOMDocument> domDoc = do_GetInterface(GetAsSupports(this));
+      nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
+      if (doc) {
+        mIsFlushingLayout = PR_TRUE;
+        doc->FlushPendingNotifications(Flush_Layout);
+        mIsFlushingLayout = PR_FALSE;
+      }
+    }
+
+    // And now check whether we're really busy; that might have changed with
+    // the layout flush.
     if (!IsBusy()) {
       PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, 
              ("DocLoader:%p: Is now idle...\n", this));
 
       nsCOMPtr<nsIRequest> docRequest = mDocumentRequest;
 
       NS_ASSERTION(mDocumentRequest, "No Document Request!");
       mDocumentRequest = 0;
--- a/uriloader/base/nsDocLoader.h
+++ b/uriloader/base/nsDocLoader.h
@@ -197,37 +197,31 @@ protected:
         // us.
         return mChildrenInOnload.AppendObject(aChild);
     }
 
     // Inform a parent docloader that aChild is done calling its onload
     // handler.
     void ChildDoneWithOnload(nsIDocumentLoader* aChild) {
         mChildrenInOnload.RemoveObject(aChild);
-        DocLoaderIsEmpty();
+        DocLoaderIsEmpty(PR_TRUE);
     }        
 
 protected:
     // IMPORTANT: The ownership implicit in the following member
     // variables has been explicitly checked and set using nsCOMPtr
     // for owning pointers and raw COM interface pointers for weak
     // (ie, non owning) references. If you add any members to this
     // class, please make the ownership explicit (pinkerton, scc).
   
     nsCOMPtr<nsIRequest>       mDocumentRequest;       // [OWNER] ???compare with document
 
     nsDocLoader*               mParent;                // [WEAK]
 
     nsVoidArray                mListenerInfoList;
-    /*
-     * This flag indicates that the loader is loading a document.  It is set
-     * from the call to LoadDocument(...) until the OnConnectionsComplete(...)
-     * notification is fired...
-     */
-    PRBool mIsLoadingDocument;
 
     nsCOMPtr<nsILoadGroup>        mLoadGroup;
     // We hold weak refs to all our kids
     nsVoidArray                   mChildList;
 
     // The following member variables are related to the new nsIWebProgress 
     // feedback interfaces that travis cooked up.
     PRInt32 mProgressStateFlags;
@@ -235,30 +229,43 @@ protected:
     nsInt64 mCurrentSelfProgress;
     nsInt64 mMaxSelfProgress;
 
     nsInt64 mCurrentTotalProgress;
     nsInt64 mMaxTotalProgress;
 
     PLDHashTable mRequestInfoHash;
 
+    /*
+     * This flag indicates that the loader is loading a document.  It is set
+     * from the call to LoadDocument(...) until the OnConnectionsComplete(...)
+     * notification is fired...
+     */
+    PRPackedBool mIsLoadingDocument;
+
     /* Flag to indicate that we're in the process of restoring a document. */
-    PRBool mIsRestoringDocument;
+    PRPackedBool mIsRestoringDocument;
+
+    /* Flag to indicate that we're in the process of flushing layout
+       undere DocLoaderIsEmpty() */
+    PRPackedBool mIsFlushingLayout;
 
 private:
     // A list of kids that are in the middle of their onload calls and will let
     // us know once they're done.  We don't want to fire onload for "normal"
     // DocLoaderIsEmpty calls (those coming from requests finishing in our
     // loadgroup) unless this is empty.
     nsCOMArray<nsIDocumentLoader> mChildrenInOnload;
     
     // DocLoaderIsEmpty should be called whenever the docloader may be empty.
     // This method is idempotent and does nothing if the docloader is not in
-    // fact empty.
-    void DocLoaderIsEmpty();
+    // fact empty.  This method _does_ make sure that layout is flushed if our
+    // loadgroup has no active requests before checking for "real" emptiness if
+    // aFlushLayout is true.
+    void DocLoaderIsEmpty(PRBool aFlushLayout);
 
     nsListenerInfo *GetListenerInfo(nsIWebProgressListener* aListener);
 
     PRInt64 GetMaxTotalProgress();
 
     nsresult AddRequestInfo(nsIRequest* aRequest);
     nsRequestInfo *GetRequestInfo(nsIRequest* aRequest);
     void ClearRequestInfoHash();