Last Comment Bug 677788 - length_is duplicates size_is
: length_is duplicates size_is
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla10
Assigned To: Bobby Holley (:bholley) (busy with Stylo)
:
Mentors:
Depends on:
Blocks: 728149
  Show dependency treegraph
 
Reported: 2011-08-09 17:24 PDT by Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
Modified: 2012-02-17 01:28 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Remove use of length_is xpt value from gecko. v1 (13.64 KB, patch)
2011-10-07 13:35 PDT, Bobby Holley (:bholley) (busy with Stylo)
mrbkap: review+
Details | Diff | Splinter Review
part 1 - Remove array/string 'capacity' argument from XPCConvert. v1 (15.17 KB, patch)
2011-10-19 22:33 PDT, Bobby Holley (:bholley) (busy with Stylo)
mrbkap: review+
Details | Diff | Splinter Review
part 2 - Remove use of length_is xpt value from gecko. v3 (19.09 KB, patch)
2011-10-19 22:34 PDT, Bobby Holley (:bholley) (busy with Stylo)
mrbkap: review+
Details | Diff | Splinter Review

Description Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-09 17:24:27 PDT
length_is is never used in our IDL, and the new IDL parser doesn't even support it.
Comment 1 Bobby Holley (:bholley) (busy with Stylo) 2011-10-07 13:18:14 PDT
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.
Comment 2 Bobby Holley (:bholley) (busy with Stylo) 2011-10-07 13:35:53 PDT
Created attachment 565637 [details] [diff] [review]
Remove use of length_is xpt value from gecko. v1

Added a patch. Flagging mrbkap for review.
Comment 3 Bobby Holley (:bholley) (busy with Stylo) 2011-10-19 22:33:31 PDT
Created attachment 568313 [details] [diff] [review]
part 1 - Remove array/string 'capacity' argument from XPCConvert. v1

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.
Comment 4 Bobby Holley (:bholley) (busy with Stylo) 2011-10-19 22:34:35 PDT
Created attachment 568314 [details] [diff] [review]
part 2 - Remove use of length_is xpt value from gecko. v3

And now, we update the previous patch a tiny bit. Flagging mrbkap for re-review - just looking at the interdiff should suffice.
Comment 5 Bobby Holley (:bholley) (busy with Stylo) 2011-11-05 11:14:44 PDT
Pushed this to try:
https://tbpl.mozilla.org/?tree=Try&rev=5bf9ebd712f1

Hopefully it can land before the branch.
Comment 6 Bobby Holley (:bholley) (busy with Stylo) 2011-11-06 04:38:03 PST
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.

Note You need to log in before you can comment on or make changes to this bug.