From b30198a89746cfcdfa3eb6fa68e5b221cebb36e4 Mon Sep 17 00:00:00 2001 From: "allan%beaufour.dk" Date: Mon, 15 May 2006 14:14:52 +0000 Subject: [PATCH] [XForms] Improve refresh inefficiencies with selects, items. Bug 329935, r=smaug+me, patch by aaronr --- extensions/xforms/nsIXFormsControlBase.idl | 5 ++ extensions/xforms/nsXFormsControlStub.cpp | 15 ++++ extensions/xforms/nsXFormsControlStub.h | 15 +++- extensions/xforms/nsXFormsModelElement.cpp | 83 +++++++++++++++++++- extensions/xforms/nsXFormsModelElement.h | 20 +++++ extensions/xforms/nsXFormsSelect1Element.cpp | 16 ++++ extensions/xforms/nsXFormsSelectElement.cpp | 19 +++++ 7 files changed, 167 insertions(+), 6 deletions(-) diff --git a/extensions/xforms/nsIXFormsControlBase.idl b/extensions/xforms/nsIXFormsControlBase.idl index 2314ef07fd1..bd9da8b0469 100644 --- a/extensions/xforms/nsIXFormsControlBase.idl +++ b/extensions/xforms/nsIXFormsControlBase.idl @@ -50,4 +50,9 @@ interface nsIXFormsControlBase : nsISupports * instance data. */ void refresh(); + + /** + * Determines whether this control is already on the deferred bind list + */ + attribute boolean onDeferredBindList; }; diff --git a/extensions/xforms/nsXFormsControlStub.cpp b/extensions/xforms/nsXFormsControlStub.cpp index 4491fad95a1..ba23c68d3c1 100644 --- a/extensions/xforms/nsXFormsControlStub.cpp +++ b/extensions/xforms/nsXFormsControlStub.cpp @@ -234,6 +234,21 @@ nsXFormsControlStubBase::GetUsesModelBinding(PRBool *aRes) return NS_OK; } +NS_IMETHODIMP +nsXFormsControlStubBase::GetOnDeferredBindList(PRBool *aOnList) +{ + NS_ENSURE_ARG_POINTER(aOnList); + *aOnList = mOnDeferredBindList; + return NS_OK; +} + +NS_IMETHODIMP +nsXFormsControlStubBase::SetOnDeferredBindList(PRBool aPutOnList) +{ + mOnDeferredBindList = aPutOnList; + return NS_OK; +} + nsresult nsXFormsControlStubBase::MaybeAddToModel(nsIModelElementPrivate *aOldModel, nsIXFormsControl *aParent) diff --git a/extensions/xforms/nsXFormsControlStub.h b/extensions/xforms/nsXFormsControlStub.h index f4a742a9027..f9a62840a33 100644 --- a/extensions/xforms/nsXFormsControlStub.h +++ b/extensions/xforms/nsXFormsControlStub.h @@ -88,6 +88,8 @@ public: nsIDOMXPathResult **aResult = nsnull); NS_IMETHOD Bind(); NS_IMETHOD Refresh(); + NS_IMETHOD GetOnDeferredBindList(PRBool *onList); + NS_IMETHOD SetOnDeferredBindList(PRBool putOnList); NS_IMETHOD TryFocus(PRBool* aOK); NS_IMETHOD IsEventTarget(PRBool *aOK); NS_IMETHOD GetUsesModelBinding(PRBool *aRes); @@ -144,6 +146,7 @@ public: mPreventLoop(PR_FALSE), mUsesModelBinding(PR_FALSE), mAppearDisabled(PR_FALSE), + mOnDeferredBindList(PR_FALSE), mBindAttrsCount(0) {}; @@ -183,15 +186,21 @@ protected: PRPackedBool mAppearDisabled; /** - * Array of repeat-elements of which the control uses repeat-index. + * Indicates whether this control is already on the deferred bind list */ - nsCOMArray mIndexesUsed; + PRPackedBool mOnDeferredBindList; /** * Used to keep track of whether this control has any single node binding * attributes. */ - PRInt32 mBindAttrsCount; + PRInt8 mBindAttrsCount; + + /** + * List of repeats that the node binding depends on. This happens when using + * the index() function in the binding expression. + */ + nsCOMArray mIndexesUsed; /** * Does control have a binding attribute? diff --git a/extensions/xforms/nsXFormsModelElement.cpp b/extensions/xforms/nsXFormsModelElement.cpp index e5f3d5769d3..e25217d6d14 100644 --- a/extensions/xforms/nsXFormsModelElement.cpp +++ b/extensions/xforms/nsXFormsModelElement.cpp @@ -76,6 +76,7 @@ #include "nsIDOMDocumentXBL.h" #include "nsIProgrammingLanguage.h" #include "nsDOMError.h" +#include "nsXFormsControlStub.h" #define XFORMS_LAZY_INSTANCE_BINDING \ "chrome://xforms/content/xforms.xml#xforms-lazy-instance" @@ -501,6 +502,7 @@ static nsIAtom* sModelPropsList[eModel__count]; // This can be nsVoidArray because elements will remove // themselves from the list if they are deleted during refresh. static nsVoidArray* sPostRefreshList = nsnull; +static nsVoidArray* sContainerPostRefreshList = nsnull; static PRInt32 sRefreshing = 0; @@ -517,7 +519,13 @@ nsPostRefresh::~nsPostRefresh() #ifdef DEBUG_smaug printf("~nsPostRefresh\n"); #endif - if (sPostRefreshList && sRefreshing == 1) { + + if (sRefreshing != 1) { + --sRefreshing; + return; + } + + if (sPostRefreshList) { while (sPostRefreshList->Count()) { // Iterating this way because refresh can lead to // additions/deletions in sPostRefreshList. @@ -535,7 +543,26 @@ nsPostRefresh::~nsPostRefresh() sPostRefreshList = nsnull; } } + --sRefreshing; + + // process sContainerPostRefreshList after we've decremented sRefreshing. + // container->refresh below could ask for ContainerNeedsPostRefresh which + // will add an item to the sContainerPostRefreshList if sRefreshing > 0. + // So keeping this under sRefreshing-- will avoid an infinite loop. + if (sContainerPostRefreshList) { + while (sContainerPostRefreshList->Count()) { + PRInt32 last = sContainerPostRefreshList->Count() - 1; + nsIXFormsControl* container = + NS_STATIC_CAST(nsIXFormsControl*, sContainerPostRefreshList->ElementAt(last)); + sContainerPostRefreshList->RemoveElementAt(last); + if (container) { + container->Refresh(); + } + } + delete sContainerPostRefreshList; + sContainerPostRefreshList = nsnull; + } } const nsVoidArray* @@ -564,11 +591,43 @@ nsXFormsModelElement::NeedsPostRefresh(nsIXFormsControl* aControl) return NS_OK; } +PRBool +nsXFormsModelElement::ContainerNeedsPostRefresh(nsIXFormsControl* aControl) +{ + + if (sRefreshing) { + if (!sContainerPostRefreshList) { + sContainerPostRefreshList = new nsVoidArray(); + if (!sContainerPostRefreshList) { + return PR_FALSE; + } + } + + if (sContainerPostRefreshList->IndexOf(aControl) < 0) { + sContainerPostRefreshList->AppendElement(aControl); + } + + // return PR_TRUE to show that the control's refresh will be delayed, + // whether as a result of this call or a previous call to this function. + return PR_TRUE; + } + + // Delaying the refresh doesn't make any sense. But since this + // function may be called from inside the control node's refresh already, + // we shouldn't just assume that we can call the refresh here. So + // we'll just return PR_FALSE to signal that we couldn't delay the refresh. + + return PR_FALSE; +} + void nsXFormsModelElement::CancelPostRefresh(nsIXFormsControl* aControl) { if (sPostRefreshList) sPostRefreshList->RemoveElement(aControl); + + if (sContainerPostRefreshList) + sContainerPostRefreshList->RemoveElement(aControl); } nsXFormsModelElement::nsXFormsModelElement() @@ -2381,9 +2440,25 @@ nsXFormsModelElement::DeferElementBind(nsIDOMDocument *aDoc, return NS_ERROR_FAILURE; } + // We are using a PRBool on each control to mark whether the control is on the + // deferredBindList. We are running into too many scenarios where a control + // could be added more than once which will lead to inefficiencies because + // calling bind and refresh on some controls is getting pretty expensive. + // We need to keep the document order of the controls AND don't want + // to walk the deferredBindList every time we want to check about adding a + // control. + nsCOMPtr controlBase(do_QueryInterface(aControl)); + NS_ENSURE_STATE(controlBase); + + PRBool onList = PR_FALSE; + controlBase->GetOnDeferredBindList(&onList); + if (onList) { + return NS_OK; + } + nsCOMArray *deferredBindList = - NS_STATIC_CAST(nsCOMArray *, - doc->GetProperty(nsXFormsAtoms::deferredBindListProperty)); + NS_STATIC_CAST(nsCOMArray *, + doc->GetProperty(nsXFormsAtoms::deferredBindListProperty)); if (!deferredBindList) { deferredBindList = new nsCOMArray(16); @@ -2398,6 +2473,7 @@ nsXFormsModelElement::DeferElementBind(nsIDOMDocument *aDoc, // when an element is trying to bind and should use its parent as a context // for the xpath evaluation but the parent isn't bound yet. deferredBindList->AppendObject(aControl); + controlBase->SetOnDeferredBindList(PR_TRUE); return NS_OK; } @@ -2425,6 +2501,7 @@ nsXFormsModelElement::ProcessDeferredBinds(nsIDOMDocument *aDoc) if (base) { base->Bind(); base->Refresh(); + base->SetOnDeferredBindList(PR_FALSE); } } diff --git a/extensions/xforms/nsXFormsModelElement.h b/extensions/xforms/nsXFormsModelElement.h index ea156ce6e07..c9fe8feef00 100644 --- a/extensions/xforms/nsXFormsModelElement.h +++ b/extensions/xforms/nsXFormsModelElement.h @@ -253,6 +253,16 @@ public: return NS_OK; }; + NS_IMETHOD GetOnDeferredBindList(PRBool *onList) { + // dummy method, so does nothing + return NS_OK; + }; + + NS_IMETHOD SetOnDeferredBindList(PRBool putOnList) { + // dummy method, so does nothing + return NS_OK; + }; + // Called after nsXFormsAtoms is registered static NS_HIDDEN_(void) Startup(); @@ -268,6 +278,16 @@ public: nsIXFormsControlBase *aControl); static nsresult NeedsPostRefresh(nsIXFormsControl* aControl); + + // Sometimes a child that is on the post refresh list will want to force + // its containing xforms control to refresh. Like if an xf:item's label + // refreshes, it wants to reflect any of its changes in the xf:select or + // xf:select1 that contains it. This function will try to minimize that by + // allowing xforms controls that function as containers (like select/select1) + // to defer their refresh until most or all of its children have refreshed. + // Well, at least until every control on the sPostRefreshList has been + // processed. This will return PR_TRUE if the refresh will be deferred. + static PRBool ContainerNeedsPostRefresh(nsIXFormsControl* aControl); static void CancelPostRefresh(nsIXFormsControl* aControl); /** diff --git a/extensions/xforms/nsXFormsSelect1Element.cpp b/extensions/xforms/nsXFormsSelect1Element.cpp index 816586a90c7..aa03d017721 100644 --- a/extensions/xforms/nsXFormsSelect1Element.cpp +++ b/extensions/xforms/nsXFormsSelect1Element.cpp @@ -71,6 +71,9 @@ public: NS_IMETHOD BeginAddingChildren(); NS_IMETHOD DoneAddingChildren(); + // nsIXFormsControl + NS_IMETHOD Refresh(); + nsXFormsSelect1Element(const nsAString& aType) : nsXFormsDelegateStub(aType) {} @@ -138,6 +141,19 @@ nsXFormsSelect1Element::ChildRemoved(PRUint32 aIndex) return NS_OK; } +// nsIXFormsControl + +NS_IMETHODIMP +nsXFormsSelect1Element::Refresh() +{ + PRBool delayRefresh = nsXFormsModelElement::ContainerNeedsPostRefresh(this); + if (delayRefresh) { + return NS_OK; + } + + return nsXFormsDelegateStub::Refresh(); +} + NS_HIDDEN_(nsresult) NS_NewXFormsSelect1Element(nsIXTFElement **aResult) { diff --git a/extensions/xforms/nsXFormsSelectElement.cpp b/extensions/xforms/nsXFormsSelectElement.cpp index 8820920c6d0..f30ee9fffe3 100644 --- a/extensions/xforms/nsXFormsSelectElement.cpp +++ b/extensions/xforms/nsXFormsSelectElement.cpp @@ -52,6 +52,7 @@ #include "nsIXFormsUIWidget.h" #include "nsIDocument.h" #include "nsNetUtil.h" +#include "nsXFormsModelElement.h" class nsXFormsSelectElement : public nsXFormsDelegateStub { @@ -65,6 +66,9 @@ public: NS_IMETHOD ChildAppended(nsIDOMNode *aChild); NS_IMETHOD ChildRemoved(PRUint32 aIndex); + // nsIXFormsControl + NS_IMETHOD Refresh(); + #ifdef DEBUG_smaug virtual const char* Name() { return "select"; } #endif @@ -86,6 +90,8 @@ nsXFormsSelectElement::OnCreated(nsIXTFBindableElementWrapper *aWrapper) return NS_OK; } +// nsIXTFElement overrides + NS_IMETHODIMP nsXFormsSelectElement::ChildInserted(nsIDOMNode *aChild, PRUint32 aIndex) { @@ -107,6 +113,19 @@ nsXFormsSelectElement::ChildRemoved(PRUint32 aIndex) return NS_OK; } +// nsIXFormsControl + +NS_IMETHODIMP +nsXFormsSelectElement::Refresh() +{ + PRBool delayRefresh = nsXFormsModelElement::ContainerNeedsPostRefresh(this); + if (delayRefresh) { + return NS_OK; + } + + return nsXFormsDelegateStub::Refresh(); +} + NS_HIDDEN_(nsresult) NS_NewXFormsSelectElement(nsIXTFElement **aResult) {