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 idunknown
push userunknown
push dateunknown
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();