Bug 660959 - Update link state less lazily r=bz
authorDavid Zbarsky <dzbarsky@gmail.com>
Mon, 14 Nov 2011 16:24:41 +1300
changeset 80216 61094ae6da186b46ed268d93be2e87f35c0f12b4
parent 80214 24c8d04f61748bdc73e1aeb9bfbac37967df3307
child 80217 03970d1fc9191ff84c8d1ec538b56ba4ecda16b6
push id323
push userrcampbell@mozilla.com
push dateTue, 15 Nov 2011 21:58:36 +0000
treeherderfx-team@3ea216303184 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs660959
milestone11.0a1
Bug 660959 - Update link state less lazily r=bz
content/base/public/Element.h
content/base/public/nsIDocument.h
content/base/src/Link.h
content/base/src/nsDocument.cpp
content/base/src/nsGenericElement.cpp
content/html/content/src/nsHTMLAnchorElement.cpp
content/html/content/src/nsHTMLAreaElement.cpp
content/html/content/src/nsHTMLLinkElement.cpp
content/html/content/test/Makefile.in
content/html/content/test/test_bug660959-1.html
content/html/content/test/test_bug660959-2.html
content/html/content/test/test_bug660959-3.html
content/mathml/content/src/nsMathMLElement.cpp
content/mathml/content/src/nsMathMLElement.h
content/svg/content/src/nsSVGAElement.cpp
content/svg/content/src/nsSVGAElement.h
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsFrameManager.cpp
layout/style/nsCSSRuleProcessor.cpp
layout/style/nsComputedDOMStyle.cpp
--- a/content/base/public/Element.h
+++ b/content/base/public/Element.h
@@ -106,53 +106,42 @@ public:
    */
   nsEventStates State() const {
     // mState is maintained by having whoever might have changed it
     // call UpdateState() or one of the other mState mutators.
     return mState;
   }
 
   /**
-   * Request an update of the link state for this element.  This will
-   * make sure that if the element is a link at all then either
-   * NS_EVENT_STATE_VISITED or NS_EVENT_STATE_UNVISITED is set in
-   * mState, and a history lookup kicked off if needed to find out
-   * whether the link is really visited.  This method will NOT send any
-   * state change notifications.  If you want them to happen for this
-   * call, you need to handle them yourself.
-   */
-  virtual void RequestLinkStateUpdate();
-
-  /**
    * Ask this element to update its state.  If aNotify is false, then
    * state change notifications will not be dispatched; in that
    * situation it is the caller's responsibility to dispatch them.
    *
    * In general, aNotify should only be false if we're guaranteed that
    * the element can't have a frame no matter what its style is
    * (e.g. if we're in the middle of adding it to the document or
    * removing it from the document).
    */
   void UpdateState(bool aNotify);
+  
+  /**
+   * Method to update mState with link state information.  This does not notify.
+   */
+  void UpdateLinkState(nsEventStates aState);
 
 protected:
   /**
    * Method to get the _intrinsic_ content state of this element.  This is the
    * state that is independent of the element's presentation.  To get the full
    * content state, use State().  See nsEventStates.h for
    * the possible bits that could be set here.
    */
   virtual nsEventStates IntrinsicState() const;
 
   /**
-   * Method to update mState with link state information.  This does not notify.
-   */
-  void UpdateLinkState(nsEventStates aState);
-
-  /**
    * Method to add state bits.  This should be called from subclass
    * constructors to set up our event state correctly at construction
    * time and other places where we don't want to notify a state
    * change.
    */
   void AddStatesSilently(nsEventStates aStates) {
     mState |= aStates;
   }
--- a/content/base/public/nsIDocument.h
+++ b/content/base/public/nsIDocument.h
@@ -161,16 +161,17 @@ public:
       mVisible(true),
       mRemovedFromDocShell(false),
       // mAllowDNSPrefetch starts true, so that we can always reliably && it
       // with various values that might disable it.  Since we never prefetch
       // unless we get a window, and in that case the docshell value will get
       // &&-ed in, this is safe.
       mAllowDNSPrefetch(true),
       mIsBeingUsedAsImage(false),
