Closed
Bug 838304
Opened 12 years ago
Closed 12 years ago
null out mResult when removing container nodes
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: mak, Assigned: mak)
Details
Attachments
(1 file, 1 obsolete file)
6.33 KB,
patch
|
asaf
:
review+
|
Details | Diff | Splinter Review |
OnRemoving of container nodes I think we should null out mResult, like we do with mParent.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #710338 -
Flags: review?(mano)
Updated•12 years ago
|
Attachment #710338 -
Flags: review?(mano) → review+
Assignee | ||
Comment 2•12 years ago
|
||
Target Milestone: --- → mozilla21
Assignee | ||
Comment 3•12 years ago
|
||
backed out, looks like some of our xpcshell tests rely on this, scary!
https://hg.mozilla.org/integration/mozilla-inbound/rev/69f50b3e8bb1
TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/bookmarks/test_423515_forceCopyShortcuts.js
TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_history_sidebar.js
TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_tagging.js
Looking into them.
Target Milestone: mozilla21 → ---
Assignee | ||
Comment 4•12 years ago
|
||
ah looks like it's just an annoying wrong assertion
ASSERTION: Containers must have valid results: 'container->mResult'
the truth is that either mParent and mResult are valid, or are invalid, if they are invalid the node is a zombie, nothing wrong with that.
Assignee | ||
Comment 5•12 years ago
|
||
a bit more changes than expected so reasking for review, tests are happy locally.
Looks like in the past we lived in an epoch where result was an invariant for containers, but most of the code null checks it, and I think it's healthier to do so than to keep a result around.
Attachment #710338 -
Attachment is obsolete: true
Attachment #710413 -
Flags: review?(mano)
Updated•12 years ago
|
Attachment #710413 -
Flags: review?(mano) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Target Milestone: --- → mozilla21
Comment 7•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•