Closed
Bug 677788
Opened 13 years ago
Closed 13 years ago
length_is duplicates size_is
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: khuey, Assigned: bholley)
References
Details
Attachments
(2 files, 1 obsolete file)
15.17 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
19.09 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
length_is is never used in our IDL, and the new IDL parser doesn't even support it.
Assignee | ||
Comment 1•13 years ago
|
||
So right now we put size_is in both the size_is and length_is slot in the typelib format: http://hg.mozilla.org/mozilla-central/file/35954e6f3167/xpcom/idl-parser/typelib.py#l86 http://hg.mozilla.org/mozilla-central/file/35954e6f3167/xpcom/idl-parser/typelib.py#l88 http://hg.mozilla.org/mozilla-central/file/35954e6f3167/xpcom/idl-parser/typelib.py#l99 Within XPConnect, we sometimes check size_is, and sometimes query length_is. We should immediately stop checking length_is within gecko, and use size_is exclusively. This will remove gecko's dependency on those bits so that, in a year or two, we'll be in a better place to kill them off entirely.
Assignee: nobody → bobbyholley+bmo
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•13 years ago
|
||
Added a patch. Flagging mrbkap for review.
Attachment #565637 -
Flags: review?(mrbkap)
Updated•13 years ago
|
Attachment #565637 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 3•13 years ago
|
||
So I actually got this wrong last time. I thought that the calls to GetArraySizeFromParam and GetArrayLengthFromParam were writing into the same value, but it they're actually writing into two separate values: 'capacity' and 'count'. So the previous patch posted here broke arrays entirely. Unfortunately, we didn't have test coverage for dependent parameters, so it didn't show up in our xpc tests. which made for a very orange try push. ;-) I've now added test coverage for this stuff over in bug 693341, which gives me more confidence that I've got it right this time. length_is duplicates size_is, so 'capacity' is just an alias for 'count'. Let's get rid of it. Flagging mrbkap for review.
Attachment #568313 -
Flags: review?(mrbkap)
Assignee | ||
Comment 4•13 years ago
|
||
And now, we update the previous patch a tiny bit. Flagging mrbkap for re-review - just looking at the interdiff should suffice.
Attachment #568314 -
Flags: review?(mrbkap)
Assignee | ||
Updated•13 years ago
|
Attachment #565637 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #568314 -
Flags: review?(mrbkap) → review+
Updated•13 years ago
|
Attachment #568313 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 5•13 years ago
|
||
Pushed this to try: https://tbpl.mozilla.org/?tree=Try&rev=5bf9ebd712f1 Hopefully it can land before the branch.
Assignee | ||
Comment 6•13 years ago
|
||
Looks good on try. Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/6c1a7d01303f https://hg.mozilla.org/integration/mozilla-inbound/rev/104c466724ac Hopefully this should stick for FF10.
Target Milestone: --- → mozilla10
Comment 7•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6c1a7d01303f https://hg.mozilla.org/mozilla-central/rev/104c466724ac
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•