Closed Bug 572609 Opened 10 years ago Closed 10 years ago

Use faster GetElementById() method...

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jst, Assigned: jst)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(2 files)

Now that we have a GetElementById() method in nsIDocument that doesn't reference count or QI the element or anything, we should use it!
Attachment #451827 - Flags: review?(jonas)
Assignee: nobody → jst
Comment on attachment 451827 [details] [diff] [review]
Use nsIDocument::GetElementById()

Nice!

>diff --git a/content/xul/document/src/nsXULDocument.cpp b/content/xul/document/src/nsXULDocument.cpp
>@@ -4065,17 +4053,18 @@ nsXULDocument::OverlayForwardReference::
>     for (i = 0; i < childCount; ++i) {
>         currContent = aOverlayNode->GetChildAt(0);
> 
>-        nsAutoString id;
>-        currContent->GetAttr(kNameSpaceID_None, nsGkAtoms::id, id);
>-
>-        nsCOMPtr<nsIDOMElement> nodeInDocument;
>-        if (!id.IsEmpty()) {
>-            nsCOMPtr<nsIDOMDocument> domDocument(
>-                        do_QueryInterface(aTargetNode->GetDocument()));
>-            if (!domDocument) return NS_ERROR_FAILURE;
>-
>-            rv = domDocument->GetElementById(id, getter_AddRefs(nodeInDocument));
>-            if (NS_FAILED(rv)) return rv;
>+        nsIAtom *idAtom = currContent->GetID();
>+
>+        nsIContent *elementInDocument = nsnull;
>+        if (idAtom) {
>+            nsDependentAtomString id(idAtom);
>+
>+            if (!id.IsEmpty()) {

No need for this check. GetID will never return an empty-string atom.

r=me with that fixed.
This landed as http://hg.mozilla.org/mozilla-central/rev/82a21d443b5e but got backed out due to test failures in the browser chrome test browser_bug415846.js. I've narrowed it down to the changes in nsXULDocument.cpp, but not which of those changes yet...
OS: Linux → All
Hardware: x86 → All
This applies on top of the actual fix and eliminates random returns of random values from nsXULDocument::InsertElement()
Attachment #453411 - Flags: review?(jonas)
Fixed.

http://hg.mozilla.org/mozilla-central/rev/f269bf14f264
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.