Closed
Bug 505316
Opened 15 years ago
Closed 15 years ago
Embedders and users should not have to know about API versioning
Categories
(Tamarin Graveyard :: Virtual Machine, defect, P2)
Tamarin Graveyard
Virtual Machine
Tracking
(Not tracked)
VERIFIED
FIXED
flash10.1
People
(Reporter: jodyer, Assigned: jodyer)
References
Details
Attachments
(1 file)
514.73 KB,
patch
|
lhansen
:
review+
|
Details | Diff | Splinter Review |
API versioning is an optional (and complex) feature that currently requires an updated asc.jar and modifications to host embedding code (ShellCore). Let's not force such a change on those who don't want it.
Requirments:
- allow TR to work with any asc.jar that FP10 works with
- allow TR to work without any (versioning related) modifications to embedding code
this patch set api versioning defaults in AvmCore so that ShellCore doesn't have to. The defaults are that there is just one version (0) and no versioned namespaces. The result is that no names (or namespaces) are marked with a version number.
(This patch requires that https://bugzilla.mozilla.org/attachment.cgi?id=389500&action=edit has already been applied.)
Attachment #389571 -
Flags: superreview?(edwsmith)
Attachment #389571 -
Flags: review?(lhansen)
Comment 2•15 years ago
|
||
Comment on attachment 389571 [details] [diff] [review]
first candidate
Two style nits:
Both apis and apis_sizes should be defined as const arrays, since I believe you don't want to update them.
For the call to setAPIInfo, instead of passing '1' as the third parameter it is customary to pass sizeof(avmplus::apis)/sizeof(avmplus::apis[0]). That way a change to the array does not need to be accomodated by changes to the code. Ditto for any other parameters in this list to which that might apply.
Attachment #389571 -
Flags: review?(lhansen) → review+
Comment 3•15 years ago
|
||
+1 to Lars minor nits, otherwise, looks good to me too.
Fixes of nits have been integrated into the latest patch of bug 472920. In Ed's absence, will take his super review there and steven's review here as SR+ for this patch (unless I hear that I should do otherwise.)
Status: NEW → ASSIGNED
Comment 5•15 years ago
|
||
(In reply to comment #4)
> Fixes of nits have been integrated into the latest patch of bug 472920. In Ed's
> absence, will take his super review there and steven's review here as SR+ for
> this patch (unless I hear that I should do otherwise.)
Make it so.
Comment 7•15 years ago
|
||
QRB: target for current milestone
QE: in-testsuite:? since we should do a one off test using an pre-"API
versioning" asc.jar
Flags: in-testsuite?
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
Flags: flashplayer-qrb? → flashplayer-qrb+
Target Milestone: --- → flash10.1
Updated•15 years ago
|
Attachment #389571 -
Flags: superreview?(edwsmith)
Pushed (see Comment #6)
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•