+      mHasLinksToUpdate(false),
       mPartID(0)
   {
     SetInDocument();
   }
 #endif
   
   /**
    * Let the document know that we're starting to load data into it.
@@ -1484,17 +1485,17 @@ public:
   virtual void SetScrollToRef(nsIURI *aDocumentURI) = 0;
   virtual void ScrollToRef() = 0;
   virtual void ResetScrolledToRefAlready() = 0;
   virtual void SetChangeScrollPosWhenScrollingToRef(bool aValue) = 0;
 
   /**
    * This method is similar to GetElementById() from nsIDOMDocument but it
    * returns a mozilla::dom::Element instead of a nsIDOMElement.
-   * It prevents converting nsIDOMElement to mozill:dom::Element which is
+   * It prevents converting nsIDOMElement to mozilla::dom::Element which is
    * already converted from mozilla::dom::Element.
    */
   virtual Element* GetElementById(const nsAString& aElementId) = 0;
 
   /**
    * This method returns _all_ the elements in this document which
    * have id aElementId, if there are any.  Otherwise it returns null.
    * The entries of the nsSmallVoidArray are Element*
@@ -1549,16 +1550,27 @@ public:
 
   virtual nsresult GetStateObject(nsIVariant** aResult) = 0;
 
   virtual nsDOMNavigationTiming* GetNavigationTiming() const = 0;
 
   virtual nsresult SetNavigationTiming(nsDOMNavigationTiming* aTiming) = 0;
 
   virtual Element* FindImageMap(const nsAString& aNormalizedMapName) = 0;
+  
+  // Add aLink to the set of links that need their status resolved. 
+  void RegisterPendingLinkUpdate(mozilla::dom::Link* aLink);
+  
+  // Remove aLink from the set of links that need their status resolved.
+  // This function must be called when links are removed from the document.
+  void UnregisterPendingLinkUpdate(mozilla::dom::Link* aElement);
+
+  // Update state on links in mLinksToUpdate.  This function must
+  // be called prior to selector matching.
+  void FlushPendingLinkUpdates();
 
 #define DEPRECATED_OPERATION(_op) e##_op,
   enum DeprecatedOperations {
 #include "nsDeprecatedOperationList.h"
     eDeprecatedOperationCount
   };
 #undef DEPRECATED_OPERATION
   void WarnOnceAbout(DeprecatedOperations aOperation);
@@ -1639,16 +1651,21 @@ protected:
   nsNodeInfoManager* mNodeInfoManager; // [STRONG]
   mozilla::css::Loader* mCSSLoader; // [STRONG]
 
   // The set of all object, embed, applet, video and audio elements for
   // which this is the owner document. (They might not be in the document.)
   // These are non-owning pointers, the elements are responsible for removing
   // themselves when they go away.
   nsAutoPtr<nsTHashtable<nsPtrHashKey<nsIContent> > > mFreezableElements;
+  
+  // The set of all links that need their status resolved.  Links must add themselves
+  // to this set by calling RegisterPendingLinkUpdate when added to a document and must
+  // remove themselves by calling UnregisterPendingLinkUpdate when removed from a document.
+  nsTHashtable<nsPtrHashKey<mozilla::dom::Link> > mLinksToUpdate;
 
   // SMIL Animation Controller, lazily-initialized in GetAnimationController
   nsRefPtr<nsSMILAnimationController> mAnimationController;
 
   // Table of element properties for this document.
   nsPropertyTable mPropertyTable;
   nsTArray<nsAutoPtr<nsPropertyTable> > mExtraPropertyTables;
 
@@ -1719,16 +1736,19 @@ protected:
 
   // True if we're an SVG document being used as an image.
   bool mIsBeingUsedAsImage;
 
   // True is this document is synthetic : stand alone image, video, audio
   // file, etc.
   bool mIsSyntheticDocument;
 
+  // True if this document has links whose state needs updating
+  bool mHasLinksToUpdate;
+
   // The document's script global object, the object from which the
   // document can get its script context and scope. This is the
   // *inner* window object.
   nsCOMPtr<nsIScriptGlobalObject> mScriptGlobalObject;
 
   // If mIsStaticDocument is true, mOriginalDocument points to the original
   // document.
   nsCOMPtr<nsIDocument> mOriginalDocument;
--- a/content/base/src/Link.h
+++ b/content/base/src/Link.h
@@ -104,16 +104,19 @@ public:
   /**
    * Invalidates any link caching, and resets the state to the default.
    *
    * @param aNotify
    *        true if ResetLinkState should notify the owning document about style
    *        changes or false if it should not.
    */
   void ResetLinkState(bool aNotify);
