Closed Bug 651124 Opened 14 years ago Closed 11 years ago

Remove GetChildAt() usage in nsBindingManager

Categories

(Core :: XBL, defect)

Product:
Core
Component:
XBL
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla26

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file, 4 obsolete files)

nsbindingmanager-remove-getchildat.patch
(deleted), patch
mrbkap
: review+
Details | Diff | Splinter Review
No description provided.
Attached patch Patch v1 (obsolete) (deleted) — — Splinter Review
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #536345 - Flags: review?(jonas)
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+
(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().
Attached patch Patch v2 (obsolete) (deleted) — — Splinter Review
A patch trying to prevent IndexOf() usage.
Attachment #536611 - Flags: review?(jonas)
Attached patch Patch v2 (obsolete) (deleted) — — Splinter Review
Attachment #536611 - Attachment is obsolete: true
Attachment #536611 - Flags: review?(jonas)
Attachment #536612 - Flags: review?(jonas)
Flags: in-testsuite-
Blocks: 661296
Summary: Make nsBindingManager::ContentAppended not use the index parameter → Make nsBindingManager::ContentAppended and nsBindingManager::ContentInserted not use the index parameter
Attached patch Patch v2.1 (obsolete) (deleted) — — Splinter Review
Removes the index for ::ContentInserted()
Attachment #536612 - Attachment is obsolete: true
Attachment #536612 - Flags: review?(jonas)
Attachment #536664 - Flags: review?(jonas)
Jonas, what about trying to do this review before it gets 4 months old? :)
Bug 653881 removes all this code anyway.
Depends on: 653881
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)
Whiteboard: [needs review] → [should be made obsolete by bug 653881]
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)
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]
Attachment #788707 - Flags: review?(mrbkap) → review+
https://hg.mozilla.org/mozilla-central/rev/f64b25b24502
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: