Embedders and users should not have to know about API versioning

VERIFIED FIXED in flash10.1

Status

Tamarin
Virtual Machine
P2
normal
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: Jeff Dyer, Assigned: Jeff Dyer)

Tracking

unspecified
flash10.1
Bug Flags:
in-testsuite +
flashplayer-qrb +
flashplayer-triage +

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
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
(Assignee)

Updated

9 years ago
Assignee: nobody → jodyer
Blocks: 472920
(Assignee)

Comment 1

9 years ago
Created attachment 389571 [details] [diff] [review]
first candidate

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

9 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

9 years ago
+1 to Lars minor nits, otherwise, looks good to me too.
(Assignee)

Comment 4

9 years ago
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

9 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.
(Assignee)

Comment 6

9 years ago
pushed: changeset - 2212:5d74ffcdc709

Comment 7

9 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?

Updated

9 years ago
Flags: flashplayer-qrb? → flashplayer-qrb+
Target Milestone: --- → flash10.1

Updated

9 years ago
Attachment #389571 - Flags: superreview?(edwsmith)

Comment 8

9 years ago
Jeff: if it's been pushed, can we close?
Priority: -- → P2
(Assignee)

Comment 9

9 years ago
Pushed (see Comment #6)
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Status: RESOLVED → VERIFIED

Comment 10

9 years ago
Testing with pre-api versioning done.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.