+  
+  // This method nevers returns a null element.
+  Element* GetElement() const { return mElement; }
 
 protected:
   virtual ~Link();
 
   bool HasCachedURI() const { return !!mCachedURI; }
 
 private:
   /**
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -1549,16 +1549,18 @@ nsDocument::nsDocument(const char* aCont
            ("DOCUMENT %p created", this));
 
   if (!gCspPRLog)
     gCspPRLog = PR_NewLogModule("CSP");
 #endif
 
   // Start out mLastStyleSheetSet as null, per spec
   SetDOMStringToNull(mLastStyleSheetSet);
+  
+  mLinksToUpdate.Init();
 }
 
 static PLDHashOperator
 ClearAllBoxObjects(const void* aKey, nsPIBoxObject* aBoxObject, void* aUserArg)
 {
   if (aBoxObject) {
     aBoxObject->Clear();
   }
@@ -7966,16 +7968,51 @@ nsIDocument::EnumerateFreezableElements(
                                         void* aData)
 {
   if (!mFreezableElements)
     return;
   EnumerateFreezablesData data = { aEnumerator, aData };
   mFreezableElements->EnumerateEntries(EnumerateFreezables, &data);
 }
 
+void
+nsIDocument::RegisterPendingLinkUpdate(Link* aLink)
+{
+  mLinksToUpdate.PutEntry(aLink);
+  mHasLinksToUpdate = true;
+}
+
+void
+nsIDocument::UnregisterPendingLinkUpdate(Link* aLink)
+{
+  if (!mHasLinksToUpdate)
+    return;
+    
+  mLinksToUpdate.RemoveEntry(aLink);
+}
+  
+static PLDHashOperator
+EnumeratePendingLinkUpdates(nsPtrHashKey<Link>* aEntry, void* aData)
+{
+  aEntry->GetKey()->GetElement()->UpdateLinkState(aEntry->GetKey()->LinkState());
+  return PL_DHASH_NEXT;
+}
+
+void
+nsIDocument::FlushPendingLinkUpdates() 
+{
+  if (!mHasLinksToUpdate)
+    return;
+    
+  nsAutoScriptBlocker scriptBlocker;
+  mLinksToUpdate.EnumerateEntries(EnumeratePendingLinkUpdates, nsnull);
+  mLinksToUpdate.Clear();
+  mHasLinksToUpdate = false;
+}
+
 already_AddRefed<nsIDocument>
 nsIDocument::CreateStaticClone(nsISupports* aCloneContainer)
 {
   nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(this);
   NS_ENSURE_TRUE(domDoc, nsnull);
   mCreatingStaticClone = true;
 
   // Make document use different container during cloning.
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -1244,21 +1244,16 @@ Element::NotifyStateChange(nsEventStates
   nsIDocument* doc = GetCurrentDoc();
   if (doc) {
     nsAutoScriptBlocker scriptBlocker;
     doc->ContentStateChanged(this, aStates);
   }
 }
 
 void
-Element::RequestLinkStateUpdate()
-{
-}
-
-void
 Element::UpdateLinkState(nsEventStates aState)
 {
   NS_ABORT_IF_FALSE(!aState.HasAtLeastOneOfStates(~(NS_EVENT_STATE_VISITED |
                                                     NS_EVENT_STATE_UNVISITED)),
                     "Unexpected link state bits");
   mState =
     (mState & ~(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED)) |
     aState;
@@ -5380,16 +5375,17 @@ inline static nsresult FindMatchingEleme
   NS_ENSURE_TRUE(selectorList, NS_OK);
 
   NS_ASSERTION(selectorList->mSelectors,
                "How can we not have any selectors?");
 
   nsIDocument* doc = aRoot->OwnerDoc();  
   TreeMatchContext matchingContext(false, nsRuleWalker::eRelevantLinkUnvisited,
                                    doc);
+  doc->FlushPendingLinkUpdates();
 
   // Fast-path selectors involving IDs.  We can only do this if aRoot
   // is in the document and the document is not in quirks mode, since
   // ID selectors are case-insensitive in quirks mode.  Also, only do
   // this if selectorList only has one selector, because otherwise
   // ordering the elements correctly is a pain.
   NS_ASSERTION(aRoot->IsElement() || aRoot->IsNodeOfType(nsINode::eDOCUMENT) ||
                !aRoot->IsInDoc(),
@@ -5488,16 +5484,17 @@ bool
 nsGenericElement::MozMatchesSelector(const nsAString& aSelector, nsresult* aResult)
 {
   nsAutoPtr<nsCSSSelectorList> selectorList;
   bool matches = false;
 
   *aResult = ParseSelectorList(this, aSelector, getter_Transfers(selectorList));
 
   if (NS_SUCCEEDED(*aResult)) {
+    OwnerDoc()->FlushPendingLinkUpdates();
     TreeMatchContext matchingContext(false,
                                      nsRuleWalker::eRelevantLinkUnvisited,
                                      OwnerDoc());
     matches = nsCSSRuleProcessor::SelectorListMatches(this, matchingContext,
                                                       selectorList);
   }
 
   return matches;
--- a/content/html/content/src/nsHTMLAnchorElement.cpp
+++ b/content/html/content/src/nsHTMLAnchorElement.cpp
@@ -113,17 +113,16 @@ public:
                               bool aNullParent = true);
   virtual bool IsHTMLFocusable(bool aWithMouse, bool *aIsFocusable, PRInt32 *aTabIndex);
 
   virtual nsresult PreHandleEvent(nsEventChainPreVisitor& aVisitor);
   virtual nsresult PostHandleEvent(nsEventChainPostVisitor& aVisitor);
   virtual bool IsLink(nsIURI** aURI) const;
   virtual void GetLinkTarget(nsAString& aTarget);
   virtual nsLinkState GetLinkState() const;
-  virtual void RequestLinkStateUpdate();
   virtual already_AddRefed<nsIURI> GetHrefURI() const;
 
   nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                    const nsAString& aValue, bool aNotify)
   {
     return SetAttr(aNameSpaceID, aName, nsnull, aValue, aNotify);
   }
   virtual nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
@@ -211,28 +210,37 @@ nsHTMLAnchorElement::BindToTree(nsIDocum
   Link::ResetLinkState(false);
 
   nsresult rv = nsGenericHTMLElement::BindToTree(aDocument, aParent,
                                                  aBindingParent,
                                                  aCompileEventHandlers);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Prefetch links
-  if (aDocument && nsHTMLDNSPrefetch::IsAllowed(OwnerDoc())) {
-    nsHTMLDNSPrefetch::PrefetchLow(this);
+  if (aDocument) {
+    aDocument->RegisterPendingLinkUpdate(this);
+    if (nsHTMLDNSPrefetch::IsAllowed(OwnerDoc())) {
+      nsHTMLDNSPrefetch::PrefetchLow(this);
+    }
   }
+
   return rv;
 }
 
 void
 nsHTMLAnchorElement::UnbindFromTree(bool aDeep, bool aNullParent)
 {
   // If this link is ever reinserted into a document, it might
   // be under a different xml:base, so forget the cached state now.
   Link::ResetLinkState(false);
+  
+  nsIDocument* doc = GetCurrentDoc();
+  if (doc) {
+    doc->UnregisterPendingLinkUpdate(this);
+  }
 
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 }
 
 bool
 nsHTMLAnchorElement::IsHTMLFocusable(bool aWithMouse,
                                      bool *aIsFocusable, PRInt32 *aTabIndex)
 {
@@ -384,22 +392,16 @@ nsHTMLAnchorElement::SetPing(const nsASt
 }
 
 nsLinkState
 nsHTMLAnchorElement::GetLinkState() const
 {
   return Link::GetLinkState();
 }
 
-void
-nsHTMLAnchorElement::RequestLinkStateUpdate()
-{
-  UpdateLinkState(Link::LinkState());
-}
-
 already_AddRefed<nsIURI>
 nsHTMLAnchorElement::GetHrefURI() const
 {
   return GetHrefURIForAnchors();
 }
 
 nsresult
 nsHTMLAnchorElement::SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
--- a/content/html/content/src/nsHTMLAreaElement.cpp
+++ b/content/html/content/src/nsHTMLAreaElement.cpp
@@ -94,17 +94,16 @@ public:
   NS_IMETHOD LinkAdded() { return NS_OK; }
   NS_IMETHOD LinkRemoved() { return NS_OK; }
 
   virtual nsresult PreHandleEvent(nsEventChainPreVisitor& aVisitor);
   virtual nsresult PostHandleEvent(nsEventChainPostVisitor& aVisitor);
   virtual bool IsLink(nsIURI** aURI) const;
   virtual void GetLinkTarget(nsAString& aTarget);
   virtual nsLinkState GetLinkState() const;
-  virtual void RequestLinkStateUpdate();
   virtual already_AddRefed<nsIURI> GetHrefURI() const;
 
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers);
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true);
   nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
@@ -208,28 +207,36 @@ nsHTMLAreaElement::GetLinkTarget(nsAStri
 }
 
 nsresult
 nsHTMLAreaElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers)
 {
   Link::ResetLinkState(false);
-
+  if (aDocument) {
+    aDocument->RegisterPendingLinkUpdate(this);
+  }
+  
   return nsGenericHTMLElement::BindToTree(aDocument, aParent,
                                                  aBindingParent,
                                                  aCompileEventHandlers);
 }
 
 void
 nsHTMLAreaElement::UnbindFromTree(bool aDeep, bool aNullParent)
 {
   // If this link is ever reinserted into a document, it might
   // be under a different xml:base, so forget the cached state now.
   Link::ResetLinkState(false);
+  
+  nsIDocument* doc = GetCurrentDoc();
+  if (doc) {
+    doc->UnregisterPendingLinkUpdate(this);
+  }
 
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 }
 
 nsresult
 nsHTMLAreaElement::SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                            nsIAtom* aPrefix, const nsAString& aValue,
                            bool aNotify)
@@ -309,22 +316,16 @@ nsHTMLAreaElement::SetPing(const nsAStri
 }
 
 nsLinkState
 nsHTMLAreaElement::GetLinkState() const
 {
   return Link::GetLinkState();
 }
 
-void
-nsHTMLAreaElement::RequestLinkStateUpdate()
-{
-  UpdateLinkState(Link::LinkState());
-}
-
 already_AddRefed<nsIURI>
 nsHTMLAreaElement::GetHrefURI() const
 {
   return GetHrefURIForAnchors();
 }
 
 nsEventStates
 nsHTMLAreaElement::IntrinsicState() const
--- a/content/html/content/src/nsHTMLLinkElement.cpp
+++ b/content/html/content/src/nsHTMLLinkElement.cpp
@@ -106,17 +106,16 @@ public:
   virtual nsresult UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aAttribute,
                              bool aNotify);
 
   virtual nsresult PreHandleEvent(nsEventChainPreVisitor& aVisitor);
   virtual nsresult PostHandleEvent(nsEventChainPostVisitor& aVisitor);
   virtual bool IsLink(nsIURI** aURI) const;
   virtual void GetLinkTarget(nsAString& aTarget);
   virtual nsLinkState GetLinkState() const;
-  virtual void RequestLinkStateUpdate();
   virtual already_AddRefed<nsIURI> GetHrefURI() const;
 
   virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;
 
   virtual nsEventStates IntrinsicState() const;
 
   virtual nsXPCClassInfo* GetClassInfo();
 protected:
@@ -208,16 +207,20 @@ nsHTMLLinkElement::BindToTree(nsIDocumen
                               bool aCompileEventHandlers)
 {
   Link::ResetLinkState(false);
 
   nsresult rv = nsGenericHTMLElement::BindToTree(aDocument, aParent,
                                                  aBindingParent,
                                                  aCompileEventHandlers);
   NS_ENSURE_SUCCESS(rv, rv);
+  
+  if (aDocument) {
+    aDocument->RegisterPendingLinkUpdate(this);
+  }
 
   void (nsHTMLLinkElement::*update)() = &nsHTMLLinkElement::UpdateStyleSheetInternal;
   nsContentUtils::AddScriptRunner(NS_NewRunnableMethod(this, update));
 
   CreateAndDispatchEvent(aDocument, NS_LITERAL_STRING("DOMLinkAdded"));
 
   return rv;
 }
@@ -241,16 +244,19 @@ nsHTMLLinkElement::UnbindFromTree(bool a
 {
   // If this link is ever reinserted into a document, it might
   // be under a different xml:base, so forget the cached state now.
   Link::ResetLinkState(false);
 
   // Once we have XPCOMGC we shouldn't need to call UnbindFromTree during Unlink
   // and so this messy event dispatch can go away.
   nsCOMPtr<nsIDocument> oldDoc = GetCurrentDoc();
+  if (oldDoc) {
+    oldDoc->UnregisterPendingLinkUpdate(this);
+  }
   CreateAndDispatchEvent(oldDoc, NS_LITERAL_STRING("DOMLinkRemoved"));
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
   UpdateStyleSheetInternal(oldDoc);
 }
 
 void
 nsHTMLLinkElement::CreateAndDispatchEvent(nsIDocument* aDoc,
                                           const nsAString& aEventName)
@@ -374,22 +380,16 @@ nsHTMLLinkElement::GetLinkTarget(nsAStri
 }
 
 nsLinkState
 nsHTMLLinkElement::GetLinkState() const
 {
   return Link::GetLinkState();
 }
 
-void
-nsHTMLLinkElement::RequestLinkStateUpdate()
-{
-  UpdateLinkState(Link::LinkState());
-}
-
 already_AddRefed<nsIURI>
 nsHTMLLinkElement::GetHrefURI() const
 {
   return GetHrefURIForAnchors();
 }
 
 already_AddRefed<nsIURI>
 nsHTMLLinkElement::GetStyleSheetURL(bool* aIsInline)
--- a/content/html/content/test/Makefile.in
+++ b/content/html/content/test/Makefile.in
@@ -269,16 +269,19 @@ include $(topsrcdir)/config/rules.mk
 		test_bug660663.html \
 		test_bug664299.html \
 		test_bug666200.html \
 		test_bug666666.html \
 		test_bug674558.html \
 		test_bug583533.html \
 		test_restore_from_parser_fragment.html \
 		test_bug617528.html \
+		test_bug660959-1.html \
+		test_bug660959-2.html \
+		test_bug660959-3.html \
 		test_checked.html \
 		test_bug677658.html \
 		test_bug677463.html \
 		test_bug682886.html \
 		file_fullscreen-api.html \
 		file_fullscreen-api-keys.html \
 		test_fullscreen-api.html \
 		file_fullscreen-plugins.html \
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug660959-1.html
@@ -0,0 +1,26 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=660959
+-->
+<head>
+  <title>Test for Bug 660959</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="reflect.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=660959">Mozilla Bug 660959</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <a href="#" id="testa"></a>
+</div>
+<pre id="test">
+<script>
+    is($("content").querySelector(":link, :visited"), $("testa"),
+       "Should find a link even in a display:none subtree");
+</script>
+</pre>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug660959-2.html
@@ -0,0 +1,31 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=660959
+-->
+<head>
+  <title>Test for Bug 660959</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="reflect.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <style>
+    :link, :visited {
+      color: red;
+    }
+  </style>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=660959">Mozilla Bug 660959</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <a href="#" id="a"></a>
+</div>
+<pre id="test">
+<script type="application/javascript">
+  var a = document.getElementById("a");
+  is(window.getComputedStyle(a).color, "rgb(255, 0, 0)", "Link is not right color?");
+</script>
+</pre>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug660959-3.html
@@ -0,0 +1,29 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=660959
+-->
+<head>
+  <title>Test for Bug 660959</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="reflect.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=660959">Mozilla Bug 660959</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <a href="http://www.example.com"></a>
+  <div id="foo">
+    <span id="test"></span>
+  </div>
+</div>
+<pre id="test">
+<script>
+    is($("foo").querySelector(":link + * span, :visited + * span"), $("test"),
+       "Should be able to find link siblings even in a display:none subtree");
+</script>
+</pre>
+</body>
+</html>
--- a/content/mathml/content/src/nsMathMLElement.cpp
+++ b/content/mathml/content/src/nsMathMLElement.cpp
@@ -84,41 +84,50 @@ nsMathMLElement::BindToTree(nsIDocument*
 
   Link::ResetLinkState(false);
 
   nsresult rv = nsMathMLElementBase::BindToTree(aDocument, aParent,
                                                 aBindingParent,
                                                 aCompileEventHandlers);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  if (aDocument && !aDocument->GetMathMLEnabled()) {
-    // Enable MathML and setup the style sheet during binding, not element
-    // construction, because we could move a MathML element from the document
-    // that created it to another document.
-    aDocument->SetMathMLEnabled();
-    aDocument->EnsureCatalogStyleSheet(kMathMLStyleSheetURI);
+  if (aDocument) {
+    aDocument->RegisterPendingLinkUpdate(this);
+    
+    if (!aDocument->GetMathMLEnabled()) {
+      // Enable MathML and setup the style sheet during binding, not element
+      // construction, because we could move a MathML element from the document
+      // that created it to another document.
+      aDocument->SetMathMLEnabled();
+      aDocument->EnsureCatalogStyleSheet(kMathMLStyleSheetURI);
 
-    // Rebuild style data for the presshell, because style system
-    // optimizations may have taken place assuming MathML was disabled.
-    // (See nsRuleNode::CheckSpecifiedProperties.)
-    nsCOMPtr<nsIPresShell> shell = aDocument->GetShell();
-    if (shell) {
-      shell->GetPresContext()->PostRebuildAllStyleDataEvent(nsChangeHint(0));
+      // Rebuild style data for the presshell, because style system
+      // optimizations may have taken place assuming MathML was disabled.
+      // (See nsRuleNode::CheckSpecifiedProperties.)
+      nsCOMPtr<nsIPresShell> shell = aDocument->GetShell();
+      if (shell) {
+        shell->GetPresContext()->PostRebuildAllStyleDataEvent(nsChangeHint(0));
+      }
     }
   }
 
   return rv;
 }
 
 void
 nsMathMLElement::UnbindFromTree(bool aDeep, bool aNullParent)
 {
   // If this link is ever reinserted into a document, it might
   // be under a different xml:base, so forget the cached state now.
   Link::ResetLinkState(false);
+  
+  nsIDocument* doc = GetCurrentDoc();
+  if (doc) {
+    doc->UnregisterPendingLinkUpdate(this);
+  }
 
   nsMathMLElementBase::UnbindFromTree(aDeep, aNullParent);
 }
 
 bool
 nsMathMLElement::ParseAttribute(PRInt32 aNamespaceID,
                                 nsIAtom* aAttribute,
                                 const nsAString& aValue,
@@ -617,22 +626,16 @@ nsMathMLElement::GetLinkTarget(nsAString
 }
 
 nsLinkState
 nsMathMLElement::GetLinkState() const
 {
   return Link::GetLinkState();
 }
 
-void
-nsMathMLElement::RequestLinkStateUpdate()
-{
-  UpdateLinkState(Link::LinkState());
-}
-
 already_AddRefed<nsIURI>
 nsMathMLElement::GetHrefURI() const
 {
   nsCOMPtr<nsIURI> hrefURI;
   return IsLink(getter_AddRefs(hrefURI)) ? hrefURI.forget() : nsnull;
 }
 
 nsresult
--- a/content/mathml/content/src/nsMathMLElement.h
+++ b/content/mathml/content/src/nsMathMLElement.h
@@ -111,17 +111,16 @@ public:
 
   NS_IMETHOD LinkAdded() { return NS_OK; }
   NS_IMETHOD LinkRemoved() { return NS_OK; }
   virtual bool IsFocusable(PRInt32 *aTabIndex = nsnull,
                              bool aWithMouse = false);
   virtual bool IsLink(nsIURI** aURI) const;
   virtual void GetLinkTarget(nsAString& aTarget);
   virtual nsLinkState GetLinkState() const;
-  virtual void RequestLinkStateUpdate();
   virtual already_AddRefed<nsIURI> GetHrefURI() const;
   nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                    const nsAString& aValue, bool aNotify)
   {
     return SetAttr(aNameSpaceID, aName, nsnull, aValue, aNotify);
   }
   virtual nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                            nsIAtom* aPrefix, const nsAString& aValue,
--- a/content/svg/content/src/nsSVGAElement.cpp
+++ b/content/svg/content/src/nsSVGAElement.cpp
@@ -140,42 +140,45 @@ nsSVGAElement::BindToTree(nsIDocument *a
                           bool aCompileEventHandlers)
 {
   Link::ResetLinkState(false);
 
   nsresult rv = nsSVGAElementBase::BindToTree(aDocument, aParent,
                                               aBindingParent,
                                               aCompileEventHandlers);
   NS_ENSURE_SUCCESS(rv, rv);
+  
+  if (aDocument) {
+    aDocument->RegisterPendingLinkUpdate(this);
+  }
 
   return NS_OK;
 }
 
 void
 nsSVGAElement::UnbindFromTree(bool aDeep, bool aNullParent)
 {
   // If this link is ever reinserted into a document, it might
   // be under a different xml:base, so forget the cached state now.
   Link::ResetLinkState(false);
+  
+  nsIDocument* doc = GetCurrentDoc();
+  if (doc) {
+    doc->UnregisterPendingLinkUpdate(this);
+  }
 
   nsSVGAElementBase::UnbindFromTree(aDeep, aNullParent);
 }
 
 nsLinkState
 nsSVGAElement::GetLinkState() const
 {
   return Link::GetLinkState();
 }
 
-void
-nsSVGAElement::RequestLinkStateUpdate()
-{
-  UpdateLinkState(Link::LinkState());
-}
-
 already_AddRefed<nsIURI>
 nsSVGAElement::GetHrefURI() const
 {
   nsCOMPtr<nsIURI> hrefURI;
   return IsLink(getter_AddRefs(hrefURI)) ? hrefURI.forget() : nsnull;
 }
 
 
--- a/content/svg/content/src/nsSVGAElement.h
+++ b/content/svg/content/src/nsSVGAElement.h
@@ -86,17 +86,16 @@ public:
                               bool aCompileEventHandlers);
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true);
   NS_IMETHOD_(bool) IsAttributeMapped(const nsIAtom* aAttribute) const;
   virtual bool IsFocusable(PRInt32 *aTabIndex = nsnull, bool aWithMouse = false);
   virtual bool IsLink(nsIURI** aURI) const;
   virtual void GetLinkTarget(nsAString& aTarget);
   virtual nsLinkState GetLinkState() const;
-  virtual void RequestLinkStateUpdate();
   virtual already_AddRefed<nsIURI> GetHrefURI() const;
   virtual nsEventStates IntrinsicState() const;
   nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                    const nsAString& aValue, bool aNotify)
   {
     return SetAttr(aNameSpaceID, aName, nsnull, aValue, aNotify);
   }
   virtual nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -4542,17 +4542,18 @@ nsCSSFrameConstructor::ResolveStyleConte
 }
 
 already_AddRefed<nsStyleContext>
 nsCSSFrameConstructor::ResolveStyleContext(nsStyleContext* aParentStyleContext,
                                            nsIContent* aContent,
                                            nsFrameConstructorState* aState)
 {
   nsStyleSet *styleSet = mPresShell->StyleSet();
-
+  aContent->OwnerDoc()->FlushPendingLinkUpdates();
+  
   if (aContent->IsElement()) {
     if (aState) {
       return styleSet->ResolveStyleFor(aContent->AsElement(),
                                        aParentStyleContext,
                                        aState->mTreeMatchContext);
     }
     return styleSet->ResolveStyleFor(aContent->AsElement(), aParentStyleContext);
 
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -1106,16 +1106,17 @@ nsFrameManager::ReResolveStyleContext(ns
     // |content| is the node that we used for rule matching of
     // normal elements (not pseudo-elements) and for which we generate
     // framechange hints if we need them.
     // XXXldb Why does it make sense to use aParentContent?  (See
     // comment above assertion at start of function.)
     nsIContent* content = localContent ? localContent : aParentContent;
 
     if (content && content->IsElement()) {
+      content->OwnerDoc()->FlushPendingLinkUpdates();
       RestyleTracker::RestyleData restyleData;
       if (aRestyleTracker.GetRestyleData(content->AsElement(), &restyleData)) {
         if (NS_UpdateHint(aMinChange, restyleData.mChangeHint)) {
           aChangeList->AppendChange(aFrame, content, restyleData.mChangeHint);
         }
         aRestyleHint = nsRestyleHint(aRestyleHint | restyleData.mRestyleHint);
       }
     }
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1244,18 +1244,16 @@ nsCSSRuleProcessor::GetWindowsThemeIdent
   return sWinThemeId;
 }
 #endif
 
 /* static */
 nsEventStates
 nsCSSRuleProcessor::GetContentState(Element* aElement)
 {
-  // FIXME: RequestLinkStateUpdate is a hack; see bug 660959.
-  aElement->RequestLinkStateUpdate();
   nsEventStates state = aElement->State();
 
   // If we are not supposed to mark visited links as such, be sure to
   // flip the bits appropriately.  We want to do this here, rather
   // than in GetContentStateForVisitedHandling, so that we don't
   // expose that :visited support is disabled to the Web page.
   if (state.HasState(NS_EVENT_STATE_VISITED) &&
       (!gSupportVisitedPseudo ||
@@ -1266,18 +1264,16 @@ nsCSSRuleProcessor::GetContentState(Elem
   }
   return state;
 }
 
 /* static */
 bool
 nsCSSRuleProcessor::IsLink(Element* aElement)
 {
-  // FIXME: RequestLinkStateUpdate is a hack; see bug 660959.
-  aElement->RequestLinkStateUpdate();
   nsEventStates state = aElement->State();
   return state.HasAtLeastOneOfStates(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED);
 }
 
 /* static */
 nsEventStates
 nsCSSRuleProcessor::GetContentStateForVisitedHandling(
                      Element* aElement,
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -442,16 +442,17 @@ nsComputedDOMStyle::GetPropertyCSSValue(
                                         nsIDOMCSSValue** aReturn)
 {
   NS_ASSERTION(!mStyleContextHolder, "bad state");
 
   *aReturn = nsnull;
 
   nsCOMPtr<nsIDocument> document = do_QueryReferent(mDocumentWeak);
   NS_ENSURE_TRUE(document, NS_ERROR_NOT_AVAILABLE);
+  document->FlushPendingLinkUpdates();
 
   nsCSSProperty prop = nsCSSProps::LookupProperty(aPropertyName);
 
   const ComputedStyleMapEntry* propEntry = nsnull;
   {
     PRUint32 length = 0;
     const ComputedStyleMapEntry* propMap = GetQueryablePropertyMap(&length);
     for (PRUint32 i = 0; i < length; ++i) {