Remove GetChildAt() usage in nsBindingManager

RESOLVED FIXED in mozilla26

Status

()

Core
XBL
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

(Blocks: 1 bug)

Trunk
mozilla26
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

7 years ago
Created attachment 536345 [details] [diff] [review]
Patch v1
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #536345 - Flags: review?(jonas)
(Assignee)

Updated

7 years ago
Whiteboard: [needs review]
Comment on attachment 536345 [details] [diff] [review]
Patch v1

Review of attachment 536345 [details] [diff] [review]:
-----------------------------------------------------------------

Please watch out for performance regressions when landing this.
Attachment #536345 - Flags: review?(jonas) → review+
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)
> Please watch out for performance regressions when landing this.

I know. Though, I will give a shot at writing a patch that doesn't call IndexOf().
(Assignee)

Comment 4

7 years ago
Created attachment 536611 [details] [diff] [review]
Patch v2

A patch trying to prevent IndexOf() usage.
Attachment #536611 - Flags: review?(jonas)
(Assignee)

Comment 5

7 years ago
Created attachment 536612 [details] [diff] [review]
Patch v2
Attachment #536611 - Attachment is obsolete: true
Attachment #536611 - Flags: review?(jonas)
Attachment #536612 - Flags: review?(jonas)
(Assignee)

Updated

7 years ago
Flags: in-testsuite-
(Assignee)

Updated

7 years ago
Blocks: 661296
(Assignee)

Updated

7 years ago
Summary: Make nsBindingManager::ContentAppended not use the index parameter → Make nsBindingManager::ContentAppended and nsBindingManager::ContentInserted not use the index parameter
(Assignee)

Comment 6

7 years ago
Created attachment 536664 [details] [diff] [review]
Patch v2.1

Removes the index for ::ContentInserted()
Attachment #536612 - Attachment is obsolete: true
Attachment #536612 - Flags: review?(jonas)
Attachment #536664 - Flags: review?(jonas)
(Assignee)

Comment 7

7 years ago
Jonas, what about trying to do this review before it gets 4 months old? :)

Comment 8

6 years ago
Bug 653881 removes all this code anyway.
Depends on: 653881
(Assignee)

Comment 9

6 years ago
Neil, if this patches land before the patches in the other bug, is that going to be really bad?
Not that I expect Jonas to do the review before the 1-year anniversary :)
Comment on attachment 536664 [details] [diff] [review]
Patch v2.1

Review of attachment 536664 [details] [diff] [review]:
-----------------------------------------------------------------

I should have moved this one a long ago :(
Attachment #536664 - Flags: review?(jonas) → review?(mrbkap)
Comment on attachment 536664 [details] [diff] [review]
Patch v2.1

Clearing review request due to bug 653881 moving forward again.
Attachment #536664 - Flags: review?(mrbkap)
(Assignee)

Updated

5 years ago
Whiteboard: [needs review] → [should be made obsolete by bug 653881]
(Assignee)

Comment 12

5 years ago
Created attachment 788707 [details] [diff] [review]
nsbindingmanager-remove-getchildat.patch

Blake, this patch is pretty trivial and removes all the usage of GetChildAt() in nsBindingManager.
Attachment #536345 - Attachment is obsolete: true
Attachment #536664 - Attachment is obsolete: true
Attachment #788707 - Flags: review?(mrbkap)
(Assignee)

Updated

5 years ago
Blocks: 651120
No longer blocks: 651121, 661296
Summary: Make nsBindingManager::ContentAppended and nsBindingManager::ContentInserted not use the index parameter → Remove GetChildAt() usage in nsBindingManager
Whiteboard: [should be made obsolete by bug 653881]

Updated

5 years ago
Attachment #788707 - Flags: review?(mrbkap) → review+
https://hg.mozilla.org/mozilla-central/rev/f64b25b24502
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.