166637 - [FIX]Remove ContentID forever from our hearts

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P1)

Description

[John Keiser (jkeiser)]

ContentID is only used as a unique identifier for frame state restoration key,
and it isn't even all that good an identifier.  Let us use an XPath ID of some
sort instead and spare ourselves the 4 bytes per element and the pain involved.

Comment 1

[Johnny Stenback (:jst)]

The content ID is still also used by the accessability code in one place, but
that code can be replaced by something along these lines (according to aaronl):

NS_IMETHODIMP nsAccessible::GetAccId(PRInt32 *aAccId)
{
  *aAccId = - NS_REINTERPRET_CAST(PRInt32, (mDOMNode.get()));
  return NS_OK;
}

Comment 2

[John Keiser (jkeiser)]

Dependent on layout history rewrite (though it admittedly could be done in
parallel with some merging)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

The time has come.  Let us remove the evil.

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 202770 [details] [diff] [review]
Proposed patch

>Index: content/base/public/nsIContent.h
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/base/public/nsIContent.h,v
>retrieving revision 3.120
>diff -u -p -d -8 -r3.120 nsIContent.h
>--- content/base/public/nsIContent.h	2 Nov 2005 00:41:36 -0000	3.120
>+++ content/base/public/nsIContent.h	11 Nov 2005 21:41:06 -0000
>@@ -60,18 +60,18 @@ class nsIDOMRange;
> class nsIEventListenerManager;
> class nsIURI;
> class nsICSSStyleRule;
> class nsRuleWalker;
> class nsAttrValue;
> 
> // IID for the nsIContent interface
> #define NS_ICONTENT_IID       \
>-{ 0x08b87f67, 0x2f64, 0x437b, \
>- { 0x93, 0x35, 0x02, 0x60, 0x17, 0x5c, 0x0e, 0xc2 } }
>+{ 0x5d098839, 0x389d, 0x41db, \
>+ { 0x8f, 0x53, 0x59, 0x07, 0xbf, 0x90, 0x0d, 0x4e } }
> 
> /**
>  * A node of content in a document's content model. This interface
>  * is supported by all content objects.
>  */
> class nsIContent : public nsISupports {
> public:
>   NS_DEFINE_STATIC_IID_ACCESSOR(NS_ICONTENT_IID)
>@@ -479,37 +479,16 @@ public:
>    *        nsEventStatus_eIgnore
>    */
>   virtual nsresult HandleDOMEvent(nsPresContext* aPresContext,
>                                   nsEvent* aEvent, nsIDOMEvent** aDOMEvent,
>                                   PRUint32 aFlags,
>                                   nsEventStatus* aEventStatus) = 0;
> 
>   /**
>-   * Get a unique ID for this piece of content.
>-   * This ID is used as a key to store state information 
>-   * about this content object and its associated frame object.
>-   * The state information is stored in a dictionary that is
>-   * manipulated by the frame manager (nsIFrameManager) inside layout.
>-   * An opaque pointer to this dictionary is passed to the session
>-   * history as a handle associated with the current document's state
>-   *
>-   * These methods are DEPRECATED, DON'T USE THEM!!!
>-   *
>-   */
>-  virtual PRUint32 ContentID() const = 0;
>-  /**
>-   * Set the unique content ID for this content.
>-   * @param aID the ID to set
>-   */
>-  virtual void SetContentID(PRUint32 aID)
>-  {
>-  }
>-
>-  /**
>    * Set the focus on this content.  This is generally something for the event
>    * state manager to do, not ordinary people.  Ordinary people should do
>    * something like nsGenericHTMLElement::SetElementFocus().  This method is
>    * the end result, the point where the content finds out it has been focused.
>    * 
>    * All content elements are potentially focusable.
>    *
>    * @param aPresContext the pres context
>Index: content/base/public/nsIDocument.h
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/base/public/nsIDocument.h,v
>retrieving revision 3.218
>diff -u -p -d -8 -r3.218 nsIDocument.h
>--- content/base/public/nsIDocument.h	8 Nov 2005 22:45:31 -0000	3.218
>+++ content/base/public/nsIDocument.h	11 Nov 2005 21:06:37 -0000
>@@ -90,41 +90,33 @@ class nsICSSLoader;
> class nsHTMLStyleSheet;
> class nsIHTMLCSSStyleSheet;
> class nsILayoutHistoryState;
> class nsIVariant;
> class nsIDOMUserDataHandler;
> 
> // IID for the nsIDocument interface
> #define NS_IDOCUMENT_IID      \
>-{ 0x6120dffe, 0xf8fc, 0x47b1, \
>- { 0x98, 0xd3, 0x97, 0x68, 0x0c, 0xc9, 0xc3, 0xcd } }
>-
>-// The base value for the content ID counter.
>-// This counter is used by the document to 
>-// assign a monotonically increasing ID to each content
>-// object it creates
>-#define NS_CONTENT_ID_COUNTER_BASE 10000
>-
>+{ 0x65d26965, 0x8fed, 0x4f55, \
>+ { 0x80, 0xef, 0xaa, 0xd0, 0x1d, 0xd6, 0x92, 0xfd } }
> 
> // Flag for AddStyleSheet().
> #define NS_STYLESHEET_FROM_CATALOG                (1 << 0)
> 
> //----------------------------------------------------------------------
> 
> // Document interface
> class nsIDocument : public nsISupports
> {
> public:
>   NS_DEFINE_STATIC_IID_ACCESSOR(NS_IDOCUMENT_IID)
>   NS_DECL_AND_IMPL_ZEROING_OPERATOR_NEW
> 
>   nsIDocument()
>     : mCharacterSet(NS_LITERAL_CSTRING("ISO-8859-1")),
>-      mNextContentID(NS_CONTENT_ID_COUNTER_BASE),
>       mNodeInfoManager(nsnull),
>       mPartID(0)
>   {
>   }
> 
>   virtual nsresult StartDocumentLoad(const char* aCommand,
>                                      nsIChannel* aChannel,
>                                      nsILoadGroup* aLoadGroup,
>@@ -559,21 +551,16 @@ public:
>                                   nsEventStatus* aEventStatus) = 0;
> 
>   /**
>    * Flush notifications for this document and its parent documents
>    * (since those may affect the layout of this one).
>    */
>   virtual void FlushPendingNotifications(mozFlushType aType) = 0;
> 
>-  PRInt32 GetAndIncrementContentID()
>-  {
>-    return mNextContentID++;
>-  }
>-
>   nsIBindingManager* BindingManager() const
>   {
>     return mBindingManager;
>   }
> 
>   /**
>    * Only to be used inside Gecko, you can't really do anything with the
>    * pointer outside Gecko anyway.
>@@ -931,20 +918,16 @@ protected:
> 
>   // This is just a weak pointer; the parent document owns its children.
>   nsIDocument* mParentDocument;
> 
>   // A weak reference to the only child element, or null if no
>   // such element exists.
>   nsIContent* mRootContent;
> 
>-  // A content ID counter used to give a monotonically increasing ID
>-  // to the content objects in the document's content model
>-  PRInt32 mNextContentID;
>-
>   nsCOMPtr<nsIBindingManager> mBindingManager;
>   nsNodeInfoManager* mNodeInfoManager; // [STRONG]
> 
>   nsICSSLoader* mCSSLoader; // [STRONG; not a COMPtr to avoid
>                             // including nsICSSLoader.h; the ownership
>                             // is managed by nsDocument]
> 
>   // Table of element properties for this document.
>Index: content/base/src/nsDocument.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/base/src/nsDocument.cpp,v
>retrieving revision 3.602
>diff -u -p -d -8 -r3.602 nsDocument.cpp
>--- content/base/src/nsDocument.cpp	8 Nov 2005 22:45:33 -0000	3.602
>+++ content/base/src/nsDocument.cpp	11 Nov 2005 21:02:22 -0000
>@@ -4893,18 +4893,16 @@ nsresult
> nsDocument::CreateElement(nsINodeInfo *aNodeInfo, PRInt32 aElementType,
>                           nsIContent** aResult)
> {
>   nsCOMPtr<nsIContent> content;
>   nsresult rv = NS_NewElement(getter_AddRefs(content), aElementType,
>                               aNodeInfo);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>-  content->SetContentID(mNextContentID++);
>-
>   content.swap(*aResult);
> 
>   return NS_OK;
> }
> 
> PRBool
> nsDocument::IsSafeToFlush() const
> {
>Index: content/base/src/nsContentUtils.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/base/src/nsContentUtils.cpp,v
>retrieving revision 1.129
>diff -u -p -d -8 -r1.129 nsContentUtils.cpp
>--- content/base/src/nsContentUtils.cpp	3 Nov 2005 22:47:06 -0000	1.129
>+++ content/base/src/nsContentUtils.cpp	12 Nov 2005 00:38:12 -0000
>@@ -1417,62 +1417,67 @@ nsContentUtils::GenerateStateKey(nsICont
>                                  nsIStatefulFrame::SpecialStateID aID,
>                                  nsACString& aKey)
> {
>   aKey.Truncate();
> 
>   PRUint32 partID = aDocument ? aDocument->GetPartID() : 0;
> 
>   // SpecialStateID case - e.g. scrollbars around the content window
>-  // The key in this case is the special state id (always < min(contentID))
>+  // The key in this case is a special state id
>   if (nsIStatefulFrame::eNoID != aID) {
>     KeyAppendInt(partID, aKey);  // first append a partID
>     KeyAppendInt(aID, aKey);
>     return NS_OK;
>   }
> 
>   // We must have content if we're not using a special state id
>   NS_ENSURE_TRUE(aContent, NS_ERROR_FAILURE);
> 
>   // Don't capture state for anonymous content
>-  PRUint32 contentID = aContent->ContentID();
>-  if (!contentID) {
>+  if (aContent->IsNativeAnonymous() || aContent->GetBindingParent()) {
>     return NS_OK;
>   }
> 
>   nsCOMPtr<nsIDOMElement> element(do_QueryInterface(aContent));
>   if (element && IsAutocompleteOff(element)) {
>     return NS_OK;
>   }
> 
>   nsCOMPtr<nsIHTMLDocument> htmlDocument(do_QueryInterface(aContent->GetCurrentDoc()));
> 
>   KeyAppendInt(partID, aKey);  // first append a partID
>+  // Make sure we can't possibly collide with an nsIStatefulFrame
>+  // special id of some sort
>+  KeyAppendInt(nsIStatefulFrame::eNoID, aKey);
>   PRBool generatedUniqueKey = PR_FALSE;
> 
>   if (htmlDocument) {
>     // Flush our content model so it'll be up to date
>     aContent->GetCurrentDoc()->FlushPendingNotifications(Flush_Content);
> 
>     nsContentList *htmlForms = htmlDocument->GetForms();
>     nsRefPtr<nsContentList> htmlFormControls =
>       GetFormControlElements(aDocument);
> 
>     NS_ENSURE_TRUE(htmlForms && htmlFormControls, NS_ERROR_OUT_OF_MEMORY);
> 
>-    // If we have a form control and can calculate form information, use
>-    // that as the key - it is more reliable than contentID.
>+    // If we have a form control and can calculate form information, use that
>+    // as the key - it is more reliable than just recording position in the
>+    // DOM.
>+    // XXXbz Is it, really?  We have bugs on this, I think...
>     // Important to have a unique key, and tag/type/name may not be.
>     //
>     // If the control has a form, the format of the key is:
>-    // type>IndOfFormInDoc>IndOfControlInForm>FormName>name
>+    // f>type>IndOfFormInDoc>IndOfControlInForm>FormName>name
>     // else:
>-    // type>IndOfControlInDoc>name
>+    // d>type>IndOfControlInDoc>name
>     //
>     // XXX We don't need to use index if name is there
>+    // XXXbz We don't?  Why not?  I don't follow.
>     //
>     nsCOMPtr<nsIFormControl> control(do_QueryInterface(aContent));
>     if (control && htmlFormControls && htmlForms) {
> 
>       // Append the control type
>       KeyAppendInt(control->GetType(), aKey);
> 
>       // If in a form, add form name / index of form / index in form
>@@ -1481,16 +1486,18 @@ nsContentUtils::GenerateStateKey(nsICont
>       control->GetForm(getter_AddRefs(formElement));
>       if (formElement) {
> 
>         if (IsAutocompleteOff(formElement)) {
>           aKey.Truncate();
>           return NS_OK;
>         }
> 
>+        KeyAppendString(NS_LITERAL_CSTRING("f"), aKey);
>+
>         // Append the index of the form in the document
>         nsCOMPtr<nsIContent> formContent(do_QueryInterface(formElement));
>         index = htmlForms->IndexOf(formContent, PR_FALSE);
>         if (index <= -1) {
>           //
>           // XXX HACK this uses some state that was dumped into the document
>           // specifically to fix bug 138892.  What we are trying to do is *guess*
>           // which form this control's state is found in, with the highly likely
>@@ -1514,16 +1521,18 @@ nsContentUtils::GenerateStateKey(nsICont
> 
>         // Append the form name
>         nsAutoString formName;
>         formElement->GetName(formName);
>         KeyAppendString(formName, aKey);
> 
>       } else {
> 
>+        KeyAppendString(NS_LITERAL_CSTRING("d"), aKey);
>+
>         // If not in a form, add index of control in document
>         // Less desirable than indexing by form info.
> 
>         // Hash by index of control in doc (we are not in a form)
>         // These are important as they are unique, and type/name may not be.
> 
>         // Note that we've already flushed content, so there's no
>         // reason to flush it again.
>@@ -1537,20 +1546,32 @@ nsContentUtils::GenerateStateKey(nsICont
>       // Append the control name
>       nsAutoString name;
>       aContent->GetAttr(kNameSpaceID_None, nsHTMLAtoms::name, name);
>       KeyAppendString(name, aKey);
>     }
>   }
> 
>   if (!generatedUniqueKey) {
>-
>-    // Either we didn't have a form control or we aren't in an HTML document
>-    // so we can't figure out form info, hash by content ID instead :(
>-    KeyAppendInt(contentID, aKey);
>+    // Either we didn't have a form control or we aren't in an HTML document so
>+    // we can't figure out form info.  First append a character that is not "d"
>+    // or "f" to disambiguate from the case when we were a form control in an
>+    // HTML document.
>+    KeyAppendString(NS_LITERAL_CSTRING("o"), aKey);
>+    
>+    // Now start at aContent and append the indices of it and all its ancestors
>+    // in their containers.  That should at least pin down its position in the
>+    // DOM...
>+    nsIContent* parent = aContent->GetParent();
>+    nsIContent* content = aContent;
>+    while (parent) {
>+      KeyAppendInt(parent->IndexOf(content), aKey);
>+      content = parent;
>+      parent = content->GetParent();
>+    }
>   }
> 
>   return NS_OK;
> }
> 
> // static
> nsresult
> nsContentUtils::NewURIWithDocumentCharset(nsIURI** aResult,
>Index: content/base/src/nsGenericDOMDataNode.h
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/base/src/nsGenericDOMDataNode.h,v
>retrieving revision 3.107
>diff -u -p -d -8 -r3.107 nsGenericDOMDataNode.h
>--- content/base/src/nsGenericDOMDataNode.h	2 Nov 2005 00:41:36 -0000	3.107
>+++ content/base/src/nsGenericDOMDataNode.h	11 Nov 2005 21:09:19 -0000
>@@ -204,17 +204,16 @@ public:
> #ifdef DEBUG
>   virtual void List(FILE* out, PRInt32 aIndent) const;
>   virtual void DumpContent(FILE* out, PRInt32 aIndent, PRBool aDumpAll) const;
> #endif
>   virtual nsresult HandleDOMEvent(nsPresContext* aPresContext,
>                                   nsEvent* aEvent, nsIDOMEvent** aDOMEvent,
>                                   PRUint32 aFlags,
>                                   nsEventStatus* aEventStatus);
>-  virtual PRUint32 ContentID() const;
>   virtual nsresult RangeAdd(nsIDOMRange* aRange);
>   virtual void RangeRemove(nsIDOMRange* aRange);
>   virtual const nsVoidArray *GetRangeList() const;
> 
>   virtual nsIContent *GetBindingParent() const;
>   virtual PRBool IsContentOfType(PRUint32 aFlags) const;
> 
>   virtual nsresult GetListenerManager(nsIEventListenerManager **aResult);
>Index: content/base/src/nsGenericDOMDataNode.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/base/src/nsGenericDOMDataNode.cpp,v
>retrieving revision 3.165
>diff -u -p -d -8 -r3.165 nsGenericDOMDataNode.cpp
>--- content/base/src/nsGenericDOMDataNode.cpp	2 Nov 2005 00:41:36 -0000	3.165
>+++ content/base/src/nsGenericDOMDataNode.cpp	11 Nov 2005 21:09:12 -0000
>@@ -896,22 +896,16 @@ nsGenericDOMDataNode::HandleDOMEvent(nsP
>     // we're in the process of dispatching this event.
>     NS_MARK_EVENT_DISPATCH_DONE(aEvent);
>   }
> 
>   return ret;
> }
> 
> PRUint32
>-nsGenericDOMDataNode::ContentID() const
>-{
>-  return 0;
>-}
>-
>-PRUint32
> nsGenericDOMDataNode::GetChildCount() const
> {
>   return 0;
> }
> 
> nsIContent *
> nsGenericDOMDataNode::GetChildAt(PRUint32 aIndex) const
> {
>Index: content/base/src/nsGenericElement.h
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/base/src/nsGenericElement.h,v
>retrieving revision 3.186
>diff -u -p -d -8 -r3.186 nsGenericElement.h
>--- content/base/src/nsGenericElement.h	2 Nov 2005 00:41:36 -0000	3.186
>+++ content/base/src/nsGenericElement.h	11 Nov 2005 21:43:54 -0000
>@@ -92,30 +92,16 @@ typedef unsigned long PtrBits;
> #define GENERIC_ELEMENT_HAS_PROPERTIES         0x00000010U
> 
> /** Whether this content may have a frame */
> #define GENERIC_ELEMENT_MAY_HAVE_FRAME         0x00000020U
> 
> /** Three bits are element type specific. */
> #define ELEMENT_TYPE_SPECIFIC_BITS_OFFSET      6
> 
>-/** The number of bits to shift the bit field to get at the content ID */
>-#define GENERIC_ELEMENT_CONTENT_ID_BITS_OFFSET 9
>-
>-/** This mask masks out the bits that are used for the content ID */
>-#define GENERIC_ELEMENT_CONTENT_ID_MASK \
>-  ((~PtrBits(0)) << GENERIC_ELEMENT_CONTENT_ID_BITS_OFFSET)
>-
>-/**
>- * The largest value for content ID that fits in
>- * GENERIC_ELEMENT_CONTENT_ID_MASK
>- */
>-#define GENERIC_ELEMENT_CONTENT_ID_MAX_VALUE \
>-  ((PRUint32)((~PtrBits(0)) >> GENERIC_ELEMENT_CONTENT_ID_BITS_OFFSET))
>-
> 
> /**
>  * Class that implements the nsIDOMNodeList interface (a list of children of
>  * the content), by holding a reference to the content and delegating GetLength
>  * and Item to its existing child list.
>  * @see nsIDOMNodeList
>  */
> class nsChildContentList : public nsGenericDOMNodeList 
>@@ -182,19 +168,16 @@ public:
>     */
>     nsIContent* mBindingParent;  // [Weak]
> 
>     /**
>     * The controllers of the XUL Element.
>     */
>     nsIControllers* mControllers; // [OWNER]
>   };
>-
>-  // DEPRECATED, DON'T USE THIS
>-  PRUint32 mContentID;
> };
> 
> class RangeListMapEntry : public PLDHashEntryHdr
> {
> public:
>   RangeListMapEntry(const void *aKey)
>     : mKey(aKey), mRangeList(nsnull)
>   {
>@@ -411,18 +394,16 @@ public:
>   virtual nsresult RangeAdd(nsIDOMRange* aRange);
>   virtual void RangeRemove(nsIDOMRange* aRange);
>   virtual const nsVoidArray *GetRangeList() const;
>   virtual nsresult HandleDOMEvent(nsPresContext* aPresContext,
>                             nsEvent* aEvent,
>                             nsIDOMEvent** aDOMEvent,
>                             PRUint32 aFlags,
>                             nsEventStatus* aEventStatus);
>-  virtual PRUint32 ContentID() const;
>-  virtual void SetContentID(PRUint32 aID);
>   virtual void SetFocus(nsPresContext* aContext);
>   virtual nsIContent *GetBindingParent() const;
>   virtual PRBool IsContentOfType(PRUint32 aFlags) const;
>   virtual nsresult GetListenerManager(nsIEventListenerManager** aResult);
>   virtual already_AddRefed<nsIURI> GetBaseURI() const;
>   virtual void* GetProperty(nsIAtom  *aPropertyName,
>                             nsresult *aStatus = nsnull) const;
>   virtual nsresult SetProperty(nsIAtom            *aPropertyName,
>@@ -896,37 +877,29 @@ protected:
>       return NS_REINTERPRET_CAST(nsDOMSlots *, mFlagsOrSlots)->mFlags;
>     }
> 
>     return mFlagsOrSlots;
>   }
> 
>   void SetFlags(PtrBits aFlagsToSet)
>   {
>-    NS_ASSERTION(!((aFlagsToSet & GENERIC_ELEMENT_CONTENT_ID_MASK) &&
>-                   (aFlagsToSet & ~GENERIC_ELEMENT_CONTENT_ID_MASK)),
>-                 "Whaaa, don't set content ID bits and flags together!!!");
>-
>     nsDOMSlots *slots = GetExistingDOMSlots();
> 
>     if (slots) {
>       slots->mFlags |= aFlagsToSet;
> 
>       return;
>     }
> 
>     mFlagsOrSlots |= aFlagsToSet;
>   }
> 
>   void UnsetFlags(PtrBits aFlagsToUnset)
>   {
>-    NS_ASSERTION(!((aFlagsToUnset & GENERIC_ELEMENT_CONTENT_ID_MASK) &&
>-                   (aFlagsToUnset & ~GENERIC_ELEMENT_CONTENT_ID_MASK)),
>-                 "Whaaa, don't set content ID bits and flags together!!!");
>-
>     nsDOMSlots *slots = GetExistingDOMSlots();
> 
>     if (slots) {
>       slots->mFlags &= ~aFlagsToUnset;
> 
>       return;
>     }
> 
>Index: content/base/src/nsGenericElement.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/base/src/nsGenericElement.cpp,v
>retrieving revision 3.423
>diff -u -p -d -8 -r3.423 nsGenericElement.cpp
>--- content/base/src/nsGenericElement.cpp	2 Nov 2005 02:42:38 -0000	3.423
>+++ content/base/src/nsGenericElement.cpp	11 Nov 2005 21:52:14 -0000
>@@ -761,19 +761,18 @@ nsDOMEventRTTearoff::AddEventListener(co
> 
>   return listener_manager->AddEventListenerByType(aListener, aType, flags,
>                                                   nsnull);
> }
> 
> //----------------------------------------------------------------------
> 
> nsDOMSlots::nsDOMSlots(PtrBits aFlags)
>-  : mFlags(aFlags & ~GENERIC_ELEMENT_CONTENT_ID_MASK),
>-    mBindingParent(nsnull),
>-    mContentID(aFlags >> GENERIC_ELEMENT_CONTENT_ID_BITS_OFFSET)
>+  : mFlags(aFlags),
>+    mBindingParent(nsnull)
> {
> }
> 
> nsDOMSlots::~nsDOMSlots()
> {
>   if (mChildNodes) {
>     mChildNodes->DropReference();
>   }
>@@ -785,18 +784,17 @@ nsDOMSlots::~nsDOMSlots()
>   if (mAttributeMap) {
>     mAttributeMap->DropReference();
>   }
> }
> 
> PRBool
> nsDOMSlots::IsEmpty()
> {
>-  return (!mChildNodes && !mStyle && !mAttributeMap && !mBindingParent &&
>-          mContentID < GENERIC_ELEMENT_CONTENT_ID_MAX_VALUE);
>+  return (!mChildNodes && !mStyle && !mAttributeMap && !mBindingParent);
> }
> 
> PR_STATIC_CALLBACK(void)
> NopClearEntry(PLDHashTable *table, PLDHashEntryHdr *entry)
> {
>   // Do nothing
> }
> 
>@@ -2242,47 +2240,16 @@ nsGenericElement::HandleDOMEvent(nsPresC
>     // Now that we're done with this event, remove the flag that says
>     // we're in the process of dispatching this event.
>     NS_MARK_EVENT_DISPATCH_DONE(aEvent);
>   }
> 
>   return ret;
> }
> 
>-PRUint32
>-nsGenericElement::ContentID() const
>-{
>-  nsDOMSlots *slots = GetExistingDOMSlots();
>-
>-  if (slots) {
>-    return slots->mContentID;
>-  }
>-
>-  PtrBits flags = GetFlags();
>-
>-  return flags >> GENERIC_ELEMENT_CONTENT_ID_BITS_OFFSET;
>-}
>-
>-void
>-nsGenericElement::SetContentID(PRUint32 aID)
>-{
>-  // This should be in the constructor!!!
>-
>-  if (HasDOMSlots() || aID > GENERIC_ELEMENT_CONTENT_ID_MAX_VALUE) {
>-    nsDOMSlots *slots = GetDOMSlots();
>-
>-    if (slots) {
>-      slots->mContentID = aID;
>-    }
>-  } else {
>-    UnsetFlags(GENERIC_ELEMENT_CONTENT_ID_MASK);
>-    SetFlags(aID << GENERIC_ELEMENT_CONTENT_ID_BITS_OFFSET);
>-  }
>-}
>-
> NS_IMETHODIMP
> nsGenericElement::MaybeTriggerAutoLink(nsIDocShell *aShell)
> {
>   return NS_OK;
> }
> 
> nsIAtom*
> nsGenericElement::GetID() const
>Index: content/html/content/src/nsGenericHTMLElement.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,v
>retrieving revision 1.620
>diff -u -p -d -8 -r1.620 nsGenericHTMLElement.cpp
>--- content/html/content/src/nsGenericHTMLElement.cpp	3 Nov 2005 16:35:28 -0000	1.620
>+++ content/html/content/src/nsGenericHTMLElement.cpp	11 Nov 2005 21:08:36 -0000
>@@ -304,25 +304,16 @@ nsGenericHTMLElement::CopyInnerTo(nsGene
>     value->ToString(valStr);
>     rv = aDst->SetAttr(name->NamespaceID(), name->LocalName(),
>                        name->GetPrefix(), valStr, PR_FALSE);
>     NS_ENSURE_SUCCESS(rv, rv);
>   }
> 
>   nsIDocument *newDoc = aDst->GetOwnerDoc();
> 
>-  PRInt32 id;
>-  if (newDoc) {
>-    id = newDoc->GetAndIncrementContentID();
>-  } else {
>-    id = PR_INT32_MAX;
>-  }
>-
>-  aDst->SetContentID(id);
>-
>   if (aDeep) {
>     nsIDocument *doc = GetOwnerDoc();
>     if (doc == newDoc) {
>       rv = CloneChildrenTo(aDst);
>     }
>     else {
>       nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(newDoc);
>       rv = ImportChildrenTo(aDst, domDoc);
>Index: content/html/document/src/nsHTMLContentSink.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/html/document/src/nsHTMLContentSink.cpp,v
>retrieving revision 3.733
>diff -u -p -d -8 -r3.733 nsHTMLContentSink.cpp
>--- content/html/document/src/nsHTMLContentSink.cpp	28 Oct 2005 11:25:08 -0000	3.733
>+++ content/html/document/src/nsHTMLContentSink.cpp	11 Nov 2005 21:08:01 -0000
>@@ -886,18 +886,16 @@ HTMLContentSink::CreateContentObject(con
> 
>   // Make the content object
>   nsGenericHTMLElement* result = MakeContentObject(aNodeType, nodeInfo, aForm,
>                                              !!mInsideNoXXXTag, PR_TRUE).get();
>   if (!result) {
>     return nsnull;
>   }
> 
>-  result->SetContentID(mDocument->GetAndIncrementContentID());
>-
>   return result;
> }
> 
> nsresult
> NS_NewHTMLElement(nsIContent** aResult, nsINodeInfo *aNodeInfo)
> {
>   *aResult = nsnull;
> 
>@@ -3632,18 +3630,16 @@ HTMLContentSink::ProcessBASETag(const ns
>     nsCOMPtr<nsINodeInfo> nodeInfo;
>     mNodeInfoManager->GetNodeInfo(nsHTMLAtoms::base, nsnull,
>                                   kNameSpaceID_None,
>                                   getter_AddRefs(nodeInfo));
> 
>     result = NS_NewHTMLElement(getter_AddRefs(element), nodeInfo);
>     NS_ENSURE_SUCCESS(result, result);
> 
>-    element->SetContentID(mDocument->GetAndIncrementContentID());
>-
>     // Add in the attributes and add the base content object to the
>     // head container.
>     result = AddAttributes(aNode, element);
>     NS_ENSURE_SUCCESS(result, result);
> 
>     parent->AppendChildTo(element, PR_FALSE);
>     if (!mInsideNoXXXTag) {
>       nsAutoString value;
>@@ -3675,18 +3671,16 @@ HTMLContentSink::ProcessLINKTag(const ns
>     nsCOMPtr<nsIContent> element;
>     nsCOMPtr<nsINodeInfo> nodeInfo;
>     mNodeInfoManager->GetNodeInfo(nsHTMLAtoms::link, nsnull, kNameSpaceID_None,
>                                   getter_AddRefs(nodeInfo));
> 
>     result = NS_NewHTMLElement(getter_AddRefs(element), nodeInfo);
>     NS_ENSURE_SUCCESS(result, result);
> 
>-    element->SetContentID(mDocument->GetAndIncrementContentID());
>-
>     nsCOMPtr<nsIStyleSheetLinkingElement> ssle(do_QueryInterface(element));
> 
>     if (ssle) {
>       // XXX need prefs. check here.
>       if (!mInsideNoXXXTag) {
>         ssle->InitStyleLinkElement(mParser, PR_FALSE);
>         ssle->SetEnableUpdates(PR_FALSE);
>       } else {
>@@ -3751,18 +3745,16 @@ HTMLContentSink::ProcessMETATag(const ns
>                                      getter_AddRefs(nodeInfo));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   nsRefPtr<nsGenericHTMLElement> it = NS_NewHTMLMetaElement(nodeInfo);
>   if (!it) {
>     return NS_ERROR_OUT_OF_MEMORY;
>   }
> 
>-  it->SetContentID(mDocument->GetAndIncrementContentID());
>-
>   // Add in the attributes and add the meta content object to the head
>   // container.
>   AddBaseTagInfo(it);
>   rv = AddAttributes(aNode, it);
>   if (NS_FAILED(rv)) {
>     return rv;
>   }
> 
>Index: content/xml/document/src/nsXMLContentSink.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/xml/document/src/nsXMLContentSink.cpp,v
>retrieving revision 1.374
>diff -u -p -d -8 -r1.374 nsXMLContentSink.cpp
>--- content/xml/document/src/nsXMLContentSink.cpp	28 Oct 2005 11:25:13 -0000	1.374
>+++ content/xml/document/src/nsXMLContentSink.cpp	11 Nov 2005 21:07:39 -0000
>@@ -917,20 +917,16 @@ nsXMLContentSink::HandleStartElement(con
>   result = mNodeInfoManager->GetNodeInfo(localName, prefix, nameSpaceID,
>                                          getter_AddRefs(nodeInfo));
>   NS_ENSURE_SUCCESS(result, result);
> 
>   result = CreateElement(aAtts, aAttsCount, nodeInfo, aLineNumber,
>                          getter_AddRefs(content), &appendContent);
>   NS_ENSURE_SUCCESS(result, result);
> 
>-  if (mDocument) {
>-    content->SetContentID(mDocument->GetAndIncrementContentID());
>-  }
>-
>   // Set the ID attribute atom on the node info object for this node
>   // This must occur before the attributes are added so the name
>   // of the id attribute is known.
>   if (aIndex != -1 && NS_SUCCEEDED(result)) {
>     nsCOMPtr<nsIAtom> IDAttr = do_GetAtom(aAtts[aIndex]);
> 
>     if (IDAttr) {
>       nodeInfo->SetIDAttributeAtom(IDAttr);
>Index: content/xul/content/src/nsXULElement.h
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/xul/content/src/nsXULElement.h,v
>retrieving revision 1.202
>diff -u -p -d -8 -r1.202 nsXULElement.h
>--- content/xul/content/src/nsXULElement.h	7 Nov 2005 20:53:47 -0000	1.202
>+++ content/xul/content/src/nsXULElement.h	11 Nov 2005 21:02:12 -0000
>@@ -497,19 +497,16 @@ public:
>     }
> #endif
>     virtual nsresult HandleDOMEvent(nsPresContext* aPresContext,
>                               nsEvent* aEvent,
>                               nsIDOMEvent** aDOMEvent,
>                               PRUint32 aFlags,
>                               nsEventStatus* aEventStatus);
> 
>-    virtual PRUint32 ContentID() const;
>-    virtual void SetContentID(PRUint32 aID);
>-
>     virtual nsresult RangeAdd(nsIDOMRange* aRange);
>     virtual void RangeRemove(nsIDOMRange* aRange);
>     virtual const nsVoidArray *GetRangeList() const;
>     virtual void SetFocus(nsPresContext* aPresContext);
>     virtual void RemoveFocus(nsPresContext* aPresContext);
> 
>     virtual nsIContent *GetBindingParent() const;
>     virtual PRBool IsContentOfType(PRUint32 aFlags) const;
>Index: content/xul/content/src/nsXULElement.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/xul/content/src/nsXULElement.cpp,v
>retrieving revision 1.597
>diff -u -p -d -8 -r1.597 nsXULElement.cpp
>--- content/xul/content/src/nsXULElement.cpp	2 Nov 2005 02:42:38 -0000	1.597
>+++ content/xul/content/src/nsXULElement.cpp	11 Nov 2005 21:02:02 -0000
>@@ -1986,27 +2004,16 @@ nsXULElement::HandleDOMEvent(nsPresConte
>         // we're in the process of dispatching this event.
>         NS_MARK_EVENT_DISPATCH_DONE(aEvent);
>     }
> 
>     return ret;
> }
> 
> 
>-PRUint32
>-nsXULElement::ContentID() const
>-{
>-    return 0;
>-}
>-
>-void
>-nsXULElement::SetContentID(PRUint32 aID)
>-{
>-}
>-
> nsresult
> nsXULElement::RangeAdd(nsIDOMRange* aRange)
> {
>     // rdf content does not yet support DOM ranges
>     return NS_OK;
> }
> 
> 
>Index: content/xul/document/src/nsXULDocument.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/content/xul/document/src/nsXULDocument.cpp,v
>retrieving revision 1.680
>diff -u -p -d -8 -r1.680 nsXULDocument.cpp
>--- content/xul/document/src/nsXULDocument.cpp	2 Nov 2005 00:41:47 -0000	1.680
>+++ content/xul/document/src/nsXULDocument.cpp	11 Nov 2005 21:03:29 -0000
>@@ -63,17 +63,16 @@
> */
> 
> // Note the ALPHABETICAL ORDERING
> #include "nsXULDocument.h"
> 
> #include "nsDOMError.h"
> #include "nsIBoxObject.h"
> #include "nsIChromeRegistry.h"
>-#include "nsIContentSink.h" // for NS_CONTENT_ID_COUNTER_BASE
> #include "nsIScrollableView.h"
> #include "nsIContentViewer.h"
> #include "nsGUIEvent.h"
> #include "nsIDOMXULElement.h"
> #include "nsIPrivateDOMEvent.h"
> #include "nsIRDFNode.h"
> #include "nsIRDFRemoteDataSource.h"
> #include "nsIRDFService.h"
>@@ -3471,18 +3470,16 @@ nsXULDocument::CreateElementFromPrototyp
>             result->BeginAddingChildren();
>         }
> #endif
> 
>         rv = AddAttributes(aPrototype, result);
>         if (NS_FAILED(rv)) return rv;
>     }
> 
>-    result->SetContentID(mNextContentID++);
>-
>     result.swap(*aResult);
> 
>     return NS_OK;
> }
> 
> nsresult
> nsXULDocument::CreateOverlayElement(nsXULPrototypeElement* aPrototype,
>                                     nsIContent** aResult)
>Index: layout/build/nsContentDLF.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/layout/build/nsContentDLF.cpp,v
>retrieving revision 1.69
>diff -u -p -d -8 -r1.69 nsContentDLF.cpp
>--- layout/build/nsContentDLF.cpp	12 Sep 2005 18:41:11 -0000	1.69
>+++ layout/build/nsContentDLF.cpp	11 Nov 2005 06:27:49 -0000
>@@ -366,17 +366,16 @@ nsContentDLF::CreateBlankDocument(nsILoa
> 
>     // blat in the structure
>     if (htmlElement && headElement && bodyElement) {
>       rv = blankDoc->SetRootContent(htmlElement);
>       if (NS_SUCCEEDED(rv)) {
>         rv = htmlElement->AppendChildTo(headElement, PR_FALSE);
> 
>         if (NS_SUCCEEDED(rv)) {
>-          bodyElement->SetContentID(blankDoc->GetAndIncrementContentID());
>           // XXXbz Why not notifying here?
>           htmlElement->AppendChildTo(bodyElement, PR_FALSE);
>         }
>       }
>     }
>   }
> 
>   // add a nice bow
>Index: accessible/src/msaa/nsDocAccessibleWrap.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/accessible/src/msaa/nsDocAccessibleWrap.cpp,v
>retrieving revision 1.34
>diff -u -p -d -8 -r1.34 nsDocAccessibleWrap.cpp
>--- accessible/src/msaa/nsDocAccessibleWrap.cpp	9 Oct 2005 02:36:27 -0000	1.34
>+++ accessible/src/msaa/nsDocAccessibleWrap.cpp	11 Nov 2005 06:26:04 -0000
>@@ -272,17 +272,16 @@ NS_IMETHODIMP nsDocAccessibleWrap::FireT
> 
>   return NS_OK;
> }
> 
> PRInt32 nsDocAccessibleWrap::GetChildIDFor(nsIAccessible* aAccessible)
> {
>   // A child ID of the window is required, when we use NotifyWinEvent, so that the 3rd party application
>   // can call back and get the IAccessible the event occured on.
>-  // We use the unique ID exposed through nsIContent::GetContentID()
> 
>   void *uniqueID;
>   nsCOMPtr<nsIAccessNode> accessNode(do_QueryInterface(aAccessible));
>   if (!accessNode) {
>     return 0;
>   }
>   accessNode->GetUniqueID(&uniqueID);
> 
>Index: layout/generic/nsIStatefulFrame.h
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/layout/generic/nsIStatefulFrame.h,v
>retrieving revision 3.12
>diff -u -p -d -8 -r3.12 nsIStatefulFrame.h
>--- layout/generic/nsIStatefulFrame.h	27 Jan 2005 22:52:53 -0000	3.12
>+++ layout/generic/nsIStatefulFrame.h	12 Nov 2005 00:36:16 -0000
>@@ -12,18 +12,15 @@ class nsPresState;
> 
> class nsIStatefulFrame : public nsISupports {
>  public: 
>   NS_DEFINE_STATIC_IID_ACCESSOR(NS_ISTATEFULFRAME_IID)
> 
>   // If you create a special type stateful frame (e.g. scroll) that needs
>   // to be captured outside of the standard pass through the frames, you'll need
>   // a special ID by which to refer to that type.
>-  //
>-  // There is space reserved between standard ID's and special ID's by the
>-  // offset NS_CONTENT_ID_COUNTER_BASE
>   enum SpecialStateID {eNoID=0, eDocumentScrollState};
> 
>   NS_IMETHOD SaveState(nsPresContext* aPresContext, nsPresState** aState) = 0;
>   NS_IMETHOD RestoreState(nsPresContext* aPresContext, nsPresState* aState) = 0;
> };
> 
> #endif /* _nsIStatefulFrame_h */
>Index: parser/htmlparser/public/nsIContentSink.h
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/parser/htmlparser/public/nsIContentSink.h,v
>retrieving revision 1.11
>diff -u -p -d -8 -r1.11 nsIContentSink.h
>--- parser/htmlparser/public/nsIContentSink.h	24 Aug 2005 20:56:55 -0000	1.11
>+++ parser/htmlparser/public/nsIContentSink.h	11 Nov 2005 21:05:50 -0000
>@@ -53,22 +53,16 @@
> #include "mozFlushType.h"
> 
> class nsIParser;
> 
> #define NS_ICONTENT_SINK_IID \
> { 0x94ec4df1, 0x6885, 0x4b1f, \
>  { 0x85, 0x10, 0xe3, 0x5f, 0x4f, 0x36, 0xea, 0xaa } }
> 
>-// The base value for the content ID counter.
>-// Values greater than or equal to this base value are used
>-// by each of the content sinks to assign unique values
>-// to the content objects created by them.
>-#define NS_CONTENT_ID_COUNTER_BASE 10000
>-
> class nsIContentSink : public nsISupports {
> public:
> 
>   NS_DEFINE_STATIC_IID_ACCESSOR(NS_ICONTENT_SINK_IID)
> 
>   /**
>    * This method gets called when the parser begins the process
>    * of building the content model via the content sink.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 202770 [details] [diff] [review]
Proposed patch

Do you need to worry about anonymous children? I.e. where IndexOf will return -1?

I'm guessing you might when dealing with scroll position for anonymous content.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Possible, but there is no native anonymous content like that (other than the div in textareas which is created via NS_NewHTMLElement, so have no content ID), and XBL anon content content ids depend on the exact (non-guaranteed) loading order of the XBL bindings.  So in brief, that already doesn't work; if we want it to work we need something else here.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 202770 [details] [diff] [review]
Proposed patch

Ah, good point.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Merged to tip

Comment 9

[Boris Zbarsky [:bzbarsky]]

Fixed.